Plack::Middleware::StackTrace and eval

[Abelard]

Hi.

I have an app that catches exceptions and appends data to it. In development, I'd like to use Plack::Middleware::StackTrace to display the trace. However, since I'm rethrowing a modified error message, it isn't finding the Devel::StraceTrack that was built.

For example, I'm doing something along these lines:

eval { dispatch() };

if (my $error = $@) {

  $error .= " lots of details about system state";

  die $error;

}

From my understanding of P::M::ST, the $SIG{DIE} handler is being trigged when the exception is raised in the eval. It's building the stack trace off of that, which is what I want.

Then I go fiddling with the error and re-raise it. Since the error has been changed it doesn't match anything in %string_traces, resulting in the real backtrace being lost.

Is there a way around that?

I found Plack::Middleware::RethrowFriendly, but I'm not sure if it's intended to handle this case. I tried it and it didn't seem to show the backtrace from within the eval.

I was thinking that if I can modify $@ before the $SIG{DIE} handler gets it, maybe that would work, but I'm not sure how to do that.

Thanks -A

[Ævar Arnfjörð Bjarmason]

This is not Plack specific, but in general in Perl:

my $stack;
local $SIG{__DIE__} = sub { $stack = longmess() };
eval { code(); 1 }
or do {
my $error = $@ || "Zombie Error";
say "You had an error with stack $stack";
};

You probably want to chop off the stack bits that land you in the
__DIE__ handler itself, and this gets quite tricky in some situations.

[Abelard]

On Friday, September 9, 2016 at 12:25:15 AM UTC-7, Ævar Arnfjörð Bjarmason wrote:

This is not Plack specific, but in general in Perl:

my $stack;
local $SIG{__DIE__} = sub { $stack = longmess() };
eval { code(); 1 }
or do {
    my $error = $@ || "Zombie Error";
    say "You had an error with stack $stack";
};

You probably want to chop off the stack bits that land you in the
__DIE__ handler itself, and this gets quite tricky in some situations.

Thanks, I have tried various forms of that. The problem is getting Plack::Middleware::StackTrace to *use* that stack info in it's formatting.

When I rethrow the exception, it displays a big wall of text (the longmess), followed by a beautifully formatted stack trace of the call stack where the error was rethrown. 

I think I need a way to modify the exception that Plack::Middleware::StackTrace sees in its DIE handler before it receives it...

I suppose maybe I could add another middleware that appends data to the response. Egh.

[Aristotle Pagaltzis]

* Abelard <abelard...@gmail.com> [2016-09-09 17:48]:

> I think I need a way to modify the exception that
> Plack::Middleware::StackTrace sees in its DIE handler before it receives
> it...

Are you looking for something like this?

my $old_die_handler = $SIG{__DIE__};

local $SIG{__DIE__} = sub {

$_[0] .= " lots of details about system state";
ref $old_die_handler ? goto &$old_die_handler : die @_;
};
dispatch(); # no eval{}

Regards,
--
Aristotle Pagaltzis // <http://plasmasturm.org/>

[Abelard]

Yes! Exactly like that. Except for this:

  Modification of a read-only value attempted at ./try line 13.

 

Is there a way around that? Here's my test code on perl 18.2:

#!/usr/bin/env perl

use strict;

use warnings;

local $SIG{__DIE__} = sub {

  die "FOO! $_[0]";

};

my $old_die_handler = $SIG{__DIE__};

local $SIG{__DIE__} = sub {

  $_[0] .= " lots of details about system state";

  ref $old_die_handler ? goto &$old_die_handler : die @_;

};

die "in dispatch";

Thanks.

[Abelard]

Here's a patch to Plack::Middleware::StackTrace that seems to work, but not nearly as elegant as Aristotle's solution.

--- StackTrace.pm.orig 2016-09-09 00:25:40.552397674 -0700

+++ StackTrace.pm 2016-09-09 23:21:17.084080486 -0700

