Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical
 
PerlMonks  

Rethrowing with die $@ considered harmful

by ysth (Canon)
on Jul 18, 2006 at 00:07 UTC ( #561895=perlmeditation: print w/ replies, xml ) Need Help??

Rethrowing an exception with die $@ if (some expression involving $@) (an idiom mentioned at least 6 times in the perl documentation) has a problem when the expression involving $@ ends up doing anything that inadvertently clears $@. Consider:
use overload q!""!=>sub{ eval { "whoops" } }; eval { die bless {} }; die $@ if $@;
Here, we die with an exception object that should stringify to "whoops", but $@ ends up being cleared before the rethrow. (To highlight the maybe-expected vs. observed results, change eval { "whoops" } to just "whoops" or remove the if $@.) The better way to do it would be
use overload q!""!=>sub{ eval { "whoops" } }; eval { die bless {} }; my $exception = $@; die $exception if $exception;

Comment on Rethrowing with die $@ considered harmful
Select or Download Code
Re: Rethrowing with die $@ considered harmful
by Tanktalus (Canon) on Jul 18, 2006 at 04:11 UTC

    I have to wonder ... would P5P consider fixing the code to match the documentation? I have to admit that your first ("wrong") example DWIMs better than the ("right") second example.

    Obviously, then, one has to change - either the docs or the code - based on this discovery. I just think it should be the code.

      The latter is the generally recommended usage. Just deal. Anything that triggers perl code could clear $@. I wouldn't have normally thought to guard against an overloaded $@ but I suppose that's possible too. In general, when examining $@ you are always expected to copy it out.

      ⠤⠤ ⠙⠊⠕⠞⠁⠇⠑⠧⠊

        Generally recommended usage? I can easily find where ysth points out that the recommended usage is the first example. I can even find where, in perlcall, G_KEEPERR is mentioned to work around this issue from XS code (I think - I don't completely grok the internals). Where is it "generally recommended" to use the latter method? And would the rest of us notice it against the backdrop of perlcompile, perlembed (x2), perlform, and perlop giving examples such as the former?

Re: Rethrowing with die $@ considered harmful (local $_ buggy)
by ikegami (Pope) on Jul 18, 2006 at 06:05 UTC

    Similarly, I've started noticing issues with the common practice of doing local $_. In some situations, that's not enough to prevent damage to the caller. Using local *_ fixes those problems. For example, consider the following function:

    sub dequote { local $_ = @_ ? $_[0] : $_; s/^"//; s/"$//; s/\\(.)/$1/g; return $_; }

    It assigns its parameter (or $_ if there are no parameters) to $_. It attempts to prevent damage to the caller by localizing $_ first, but I will show it doesn't always work.

    Case 1

    $_ is tied, or $_ is an alias to something which is tied.

    Case 2

    pos($_) is used by the caller.

    The Solution

    sub dequote { #local $_ = @_ ? $_[0] : $_; # XXX my $s = @_ ? $_[0] : $_; # Fix local *_ = \$s; # Fix s/^"//; s/"$//; s/\\(.)/$1/g; return $_; }

      In my own code i always local'ize $_ using for, as recommended in Camel III:

      Solution 2

      sub dequote { my $string = @_ ? $_[0] : $_; for ($string) { s/^"//; s/"$//; s/\\(.)/$1/g; return $_; } }
      It's IMO even more readable than the local $_ variant, and passes all your tests.

           s;;Just-me-not-h-Ni-m-P-Ni-lm-I-ar-O-Ni;;tr?IerONim-?HAcker ?d;print
      Another alternative: The local of Data::Alias does it right (it ignores ties and such), so this works:
      alias(local $_) = @_ ? $_[0] : $_;

      And it avoids localizing the entire *_

      The parens here are needed because alias local $_ = ... would be parsed as alias(local $_ = ...) which means the assignment would make an alias, which isn't what you want here.

Re: Rethrowing with die $@ considered harmful
by shmem (Canon) on Jul 18, 2006 at 07:56 UTC
    Rethrowing an exception with die $@ if (some expression involving $@) (an idiom mentioned at least 6 times in the perl documentation) has a problem when the expression involving $@ ends up doing anything that inadvertently clears $@.

    I would consider inadvertently clearing $@ (or $!) as harmful, not the rethrowing.

    When you ask for problems, you get them. E.g. if you overload, you should know what you are doing...

    --shmem

    _($_=" "x(1<<5)."?\n".q/)Oo.  G\        /
                                  /\_/(q    /
    ----------------------------  \__(m.====.(_("always off the crowd"))."
    ");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}
      I would consider inadvertently clearing $@ (or $!) as harmful, not the rethrowing.

      But there are so many places where this can happen.

      eval { foo() } ; if ( $@ ) { bar(); die $@ }

      Does bar() throw an exception itself? If it doesn't now will it sometime in the future? What about the stuff that bar() calls? What about the next version of the module Foo used by module Bar used by the bar() function?

      If you rely on $@ you're relying on everything in your exception handling code restoring it before you use it. Me - I'll copy it instead. Coupling bad.

      Sure the overloading thing is daft - but that doesn't mean copying $@ isn't the right thing to do.

        I consider this code buggy. If $@ is set, it must be reported/processed immediately without any further function call. If you just can't die from it, use warn:
        eval { foo() } ; if ( $@ ) { warn $@; bar(); } Undefined subroutine &main::foo called at 561955.pl line 2. Undefined subroutine &main::bar called at 561955.pl line 5.

        Note: I'm not against copying $@. If you need to make further calls first, copy it and report it later. But inadvertently clearing is a programmer's bug, not perl's fault.

        --shmem

        _($_=" "x(1<<5)."?\n".q/)Oo.  G\        /
                                      /\_/(q    /
        ----------------------------  \__(m.====.(_("always off the crowd"))."
        ");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}
        200th post
        Does bar() throw an exception itself?

        Getting away from working with $@ directly is one reason why I wrote Exception::Class::TryCatch -- the try keyword on eval isn't just sugar, it saves away $@ onto a stack for a later catch. That way, it can even be nested. E.g.

        use Exception::Class::TryCatch; try eval { Exception::Class::Base->throw('error') }; # can eval or use try/catch again regardless of $@ above bar(); catch my $err; # catches the matching "try"

        -xdg

        Code written by xdg and posted on PerlMonks is public domain. It is provided as is with no warranties, express or implied, of any kind. Posted code may not have been tested. Use of posted code is at your own risk.

Re: Rethrowing with die $@ considered harmful
by fce2 (Sexton) on Jul 18, 2006 at 11:32 UTC
    I don't know if its directly related, but this makes me think of a LDAP helper module I wrote for a system I now have in production.

    I won't go into too much detail, but (among other things) it had a "login" and "logout" method. Each updated a global session data hash with info about the current user. These methods throw exceptions if something goes wrong. Nothing fancy so far.

    Later, I wanted to make a facility for administrators to "become" a normal user (think Unix "su"). However, I wanted to make it so that if the operation failed, it would restore the existing session. So, I wrote this code:

    my %ubackup = %udat; eval { $self->logout; $self->login(username => $username); }; if($@) { %udat = %ubackup; die $@; }
    Now this looks fine, but any errors were being mysteriously swallowed. Turns out the culprit was my LDAP object destructor, which did this:

    sub DESTROY { my ($self) = @_; eval { $self->_unbind; }; }
    ie shut down the connection, and ignore errors.

    The login/logout methods create LDAP objects as part of their operation, so of course they were being destroyed at the end of the method. The call to _unbind was trampling $@.

    This was a nightmare to track down. The solution was to simply localise $@ in the destructor.

    local $@; eval { $self->_unbind; };
      This was a nightmare to track down. The solution was to simply localise $@ in the destructor.

      Yeah, side effects and destructors don't mix well accoding to perlobj...

      Since DESTROY methods can be called at unpredictable times, it is important that you localise any global variables that the method may update. In particular, localise $@ if you use "eval {}" and localise $? if you use "system" or backticks.

      You can see where I got bit by this a few years ago here Devious Destructor. It would be nice if perl did this for you (but perhaps it can't?) since sometimes it's not obvious that what you are calling is altering globals, and it's not easy to remember to do this.

        So I think the better solution is to extend this warning / practice to FETCH- and STORE-type subs, rather than having code that uses $@ have to be paranoid that $@ might be tied to something that uses eval in its FETCH. Such a FETCH should be considered "broken" from a "best practices" point of view.

        Being paranoid about $@ changing on you is not a bad practice. I'm just saying that in comparison, using eval w/o local($@) in a FETCH is a worse practice. And that is especially true for a FETCH meant to be used with $@.

        - tye        

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlmeditation [id://561895]
Approved by GrandFather
Front-paged by bobf
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others browsing the Monastery: (10)
As of 2014-07-25 19:25 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite superfluous repetitious redundant duplicative phrase is:









    Results (174 votes), past polls