blade_bunny (blade_bunny) wrote in lj_dev,
blade_bunny
blade_bunny
lj_dev

Proposed change to LJ::Error::errobj

In my continuing saga of trying to get livejournal running on RHEL 4 w/ perl 5.8.5 and mod_perl 1.99 (I know, wtf?)...

I ran into a weird erro that appears to originate in Error.pm:

[Fri Nov 17 04:31:23 2006] [error] [client 128.248.155.65] Several modules (APR::Bucket, APR::UUID, Apache::RequestUtil, APR::Brigade, APR::ThreadMutex, APR::IpSubnet, APR::Pool) contain method 'new' but none of them matches class 'LJ::Error::Database::Unavailable';\n at /livejournal/cgi-bin/LJ/Error.pm line 44\n


Now I'm not too sure where or why that error shows up, but it's not the ""Can't locate object method "new" via package "(LJ::Error::\S+?)""" error that errobj expects. Now, IMHO, the code that adds LJ::Error to the @ISA of LJ::Error::* is a wee bit ugly. Especially relying on particular text of an error message to determine whether it needs to touch @ISA. I propose it would be better to make use of UNIVERSAL::can('new') as shown below. It makes it more obvious what is going on, avoids a string eval (which are just kind of icky even when carefully checked) and it doesn't break my system. ;-)

[root@development-4 ~]# diff -U 6 /livejournal/cvs/livejournal/cgi-bin/LJ/Error.pm /livejournal/cgi-bin/LJ/Error.pm
--- /livejournal/cvs/livejournal/cgi-bin/LJ/Error.pm    2006-11-16 20:46:18.000000000 -0600
+++ /livejournal/cgi-bin/LJ/Error.pm    2006-11-17 10:31:11.000000000 -0600
@@ -38,20 +38,17 @@
 sub errobj {
     # constructing a new LJ::Error instance.  either with a classname
     # and args, or just a classname (no whitespace, must have one capital letter)
     if (@_ > 0 && (@_ > 1 || ($_[0] !~ /\s/ && $_[0] =~ /[A-Z]/ && $_[0] !~ /[^:\w]/))) {
         my ($classtail, @ctor_args) = @_;
         my $class = "LJ::Error::$classtail";
-        my $makeit = sub { $class->new(@ctor_args); };
-        my $val = eval { $makeit->(); };
-        if ($@ =~ /^Can\'t locate object method "new" via package "(LJ::Error::\S+?)"/) {
-            my $class = $1;
-            my $code = "\@${class}::ISA = ('LJ::Error'); 1;";
-            eval $code or die "Failed to set ISA: [$@] for code: [$code]\n";
+        unless( $class->can('new') ) {
+            no strict 'refs';
+            push @{"${class}::ISA"}, 'LJ::Error';
         }
-        return $val || $makeit->();
+        return $class->new(@ctor_args);
     }

     # if no parameters, act like errobj($@)
     unless (@_) {
         $_[0] = $@
             or return undef;

Subscribe
  • Post a new comment

    Error

    Anonymous comments are disabled in this journal

    default userpic

    Your reply will be screened

    Your IP address will be recorded 

  • 1 comment