Worrisome error involving user prop "s1_lastn_style"

[foxfirefey]

So, due to some difficulties with my VPS provider, I've had to reinstall Dreamwidth from scratch. I didn't mind! It was a great opportunity to brush up some of the installation documentation and reacquaint myself with the installation process. However, I've run into an awkward error involving user prop "s1_lastn_style" not existing in this new installation.

The Apache error log says this:

Prop does not exist: user - s1_lastn_style at /home/dw/cgi-bin/ljlib.pl line 780.
[Wed Jul 22 16:04:32 2009] [error] [client 67.160.37.217] Invalid userprop s1_lastn_style passed to LJ::load_user_props. at /home/dw/cgi-bin/LJ/User.pm line 6815.\n, referer: http://foxfirefey.dev.memewidth.org/profile

I can stop it from happening by commenting out an offending line of code:

push @needed_props, $s1prop if $eff_view =~ /^(?:calendar|day|friends|lastn)$/;

Which you can see in more context here:

diff -r 215c981ac1f0 cgi-bin/LJ/User.pm
--- a/cgi-bin/LJ/User.pm        Tue Jul 21 23:08:22 2009 -0500
+++ b/cgi-bin/LJ/User.pm        Wed Jul 22 20:51:35 2009 +0000
@@ -8309,7 +8309,7 @@
     $u->{'_journalbase'} = LJ::journal_base($u->{'user'}, $opts->{'vhost'});
 
     my $eff_view = $LJ::viewinfo{$view}->{'styleof'} || $view;
-    my $s1prop = "s1_${eff_view}_style";
+    #my $s1prop = "s1_${eff_view}_style";
 
     my @needed_props = ("stylesys", "s2_style", "url", "urlname", "opt_nctalklinks",
                         "renamedto",  "opt_blockrobots", "opt_usesharedpic", "icbm",
@@ -8320,7 +8320,7 @@
     # is reply/month/entry/res and that means it *has* to be S2--S1 defaults to a
     # BML page to handle those, but we don't want to attempt to load a userprop
     # because now load_user_props dies if you try to load something invalid
-    push @needed_props, $s1prop if $eff_view =~ /^(?:calendar|day|friends|lastn)$/;
+    #push @needed_props, $s1prop if $eff_view =~ /^(?:calendar|day|friends|lastn)$/;
 
     # preload props the view creation code will need later (combine two selects)
     if (ref $LJ::viewinfo{$eff_view}->{'owner_props'} eq "ARRAY") {

`

But, I'm not sure that's the entire issue, so I'd really like people with more experience to know about this, just in case it's important. sophie has been helping me with suggestions, and I can note that:

* I have upgraded to tip
* Deleting all the files that are obsolete with $LJHOME/bin/cvsreport.pl -n -1 | xargs -i rm $LJHOME/{} has not fixed it
* This user should be using S2--the default style is S2 Negatives, and I haven't changed anything since account creation on them, and when I comment out that line the page does display Negatives.

sophie has also had me do a query of:

SELECT `userid`, `name`, `value` FROM `userprop` LEFT OUTER JOIN `userproplist` USING (`upropid`);

To make sure there are no null userprops or stylesys ones.

Comments

[denise]

I think this might be related to bug 1378.

[foxfirefey]

Huh! Agreed!

I didn't know if I just wasn't setting things up right or not. I wonder how much else is there to rip out?

[foxfirefey]

sophie reports it makes Dreamhacks go boom, too, so it is something that should definitely get fixed before the next code push! But I don't know yet if my one line fix is enough or if there is more to it I'm not seeing yet.

(Although it seems to be fine on older sites that already have those userprops stored, it's new installs that go sadface.)

[mark]

The s1 props should die, including all references to them in the code. Might be a little tricky to remove them, but I think your patch is probably right. Test a bunch!

[foxfirefey]

Well, things were broken before this patch, but after this patch they seem to be okay. Unfortunately, I have some concerns--there might be other issues lurking that I just don't know about yet. Someone should test this patch on a broken in install, to make sure it doesn't mess up existing installations--that I can't test so much on my new installation.

I did see a force_s1 hook being called that I might make another patch to remove, I doubt it would have an effect seeing as that hook no longer exists.

[kareila]

The problem is described right there in the comment: "load_user_props dies if you try to load something invalid." $u->preload_props is just a user method that calls LJ::load_user_props.

Now that we've pulled all the s1 props out, anything that tries to load any them will break. It's okay not to try to load them, because we weren't using them in the code anyway, because s1 is dead.

The reason it doesn't break on existing installs is that the properties still exist in the MySQL database; they've only been removed from the file that populates the database for new installs (proplists.dat).