4

I'm having a hard time trying to figure out a way to handle unexpected errors in my Catalyst::Controller::REST based API.

BEGIN { extends 'Catalyst::Controller::REST' }

__PACKAGE__->config(
    json_options => { relaxed => 1, allow_nonref => 1 },
    default      => 'application/json',
    map          => { 'application/json' => [qw(View JSON)] },
);

sub default : Path : ActionClass('REST') { }

sub default_GET {
    my ( $self, $c, $mid ) = @_;

   ### something happens here and it dies
}

If default_GET dies unexpectedly, the application standard status 500 error page is shown. I'd expect the REST library behind the controller to take control over it and show a JSON error (or whatever serialization response the REST request accepts).

Adding error control (with ie Try::Tiny) action by action is not an option. I'm looking to centralize all error handling. I've tried using an sub end action but that did not work.

sub error :Private {
    my ( $self, $c, $code, $reason ) = @_;

    $reason ||= 'Unknown Error';
    $code ||= 500;

    $c->res->status($code);

    $c->stash->{data} = { error => $reason };
}
ojosilva
  • 1,984
  • 2
  • 15
  • 21

1 Answers1

3

This is not a best practice. It's just how I would do it.

You can use Try::Tiny to catch the errors in your controller, and the helpers that Catalyst::Action::REST brings to send the appropriate response codes. It will take care of converting the response to the right format (i.e. JSON) for you.

But that still requires you to do it for each type of error. Basically it boils down to this:

use Try::Tiny;
BEGIN { extends 'Catalyst::Controller::REST' }

__PACKAGE__->config(
    json_options => { relaxed => 1, allow_nonref => 1 },
    default      => 'application/json',
    map          => { 'application/json' => [qw(View JSON)] },
);

sub default : Path : ActionClass('REST') { }

sub default_GET {
    my ( $self, $c, $mid ) = @_;

    try {
        # ... (there might be a $c->detach in here)
    } catch {
        # this is thrown by $c->detach(), so don't 400 in this case
        return if $_->$_isa('Catalyst::Exception::Detach');

        $self->status_bad_request( $c, message => q{Boom!} );
    }
}

The methods for those kinds of responses are listed in Catalyst::Controller::REST under STATUS HELPERS. They are:

You can implement your own for the missing status1 by subclassing Catalyst::Controller::REST or by adding into its namespace. Refer to one of them for how they are constructed. Here's an example.

*Catalyst::Controller::REST::status_teapot = sub {
    my $self = shift;
    my $c    = shift;
    my %p    = Params::Validate::validate( @_, { message => { type => SCALAR }, }, );

    $c->response->status(418);
    $c->log->debug( "Status I'm A Teapot: " . $p{'message'} ) if $c->debug;
    $self->_set_entity( $c, { error => $p{'message'} } );
    return 1;
}

If that is too tedious because you have a lot of actions, I suggest you make use of the end action like you intended. More to how that works a bit further below.

In that case, don't add the Try::Tiny construct to your actions. Instead, make sure that all of your models or other modules you are using throw good exceptions. Create exception classes for each of the cases, and hand the control of what is supposed to happen in which case to them.

A good way to do all this is to use Catalyst::ControllerRole::CatchErrors. It lets you define a catch_error method that will handle errors for you. In that method, you build a dispatch table that knows what exception should cause which kind of response. Also look at the documentation of $c->error as it has some valuable information here.

package MyApp::Controller::Root;
use Moose;
use Safe::Isa;

BEGIN { extends 'Catalyst::Controller::REST' }
with 'Catalyst::ControllerRole::CatchErrors';

__PACKAGE__->config(
    json_options => { relaxed => 1, allow_nonref => 1 },
    default      => 'application/json',
    map          => { 'application/json' => [qw(View JSON)] },
);

sub default : Path : ActionClass('REST') { }

sub default_GET {
    my ( $self, $c, $mid ) = @_;

    $c->model('Foo')->frobnicate;
}

sub catch_errors : Private {
    my ($self, $c, @errors) = @_;

    # Build a callback for each of the exceptions.
    # This might go as an attribute on $c in MyApp::Catalyst as well.
    my %dispatch = (
        'MyApp::Exception::BadRequest' => sub { 
            $c->status_bad_request(message => $_[0]->message); 
         },
        'MyApp::Exception::Teapot' => sub {
            $c->status_teapot; 
         },
    );

    # @errors is like $c->error
    my $e = shift @errors;

    # this might be a bit more elaborate
    if (ref $e =~ /^MyAPP::Exception/) {
        $dispatch{ref $e}->($e) if exists $dispatch{ref $e};
        $c->detach;
    }

    # if not, rethrow or re-die (simplified)
    die $e;
}

The above is a crude, untested example. It might not work like this exactly, but it's a good start. It would make sense to move the dispatching into an attribute of your main Catalyst application object (the context, $c). Place it in MyApp::Catalyst to do that.

package MyApp::Catalyst;
# ...

has error_dispatch_table => (
    is => 'ro',
    isa => 'HashRef',
    traits => 'Hash',
    handles => {
        can_dispatch_error => 'exists',
        dispatch_error => 'get',
    },
    builder => '_build_error_dispatch_table',
);

sub _build_error_dispatch_table {
    return {
        'MyApp::Exception::BadRequest' => sub { 
            $c->status_bad_request(message => $_[0]->message); 
         },
        'MyApp::Exception::Teapot' => sub { 
            $c->status_teapot; 
         },
    };
}

And then do the dispatching like this:

$c->dispatch_error(ref $e)->($e) if $c->can_dispatch_error(ref $e);

Now all you need is good exceptions. There are different ways to do those. I like Exception::Class or Throwable::Factory.

package MyApp::Model::Foo;
use Moose;
BEGIN { extends 'Catalyst::Model' };

# this would go in its own file for reusability
use Exception::Class (
    'MyApp::Exception::Base',
    'MyApp::Exception::BadRequest' => {
        isa => 'MyApp::Exception::Base',
        description => 'This is a 400',
        fields => [ 'message' ],
    },
    'MyApp::Exception::Teapot' => {
        isa => 'MyApp::Exception::Base',
        description => 'I do not like coffee',
    },
);

sub frobnicate {
    my ($self) = @_;

    MyApp::Exception::Teapot->throw;
}

Again, it would make sense to move the exceptions into their own module so you can reuse them everywhere.

This can be nicely extended I believe. Also keep in mind that coupling the business logic or models too strongly to the fact that it's a web app is be bad design. I chose very speaking exception names because it's easy to explain that way. You might want to just more generic or rather less web centric names, and your dispatch thingy should take of actually mapping them. Otherwise it's too much tied to the web layer.


1) Yes, this is the plural. See here.

Community
  • 1
  • 1
simbabque
  • 53,749
  • 8
  • 73
  • 136
  • 1
    Excellent answer, thanks for being so thorough. I've looked at the `Catalyst::ControllerRole::CatchErrors` source and they use `before 'end' => sub { ... }` which is exactly what I was looking for. The only problem is that the `@error` object comes wrapped into a string such as `Caught exception in App::API::Foo->default ...` probably added by the REST controller class, so exceptions can't be easily dispatched. – ojosilva Sep 15 '16 at 15:50
  • @ojosilva hmm, that is bad. Is there no actual object there? Or did you just `die`? – simbabque Sep 15 '16 at 16:14
  • My model, which exists from way before this new REST api I'm working on, uses `die` profusely. Using `Try::Tiny` action by action is not a good option given the sheer number of actions, but it may be the only way to avoid it having the inner model error wrapped into an outer message string. – ojosilva Sep 15 '16 at 16:43
  • @ojo Catalyst wraps your error message in an object. You might need to build pattern matching into the dispa the thing I suggested. If you don't have your own error objects where you can work with the ref then just work with the messages. – simbabque Sep 15 '16 at 16:47
  • 1
    You're right. If I throw an object it comes out clean. The problem is that the errors being thrown from within these models, and unexpected errors anyway, are all strings. – ojosilva Sep 15 '16 at 17:51
  • @ojo then all you can do is really string match them to determine what to do. That even sounds like it would be a controller's job. If you move it into your callbacks you should be fine. – simbabque Sep 15 '16 at 17:58