Perl Best Practices
13.14. Unpacking Exceptions
Unpack the exception variable in extended exception handlers. If an exception handler becomes long or complex, you may need to refactor parts of it. For example, consider the X::EOF handler inside TRy_next_line( ) from the "OO Exceptions" guideline: sub try_next_line { # Give get_next_line( ) two chances... for my $already_retried (0..1) { # Return immediately on success, but catch any failure... eval { return get_next_line( ) }; # Rethrow the caught exception if it isn't an EOF problem... croak $EVAL_ERROR if !X::EOF->caught( ); # Also rethrow the caught exception # if we've already tried rewinding the filehandle... croak $EVAL_ERROR if $already_retried; # Otherwise, try rewinding the filehandle... seek $EVAL_ERROR->handle( ), 0, 0; } }
This code would seem to be cleaner and easier to extend if the separate rethrows were refactored like this: sub try_next_line { # Give get_next_line( ) two chances... for my $already_retried (0..1) { # Return immediately on success, but catch any failure... eval { return get_next_line( ) }; # If we can handle this exception... if (X::EOF->caught( ) ) { # Fail on irremedially bad cases... fail_if_incorrigible($EVAL_ERROR, $already_retried); # Otherwise, try rewinding the filehandle... seek $EVAL_ERROR->handle( ), 0, 0; } # Otherwise, let some caller handle it... else { $EVAL_ERROR->rethrow( ); } } }
Refactoring in this way is usually a highly recommended practice, but in this case it has the potential to introduce a subtle bug. The problem is that the $EVAL_ERROR exception variable (a.k.a. $@) is a global variable. So if fail_if_incorrigible( ) happens to throwand then internally catchsome other exception during its execution, that nested exception will overwrite $EVAL_ERROR. So, after the call to fail_if_incorrigible( ), the exception variable might still have the original X::EOF exception, or it might contain some other totally unrelated exception, left over from the internal machinations of fail_if_incorrigible( ). That uncertainty makes the seek statement after the call to fail_if_incorrigible( ) problematic. It tries to seek the handle that was sent back inside the original exception, but there's now no guarantee that $EVAL_ERROR still contains that exception. Fortunately, it's comparatively simple to solve this problem: just copy the exception into a lexical variable before attempting to handle it: if (X::EOF->caught( ) ) { my $exception = $EVAL_ERROR; # Fail on irremedially bad cases... fail_if_incorrigible($exception, $already_retried); # Otherwise, try rewinding the filehandle... seek $exception->handle( ), 0, 0; } Using the Exception::Class module makes this practice even easier to follow, because its version of the caught( ) method always returns a copy of $EVAL_ERROR on success. So you can just write: if (my $exception = X::EOF->caught( ) ) { # Fail on irremedially bad cases... fail_if_incorrigible($exception, $already_retried); # Otherwise, try rewinding the filehandle... seek $exception->fh( ), 0, 0; }
|