Dancer icon indicating copy to clipboard operation
Dancer copied to clipboard

halt() doesn't execute after hooks

Open danschmidt5189 opened this issue 10 years ago • 6 comments

I have a standard on_route_exception hook setup to catch thrown exceptions:

hook on_route_exception => sub {
    my ($exc) = @_;
    error "$exc";
    ... stuff
    halt err $exc->code, $exc->err_id, $exc->msg;
};

The problem is that halt prevents other hooks from being executed, e.g. my after hook, which does some logging that should also be called when the exception is thrown.

Is there a way around this? I'd like to be able to die $exc in my routes, but to also execute hooks that would normally follow a non-halted request.

danschmidt5189 avatar Sep 18 '14 17:09 danschmidt5189

How about calling the 'after' stuff explicitly in the route exception?

sub after_job { ... }

hook after => \&after_job;

hook on_route_exception => sub {
    my ($exc) = @_;
    error "$exc";
    ... stuff
    after_job(); 
    halt err $exc->code, $exc->err_id, $exc->msg;
};

?

yanick avatar Sep 19 '14 00:09 yanick

@yanick after expects a response object, but in on_route_exception we don't have access to the final response object. For example, whatever value you halt on can still be serialized, meaning if we want to log some raw content we must resort to some hacky-ness.

It might help to know that the implementation of err I'm working with doesn't return a Dancer::Response. It uses Dancer::Plugin::Res and essentially just sets the status and returns a hashref.

And hopefully it goes without saying, but I want the final $res in that hook.

danschmidt5189 avatar Sep 19 '14 01:09 danschmidt5189

after expects a response object, but in on_route_exception we don't have access to the final response object.

Well, yeah. If we encounter an exception, we can't really except to have a final response object as at some point the car left the road and plunged down the cliff. :-)

What you can do is to create and forge your own Dancer::Response object. Or look at the current Dancer::Shared->response (iirc) and try to salvage what you can. Or maybe I just don't grasp what you are trying to do with exceptions in your app?

yanick avatar Sep 19 '14 12:09 yanick

My team does stuff like this throughout the codebase:

use App::Exception qw(exc :constants);
# ...
put '/invoices/:id' => sub {
    my $invoice = rset('Invoice')->find(param 'id')
        or die exc 404, NOT_FOUND;
    # ...
};

So really, they're not exactly exceptions - they're business-logic errors that can be rendered as HTTP responses. They can be thrown from routes, but also from methods in our service layer.

Currently we use on_route_exception to transform them into a response and send the response:

hook on_route_exception => sub {
    my ($exc) = @_;
    error "$exc";

    my $content;
    if (blessed $exc and $exc->isa('App::Exception')) {
        $content = {
            code     => $exc->code,
            error_id => $exc->error_id,
            error    => $exc->msg,
        };
    } else {
        $content = {
            code     => 500,
            error_id => INTERNAL_SERVER_ERROR,
            error    => "$exc",
        };
    }

    status $code;
    halt $content;
};

We did this because it wasn't clear how else to customize the format of the error response. (We didn't want to use Dancer's default errors, and we only ever deal with JSON so serialization wasn't a problem.) But this approach causes problems, e.g. the one I mentioned about losing headers.

I noticed that having our Exception class extend Dancer::Continuation::Route and implement return_value solves our problem. It's treated as a normal response, serialized appropriately, and we don't lose headers or hooks. Is this an acceptable way of solving the problem?

danschmidt5189 avatar Nov 01 '14 21:11 danschmidt5189

I'm wondering if there is any hook called on halt? I checked, after is not called, after_error_render is not called also. any suggestion?

Thanks

fayland avatar Nov 18 '14 11:11 fayland

@fayland @danschmidt5189 Hooks are not processed if application was halted: https://github.com/PerlDancer/Dancer/blob/devel/lib/Dancer/Hook.pm#L54

Try to use send_error instead. https://github.com/PerlDancer/Dancer/pull/1234

KES777 avatar Jun 30 '22 13:06 KES777