More exception handling fun

autarch on 2005-07-09T01:42:42

A bit of insidious evil spotted by my coworker Chris Dent.

Consider, if you will, this bit of code:

eval "require $module" unless $module->can('new');
die $@ if $@;
return $module->new();

This is a handy way to conditionally load a class. You need to check $@ because if the module fails to load you want to know.

But there's a somewhat subtle bug lurking in there.

If the module has been loaded already, then the eval is never executed. But it always checks $@. This is bad. If earlier you had some code that did something like this:

eval { thing_that_may_die() };
log_error($@) if $@;
return if $@;

Now $@ has a value and since we may not ever call the string eval, then $@ will still be populated with an old exception at the die $@ if $@ line.

The upshot is that you need to be careful with your use of eval and $@. Make sure that checks on $@ are always preceded by an eval.

So our original bit of code can be rewritten as:

unless ( $module->can('new') )
{
    eval "require $module";
    die $@ if $@;
}
return $module->new();

---

And another nice thing about your use of braces...

davebaker on 2005-07-09T03:19:45

(in your rewritten code) is that $@ is cleared to become the empty string when control leaves the block. So you shouldn't have the pre-existing value trouble later on when testing "if $@".

At YAPC::NA::2005, Dave Rolsky cautioned that even with the braces around the eval and the test, you can get into trouble if a later test is "if defined($@)" ... because $@ will have been defined but contain the empty string after leaving an earlier block in which it was used. So he said to use "if length($@)" rather than if defined($@).

But now that I think about it, I'm not sure why your "if $@" wouldn't work just as well (i.e., it wouldn't return true if $@ contains the empty string, right?).

Caution: I am something of a newbie and might not know what I'm talking about :-)

Re:And another nice thing about your use of braces

autarch on 2005-07-09T03:36:04

That Dave Rolsky guy is a big fucking idiot!

Oh wait, I am Dave Rolsky!

The "if $@" bit is indeed a bit lazy. The chances of an exception being thrown that evaluates to false in a boolean context but has a length are close to 0, but yes, it's probably best to check "length $@".

Re:And another nice thing about your use of braces

Matts on 2005-07-09T13:07:15

Early versions of Error.pm exceptions evaluated to 0 in bool context, which really screwed things up (mostly it screwed my branes up!).

Close to 0

davebaker on 2005-07-09T17:09:20

Was that a pun :-) (See Matt's comment re earlier Error.pm exceptions.)

Re:

Aristotle on 2005-07-09T09:51:59

When an eval fails to compile the code or the code dies, the eval always returns undef. You can use this to shorten many such snippets:

eval "require $module; 1" or die $@ if not $module->can( 'new' );

In fact the 1 is actually redundant here, as require always returns a true value itself if it succeeds. To be precise, it returns the value of the last expression in the file, which, as everyone knows, must be true. But neither fact is, to my knowledge, documented.