@@ -20,7 +20,9 @@

 

     my ($trace, %string_traces, %ref_traces);

     local $SIG{__DIE__} = sub {

-        $trace = $StackTraceClass->new(

+        undef $trace unless $env->{'plack.stacktrace.rethrow'};

+

+        $trace ||= $StackTraceClass->new(

             indent => 1, message => munge_error($_[0], [ caller ]),

             ignore_package => __PACKAGE__, no_refs => 1,

         );

So with that, if you set $env->{'plack.stacktrace.rethrow'} to true, it reuses the previously built trace.

I'm not sure how safe that is for the general case, but it seems to work for me. It's obviously intended

more for use by a framework than an end-user.

So the idiom would be something like:

eval { dispatch() };

if ($@) {

  $env->{'plack.stacktrace.rethrow'} = 1;

  die "now with more suds! $@";

}

Thoughts?

[Ævar Arnfjörð Bjarmason]

On Sat, Sep 10, 2016 at 8:26 AM, Abelard <abelard...@gmail.com> wrote:

> So the idiom would be something like:
>
> eval { dispatch() };
> if ($@) {
> $env->{'plack.stacktrace.rethrow'} = 1;
> die "now with more suds! $@";
> }
>
> Thoughts?

Using that eval idiom is a logic error. The $@ is only significant if
the eval itself fails, so do:

eval {
dispatch();

1;
} or do {
my $error = $@ || "Zombie Error";

....;
};

It's an error because:

1. The app could do an eval {} somewhere which'll set $@ but not die,
now you die because you check if $@ is set without *your* eval
failing.

2. You can die but $@ can get lost due to various past/current
bugs/misfeatures of perl, hence the "Zombie Error";.

[Aristotle Pagaltzis]

Hi Abelard,

* Abelard <abelard...@gmail.com> [2016-09-10 08:12]:

> On Friday, September 9, 2016 at 9:44:51 PM UTC-7, Aristotle Pagaltzis wrote:
> > Are you looking for something like this?
> >
> > my $old_die_handler = $SIG{__DIE__};
> > local $SIG{__DIE__} = sub {
> > $_[0] .= " lots of details about system state";
> > ref $old_die_handler ? goto &$old_die_handler : die @_;
> > };
> > dispatch(); # no eval{}
>

> Yes! Exactly like that. Except for this:
>
> Modification of a read-only value attempted at ./try line 13.
>
> Is there a way around that?

Err, of course? Presumably something like this:

{

my $old_die_handler = $SIG{__DIE__};
local $SIG{__DIE__} = sub {

my $msg = shift;
unshift @_, $msg . " lots of details about system state";

ref $old_die_handler ? goto &$old_die_handler : die @_;
};
dispatch();
}

(Just as untested as my previous scribble.) (E.g. this time I remembered
to add a block to limit the scope of the `local`.)


* Abelard <abelard...@gmail.com> [2016-09-10 08:36]:

> Here's a patch to Plack::Middleware::StackTrace that seems to work,

You don’t want that.

The __DIE__ handler gets called for any and all exceptions. During
unwind there can be other caught-and-handled exceptions after the
__DIE__ handler is called but before the eval{} returns, and the __DIE__
handler will be called for all of those too. If you only keep the stack
trace for the last thrown exception, that won’t necessarily match the
exception caught by the eval{} block.

That’s why the %string_traces/%ref_traces stuff was added in the first
place:

https://github.com/plack/Plack/issues/475
https://github.com/plack/Plack/pull/476

Yes, it keeps you from rethrowing your exception easily. It has to, in
order to allow other code to do its own exception handling in peace,
without getting mixed up with yours. A mechanism to disable that is not
“supporting rethrowing” as the option name in your patch suggests, it’s
“reintroducing an old bug”.

The only way you can avoid mismatches is to modify the exception you
care about before it gets into P:M::StackTrace’s hands.

That’s why I proposed what I proposed.

[Abelard]

On Saturday, September 10, 2016 at 12:40:47 PM UTC-7, Aristotle Pagaltzis wrote:

* Abelard <abelard...@gmail.com> [2016-09-10 08:12]:
> On Friday, September 9, 2016 at 9:44:51 PM UTC-7, Aristotle Pagaltzis wrote:
> > Are you looking for something like this?
> >
> >     my $old_die_handler = $SIG{__DIE__};
> >     local $SIG{__DIE__} = sub {
> >         $_[0] .= " lots of details about system state";
> >         ref $old_die_handler ? goto &$old_die_handler : die @_;
> >     };
> >     dispatch();  # no eval{}
>
> Yes! Exactly like that. Except for this:
>
>   Modification of a read-only value attempted at ./try line 13.
>
> Is there a way around that?

Err, of course? Presumably something like this:

    {
        my $old_die_handler = $SIG{__DIE__};
        local $SIG{__DIE__} = sub {
            my $msg = shift;
            unshift @_, $msg . " lots of details about system state";
            ref $old_die_handler ? goto &$old_die_handler : die @_;
        };
        dispatch();
    }

(Just as untested as my previous scribble.) (E.g. this time I remembered
to add a block to limit the scope of the `local`.)

I'm seeing an infinite loop with that.  Here's a self-contained test, run as

"plackup -MPlack::Middleware::StackTrace ./hello.psgi"

use strict;

use warnings;

my $app = sub {

  my $old_die_handler = $SIG{__DIE__};

  local $SIG{__DIE__} = sub {

    # warn "my die called from ", caller, "\n";

    my $msg = shift;

    unshift @_, $msg . " lots of details about system state";

    ref $old_die_handler ? goto &$old_die_handler : die @_;

  };

  die "goodbye world!";

  # return [ 200, [ 'Content-Type' => 'text/plain' ], [ 'Hello World' ] ];

};

With the new DIE handler commented out, it works. Once I add that,

I get "deep recursion" error and out of memory failure.

Adding in the "warn" there, I see it's getting called repeatedly by the

StackTrace middleware.

Does a goto from a DIE handler prevent the disabling of handler.  perlvar says

The "__DIE__" handler is explicitly disabled during the call, so that you can die
from a "__DIE__" handler.

but I don't understand why it's continuing calling my die routine then... 

 

* Abelard <abelard...@gmail.com> [2016-09-10 08:36]:
> Here's a patch to Plack::Middleware::StackTrace that seems to work,

You don’t want that.

The __DIE__ handler gets called for any and all exceptions. During
unwind there can be other caught-and-handled exceptions after the
__DIE__ handler is called but before the eval{} returns, and the __DIE__
handler will be called for all of those too. If you only keep the stack
trace for the last thrown exception, that won’t necessarily match the
exception caught by the eval{} block.

Yes, I saw that. I was thinking as long as you don't do anything between the eval and the reraise, that you'd get the right trace, but I guess not. My first approach was to provide aliases (e.g., new_error => old_error) so that StackTrace could follow the chain, but it seemed so complicated.

Obviously I'd rather just get your code working :)

Thanks.

[Abelard]

This seems to work. 

use strict;

use warnings;

my $app = sub {

  my $old_die_handler = $SIG{__DIE__};

  local $SIG{__DIE__} = sub {

    # warn "my die called from ", caller, "\n";

    my $msg = shift;

    unshift @_, $msg . " lots of details about system state";

    die @_ unless $old_die_handler;

    $SIG{__DIE__} = $old_die_handler;

    goto &$old_die_handler;

  };

  die "goodbye world!";

  # return [ 200, [ 'Content-Type' => 'text/plain' ], [ 'Hello World' ] ];

};

The difference being that before we call the original die handler, we reassign SIG{__DIE__} back to it, which I guess re-enables the recursive call protection.

Does that seem okay to do?

[Aristotle Pagaltzis]

* Abelard <abelard...@gmail.com> [2016-09-12 08:36]:

> The difference being that before we call the original die handler,
> we reassign SIG{__DIE__} back to it, which I guess re-enables the
> recursive call protection.

Ah, makes sense. But there’s no mechanism that restores your own die
handler, is there? It seems to me this this will only ever modify one
exception – if it happens to be the one you wanted, great, otherwise,
not so great.

The only solution I could think of is a bit crazy: store a scope guard
in @_ which restores your die handler on DESTROY.

A less crazy solution might be here:
https://github.com/moose/Moo/blob/master/t/demolish-throw.t
Looks like the die handler implements the recursion check manually.

Another option is to not use the `goto` and just call $old_die_handler
normally. That works reliably as long you *know* that $old_die_handler
will call `die` (which happens to be the case here) instead of trying
to abandon the exception using `goto`.

If it does try to abandon the exception, you must call it with `goto`
also, otherwise it will not succeed. But as long as it never tries to
`die`, that’s fine, so that works too.

So I suppose this is solvable as long as you expect a particular die
handler in %SIG and pick the matching approach. But I’m not sure one
can chain die handlers generically.

[Abelard]

On Monday, September 12, 2016 at 5:40:35 AM UTC-7, Aristotle Pagaltzis wrote:

* Abelard <abelard...@gmail.com> [2016-09-12 08:36]:
> The difference being that before we call the original die handler,
> we reassign SIG{__DIE__} back to it, which I guess re-enables the
> recursive call protection.

Ah, makes sense. But there’s no mechanism that restores your own die
handler, is there? It seems to me this this will only ever modify one
exception – if it happens to be the one you wanted, great, otherwise,
not so great.

Yes, you're right. I was able to trigger that problem.

The only solution I could think of is a bit crazy: store a scope guard
in @_ which restores your die handler on DESTROY.

A less crazy solution might be here:
https://github.com/moose/Moo/blob/master/t/demolish-throw.t
Looks like the die handler implements the recursion check manually.

Another option is to not use the `goto` and just call $old_die_handler
normally. That works reliably as long you *know* that $old_die_handler
will call `die` (which happens to be the case here) instead of trying
to abandon the exception using `goto`.

If it does try to abandon the exception, you must call it with `goto`
also, otherwise it will not succeed. But as long as it never tries to
`die`, that’s fine, so that works too.

Calling $old_die_handler directly does seem to work great, thanks.

What's the issue if $old_die_handler uses goto instead of die? This is a worry about the inifinite loop again, or something else?

I was trying to identify the problem with this code but it seems to be working okay:

use strict;

use warnings;

sub _old_die2 {

  die "in old die2: $_[0]";

}

my $app = sub {

  # override global DIE handler

  $SIG{__DIE__} = sub { goto &_old_die2 };

  my $old_die_handler = $SIG{__DIE__};

  local $SIG{__DIE__} = sub {

    my $msg = shift;

    unshift @_, $msg . " lots of details about system state";

    die @_ unless $old_die_handler;

    $old_die_handler->(@_);

  };

  eval { die "i'm not dead yet" };

  die "goodbye world!";

  return [ 200, [ 'Content-Type' => 'text/plain' ], [ 'Hello World' ] ];

};

I get:

The application raised the following error:

  in old die2: goodbye world! at /home/a/tmp/hello.psgi line 24.
 lots of details about system state at /home/a/tmp/hello.psgi line 5.

[Aristotle Pagaltzis]

* Abelard <abelard...@gmail.com> [2016-09-12 21:12]:

> What's the issue if $old_die_handler uses goto instead of die? This is
> a worry about the inifinite loop again, or something else?

There is this bit in perlvar:

When a "__DIE__" hook routine returns, the exception processing
continues as it would have in the absence of the hook, unless the
hook routine itself exits via a "goto &sub", a loop exit, or
a "die()".

So if the old die handler uses goto, it abandons the exception, but if
you call it regularly from within another die handler, then the old die
handler is Just Another Function, and when it does its goto, it doesn’t
mean anything special.

So in that case, your die handler has to do the goto which abandons the
exception; after which it doesn’t matter what the old die handler does.

At least, that’s my understanding of the documentation. In testing I’ve
not succeeded in making an exception go away, so far.

> I was trying to identify the problem with this code but it seems to be
> working okay:

Yes, that code should be OK. Because _old_die2 dies, you can just call
it instead of using goto, which allows the recursion check to find your
die handler on the stack, so it won’t try to call you again, and all is
well in the world.

So if you know the old handler always dies, you just call it normally.
And if you know it always does goto, you use goto.

That’s good enough for you case.

The remaining question is how to chain die handlers when you *don’t*
know what it will do, *or*, when the old die handler *sometimes* dies
but other times uses goto. You would need to both remove your stack
frame (so the handler can use goto) and leave it (so the handler can
die) at the same time – which is a contradiction. (Again, based on my
understanding; I have not been able to confirm this in testing.)

I don’t see a good answer for that, only the ones I mentioned (scope
guard in @_ to change $SIG{__DIE__}, or a manual recursion check).

But you don’t have that situation anyway, so you’re fine.