Remove local $@; eval { ... } in autodie::exception #6

Closed
miyagawa opened this Issue Nov 3, 2011 · 4 comments

Projects

None yet

3 participants

@miyagawa
miyagawa commented Nov 3, 2011

Try the following code (assuming you have Plack installed :D) and open the localhost:5000 in the browser:

plackup -e 'sub { use autodie; open my $fh, "<:utf8", "/tmp/x/y/z" }'

you'll get a stacktrace of:

Internal autodie::exception error: Don't know how to format mode '<:utf8'. at ...

This is fundamentally because autodie::exception internally localizes $@ and tries _format_open_with_mode() with eval, causing it to overwrite the captured error for StackTrace middleware.

There are probably two separate issues here:

  • Could it be rewritten so that it doesn't use local $@ and eval {}, like just returning undef (return;) instead of throwing errors in the _format_open_with_mode()? Fiddling with exceptions in autodie::exception sounds like a cause of troubles :)
  • The open mode detecter that is being discussed doesn't support PerlIO like this one <:utf8 - it would be nice if we detect that. Or at least you can change the $mode detector to be relaxed like using regexes instead of an exact match - IMO.
@miyagawa
miyagawa commented Nov 3, 2011

Proof of concept patch to remove local $@: http://gist.github.com/1336706

@pjf
Owner
pjf commented Nov 4, 2011

Thank you!

@pjf
Owner
pjf commented Feb 21, 2013

So apparently part of this was fixed ages ago (support for formatting nicer messages for PerlIO layers), but that doesn't actually fix the problem of overwriting the captured error for the StackTrace middleware.

Having poked around wondering why we're seeing what we're seeing, I've noticed that StackTrace will be invoked even if our exception is going to get caught (ie, $^S is true). Is that the intended behaviour? (If so, then at a glance it seems we're generating stacktraces for things which don't need them...)

Paul

@nthykier
Collaborator

Assumed closed due to no follow up. Let us know if the problem still exists and we will reopen the issue.

@nthykier nthykier closed this Sep 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment