[Koha-bugs] [Bug 18206] REST API: Default exception handling

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Thu Mar 16 14:04:23 CET 2017


https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18206

Lari Taskula <lari.taskula at jns.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #61087|0                           |1
        is obsolete|                            |

--- Comment #5 from Lari Taskula <lari.taskula at jns.fi> ---
Created attachment 61170
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61170&action=edit
Bug 18206: Default exception handling for REST API

Many of our operations in REST API controllers now use try-catch blocks to
catch
exceptions and handle them appropriately. This is great, but we should
introduce
a centralized way of handling default HTTP 500 errors. Currently they are
checked
over and over again in each operation. As an example this same lovely poem, in
many cases, is currently replicated for each operation:

sub list {
  ...
  try {
    blabla
  } catch {
    # This should stay here, custom error handling for this particular
operation
    if ($_->isa('Koha::Exceptions::Patron::Something')) {
       return $c->render(status => 400, openapi => { error => $_->error });
    }
    # But the checks below can be centralized!
    elsif ($_->isa('DBIx::Class::Exception')) {
       return $c->render(status => 500, openapi => { error => $_->{msg} });
    }
    elsif ($_->isa('Koha::Exceptions::Exception')) {
       return $c->render(status => 500, openapi => { error => $_->error });
    }
    else {
       return $c->render(status => 500, openapi => { error =>
         "Something went wrong, check the logs." });
    }
  };
}

Instead, my proposal for a more centralized solution is to use a before_render
hook to catch all of the default exceptions before rendering that are expected
to return a 500, logging the error and displaying an appropriate error message
in response body. After this patch, the above example would then look like
this:

sub list {
  ...
  try {
    blabla
  } catch {
    # This should stay here, custom error handling for this particular
operation
    if ($_->isa('Koha::Exceptions::Patron::Something')) {
       return $c->render(status => 400, openapi => { error => $_->error });
    }
    # Simply rethrow the exception with the help of below function - it will
then
    # be handled in before_render hook
    Koha::Exceptions::rethrow_exception($_);
  };
}

What does this patch actually do?

After this patch, in case of an exception, we will normally visit the
catch-block.
If none of the specified Koha::Exceptions match the thrown $_, we will now
rethrow
the exception. This does not crash the whole app, but forwards the exception
eventually into our before_render hook at
Koha::REST::V1::handle_default_exceptions.
There, we are able to customize our way of handling these exceptions. In this
patch
I have added an error logging there. We should also discuss whether we want to
display a detailed error message, or simply "Something went wrong, check the
logs."
for all of the default exceptions. Perhaps this could be controlled by some
sort
of configuration for development/production (e.g. MOJO_MODE) ?

To test:
1. prove t/db_dependent/api
2. prove t/Koha/Exceptions.t

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list