From a87649298ba1960887a3b92e53f67f5e70a779f7 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 2 Apr 2020 14:43:27 -0300 Subject: [PATCH] Bug 25032: Make existing controllers use unhandled_exception This simple patch removes 'just in case' handling of specific exceptions and makes the current routes controllers use the unhandled_exception helper. Most possible exceptions are already catch by our tools (Koha::Object, etc) and the existing code is not looking for known possible exceptions but has just been copied and pasted since our beginings. Anytime a situation in which an unhandled exception is caught, we (the devs) should report it and specific exception handling discussed and solved. But this has just been useless scaffolding so far. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => SUCCESS: Tests pass 2. Apply this patch 3. Repeat 1. => SUCCESS: Tests still pass 4. Sign off :-D Signed-off-by: Jonathan Druart --- Koha/Exceptions/ClubHold.pm | 2 +- Koha/REST/V1/Acquisitions/Funds.pm | 9 +-- Koha/REST/V1/Acquisitions/Orders.pm | 73 ++++------------- Koha/REST/V1/Acquisitions/Vendors.pm | 81 ++++++++----------- Koha/REST/V1/Biblios.pm | 18 +---- Koha/REST/V1/Checkouts.pm | 109 ++++++++++++++------------ Koha/REST/V1/Cities.pm | 57 ++++---------- Koha/REST/V1/Clubs/Holds.pm | 26 ++---- Koha/REST/V1/Holds.pm | 113 ++++++++++----------------- Koha/REST/V1/Items.pm | 36 +++------ Koha/REST/V1/Libraries.pm | 76 +++++------------- Koha/REST/V1/Patrons.pm | 67 ++++------------ Koha/REST/V1/Patrons/Account.pm | 59 ++++++-------- Koha/REST/V1/Patrons/Password.pm | 25 +++--- Koha/REST/V1/ReturnClaims.pm | 47 ++--------- 15 files changed, 267 insertions(+), 531 deletions(-) diff --git a/Koha/Exceptions/ClubHold.pm b/Koha/Exceptions/ClubHold.pm index 429f89c0bd..e787c27908 100644 --- a/Koha/Exceptions/ClubHold.pm +++ b/Koha/Exceptions/ClubHold.pm @@ -12,4 +12,4 @@ use Exception::Class ( }, ); -1; \ No newline at end of file +1; diff --git a/Koha/REST/V1/Acquisitions/Funds.pm b/Koha/REST/V1/Acquisitions/Funds.pm index 533d3e23b2..ee0704674a 100644 --- a/Koha/REST/V1/Acquisitions/Funds.pm +++ b/Koha/REST/V1/Acquisitions/Funds.pm @@ -51,14 +51,7 @@ sub list { ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs. $_" } ); - } + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Acquisitions/Orders.pm b/Koha/REST/V1/Acquisitions/Orders.pm index 989cb6b2e6..a9ed5123f9 100644 --- a/Koha/REST/V1/Acquisitions/Orders.pm +++ b/Koha/REST/V1/Acquisitions/Orders.pm @@ -52,18 +52,7 @@ sub list { ); } catch { - if ( $_->isa('Koha::Exceptions::Exception') ) { - return $c->render( - status => 500, - openapi => { error => "Unhandled exception ($_)" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs. ($_)" } - ); - } + $c->unhandled_exception($_); }; } @@ -85,12 +74,17 @@ sub get { ); } - my $embed = $c->stash('koha.embed'); + return try { + my $embed = $c->stash('koha.embed'); - return $c->render( - status => 200, - openapi => $order->to_api({ embed => $embed }) - ); + return $c->render( + status => 200, + openapi => $order->to_api({ embed => $embed }) + ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 add @@ -116,27 +110,14 @@ sub add { ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 500, - openapi => { - error => - "Something went wrong, check Koha logs for details." - } - ); - } - if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) { + if ( blessed $_ and $_->isa('Koha::Exceptions::Object::DuplicateID') ) { return $c->render( status => 409, openapi => { error => $_->error, conflict => $_->duplicate_id } ); } - else { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); - } + + $c->unhandled_exception($_); }; } @@ -168,18 +149,7 @@ sub update { ); } catch { - if ( $_->isa('Koha::Exceptions::Exception') ) { - return $c->render( - status => 500, - openapi => { error => "Unhandled exception ($_)" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } @@ -211,18 +181,7 @@ sub delete { ); } catch { - if ( $_->isa('Koha::Exceptions::Exception') ) { - return $c->render( - status => 500, - openapi => { error => "Unhandled exception ($_)" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Acquisitions/Vendors.pm b/Koha/REST/V1/Acquisitions/Vendors.pm index 974491b568..8fcf715cfb 100644 --- a/Koha/REST/V1/Acquisitions/Vendors.pm +++ b/Koha/REST/V1/Acquisitions/Vendors.pm @@ -49,14 +49,7 @@ sub list { ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs." } ); - } + $c->unhandled_exception($_); }; } @@ -75,10 +68,15 @@ sub get { openapi => { error => "Vendor not found" } ); } - return $c->render( - status => 200, - openapi => $vendor->to_api - ); + return try { + return $c->render( + status => 200, + openapi => $vendor->to_api + ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 add @@ -101,14 +99,7 @@ sub add { ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->msg } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs." } ); - } + $c->unhandled_exception($_); }; } @@ -134,17 +125,14 @@ sub update { } catch { if ( not defined $vendor ) { - return $c->render( status => 404, - openapi => { error => "Object not found" } ); - } - elsif ( $_->isa('Koha::Exceptions::Object') ) { - return $c->render( status => 500, - openapi => { error => $_->message } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs." } ); + return $c->render( + status => 404, + openapi => { error => "Object not found" } + ); } + + $c->unhandled_exception($_); + }; } @@ -158,29 +146,26 @@ Controller function that handles deleting a Koha::Acquisition::Bookseller object sub delete { my $c = shift->openapi->valid_input or return; - my $vendor; - return try { - $vendor = Koha::Acquisition::Booksellers->find( $c->validation->param('vendor_id') ); + my $vendor = Koha::Acquisition::Booksellers->find( $c->validation->param('vendor_id') ); + + unless ( $vendor ) { + return $c->render( + status => 404, + openapi => { error => "Object not found" } + ); + } + $vendor->delete; - return $c->render( status => 200, - openapi => q{} ); + + return $c->render( + status => 200, + openapi => q{} + ); } catch { - if ( not defined $vendor ) { - return $c->render( status => 404, - openapi => { error => "Object not found" } ); - } - elsif ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->msg } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs." } ); - } + $c->unhandled_exception($_); }; - } 1; diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index af13f283bd..13f55bacf3 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -94,10 +94,7 @@ sub get { } } catch { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs ($_)" } - ); + $c->unhandled_exception($_); }; } @@ -133,18 +130,7 @@ sub delete { } } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( - status => 500, - openapi => { error => $_->{msg} } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index 3a5f448cc6..9d5e5b42a1 100644 --- a/Koha/REST/V1/Checkouts.pm +++ b/Koha/REST/V1/Checkouts.pm @@ -100,17 +100,7 @@ sub list { return $c->render( status => 200, openapi => $checkouts->to_api ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( - status => 500, - openapi => { error => $_->{msg} } - ); - } else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } @@ -135,10 +125,15 @@ sub get { ); } - return $c->render( - status => 200, - openapi => $checkout->to_api - ); + return try { + return $c->render( + status => 200, + openapi => $checkout->to_api + ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 renew @@ -160,27 +155,32 @@ sub renew { ); } - my $borrowernumber = $checkout->borrowernumber; - my $itemnumber = $checkout->itemnumber; + return try { + my $borrowernumber = $checkout->borrowernumber; + my $itemnumber = $checkout->itemnumber; + + my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( + $borrowernumber, $itemnumber); + + if (!$can_renew) { + return $c->render( + status => 403, + openapi => { error => "Renewal not authorized ($error)" } + ); + } - my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( - $borrowernumber, $itemnumber); + AddRenewal($borrowernumber, $itemnumber, $checkout->branchcode); + $checkout = Koha::Checkouts->find($checkout_id); - if (!$can_renew) { + $c->res->headers->location( $c->req->url->to_string ); return $c->render( - status => 403, - openapi => { error => "Renewal not authorized ($error)" } + status => 201, + openapi => $checkout->to_api ); } - - AddRenewal($borrowernumber, $itemnumber, $checkout->branchcode); - $checkout = Koha::Checkouts->find($checkout_id); - - $c->res->headers->location( $c->req->url->to_string ); - return $c->render( - status => 201, - openapi => $checkout->to_api - ); + catch { + $c->unhandled_exception($_); + }; } =head3 allows_renewal @@ -202,29 +202,34 @@ sub allows_renewal { ); } - my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( - $checkout->borrowernumber, $checkout->itemnumber); + return try { + my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( + $checkout->borrowernumber, $checkout->itemnumber); - my $renewable = Mojo::JSON->false; - $renewable = Mojo::JSON->true if $can_renew; + my $renewable = Mojo::JSON->false; + $renewable = Mojo::JSON->true if $can_renew; - my $rule = Koha::CirculationRules->get_effective_rule( - { - categorycode => $checkout->patron->categorycode, - itemtype => $checkout->item->effective_itemtype, - branchcode => $checkout->branchcode, - rule_name => 'renewalsallowed', - } - ); - return $c->render( - status => 200, - openapi => { - allows_renewal => $renewable, - max_renewals => $rule->rule_value, - current_renewals => $checkout->renewals, - error => $error - } - ); + my $rule = Koha::CirculationRules->get_effective_rule( + { + categorycode => $checkout->patron->categorycode, + itemtype => $checkout->item->effective_itemtype, + branchcode => $checkout->branchcode, + rule_name => 'renewalsallowed', + } + ); + return $c->render( + status => 200, + openapi => { + allows_renewal => $renewable, + max_renewals => $rule->rule_value, + current_renewals => $checkout->renewals, + error => $error + } + ); + } + catch { + $c->unhandled_exception($_); + }; } 1; diff --git a/Koha/REST/V1/Cities.pm b/Koha/REST/V1/Cities.pm index 4c49e1a346..6b76daae10 100644 --- a/Koha/REST/V1/Cities.pm +++ b/Koha/REST/V1/Cities.pm @@ -40,14 +40,7 @@ sub list { return $c->render( status => 200, openapi => $cities ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs."} ); - } + $c->unhandled_exception($_); }; } @@ -59,13 +52,18 @@ sub list { sub get { my $c = shift->openapi->valid_input or return; - my $city = Koha::Cities->find( $c->validation->param('city_id') ); - unless ($city) { - return $c->render( status => 404, - openapi => { error => "City not found" } ); - } + return try { + my $city = Koha::Cities->find( $c->validation->param('city_id') ); + unless ($city) { + return $c->render( status => 404, + openapi => { error => "City not found" } ); + } - return $c->render( status => 200, openapi => $city->to_api ); + return $c->render( status => 200, openapi => $city->to_api ); + } + catch { + $c->unhandled_exception($_); + } } =head3 add @@ -85,18 +83,7 @@ sub add { ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( - status => 500, - openapi => { error => $_->{msg} } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } @@ -120,14 +107,7 @@ sub update { return $c->render( status => 200, openapi => $city->to_api ); } catch { - if ( $_->isa('Koha::Exceptions::Object') ) { - return $c->render( status => 500, - openapi => { error => $_->message } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs."} ); - } + $c->unhandled_exception($_); }; } @@ -149,14 +129,7 @@ sub delete { return $c->render( status => 200, openapi => "" ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs."} ); - } + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Clubs/Holds.pm b/Koha/REST/V1/Clubs/Holds.pm index 5851e15096..6acaced0c3 100644 --- a/Koha/REST/V1/Clubs/Holds.pm +++ b/Koha/REST/V1/Clubs/Holds.pm @@ -125,7 +125,7 @@ sub add { ); } catch { - if ( blessed $_ and $_->isa('Koha::Exceptions::Object') ) { + if ( blessed $_ ) { if ( $_->isa('Koha::Exceptions::Object::FKConstraint') ) { my $broken_fk = $_->broken_fk; @@ -135,32 +135,16 @@ sub add { openapi => $Koha::REST::V1::Clubs::Holds::to_api_mapping->{$broken_fk} . ' not found.' ); } - else { - return $c->render( - status => 500, - openapi => { error => "Uncaught exception: $_" } - ); - } } - else { + elsif ($_->isa('Koha::Exceptions::ClubHold')) { return $c->render( status => 500, - openapi => { error => "$_" } + openapi => { error => $_->description } ); } } - elsif (blessed $_ and $_->isa('Koha::Exceptions::ClubHold')) { - return $c->render( - status => 500, - openapi => { error => $_->description } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong. check the logs." } - ); - } + + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index a806d6757a..2a8c055d3f 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -48,18 +48,7 @@ sub list { return $c->render( status => 200, openapi => $holds ); } catch { - if ( blessed $_ && $_->isa('Koha::Exceptions') ) { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check Koha logs for details." } - ); - } + $c->unhandled_exception($_); }; } @@ -201,26 +190,10 @@ sub add { openapi => Koha::Holds->new->to_api_mapping->{$broken_fk} . ' not found.' ); } - else { - return $c->render( - status => 500, - openapi => { error => "Uncaught exception: $_" } - ); - } - } - else { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); } } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong. check the logs." } - ); - } + + $c->unhandled_exception($_); }; } @@ -233,36 +206,41 @@ Method that handles modifying a Koha::Hold object sub edit { my $c = shift->openapi->valid_input or return; - my $hold_id = $c->validation->param('hold_id'); - my $hold = Koha::Holds->find( $hold_id ); + return try { + my $hold_id = $c->validation->param('hold_id'); + my $hold = Koha::Holds->find( $hold_id ); - unless ($hold) { - return $c->render( status => 404, - openapi => {error => "Hold not found"} ); - } + unless ($hold) { + return $c->render( status => 404, + openapi => {error => "Hold not found"} ); + } - my $body = $c->req->json; + my $body = $c->req->json; - my $pickup_library_id = $body->{pickup_library_id} // $hold->branchcode; - my $priority = $body->{priority} // $hold->priority; - # suspended_until can also be set to undef - my $suspended_until = exists $body->{suspended_until} ? $body->{suspended_until} : $hold->suspend_until; + my $pickup_library_id = $body->{pickup_library_id} // $hold->branchcode; + my $priority = $body->{priority} // $hold->priority; + # suspended_until can also be set to undef + my $suspended_until = exists $body->{suspended_until} ? $body->{suspended_until} : $hold->suspend_until; - my $params = { - reserve_id => $hold_id, - branchcode => $pickup_library_id, - rank => $priority, - suspend_until => $suspended_until ? output_pref(dt_from_string($suspended_until, 'rfc3339')) : '', - itemnumber => $hold->itemnumber - }; + my $params = { + reserve_id => $hold_id, + branchcode => $pickup_library_id, + rank => $priority, + suspend_until => $suspended_until ? output_pref(dt_from_string($suspended_until, 'rfc3339')) : '', + itemnumber => $hold->itemnumber + }; - C4::Reserves::ModReserve($params); - $hold->discard_changes; # refresh + C4::Reserves::ModReserve($params); + $hold->discard_changes; # refresh - return $c->render( - status => 200, - openapi => $hold->to_api - ); + return $c->render( + status => 200, + openapi => $hold->to_api + ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 delete @@ -281,9 +259,14 @@ sub delete { return $c->render( status => 404, openapi => { error => "Hold not found." } ); } - $hold->cancel; + return try { + $hold->cancel; - return $c->render( status => 200, openapi => {} ); + return $c->render( status => 200, openapi => {} ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 suspend @@ -329,12 +312,8 @@ sub suspend { if ( blessed $_ and $_->isa('Koha::Exceptions::Hold::CannotSuspendFound') ) { return $c->render( status => 400, openapi => { error => "$_" } ); } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong. check the logs." } - ); - } + + $c->unhandled_exception($_); }; } @@ -360,10 +339,7 @@ sub resume { return $c->render( status => 204, openapi => {} ); } catch { - return $c->render( - status => 500, - openapi => { error => "Something went wrong. check the logs." } - ); + $c->unhandled_exception($_); }; } @@ -398,10 +374,7 @@ sub update_priority { return $c->render( status => 200, openapi => $priority ); } catch { - return $c->render( - status => 500, - openapi => { error => "Something went wrong. check the logs." } - ); + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Items.pm b/Koha/REST/V1/Items.pm index 3213fad35c..4781024b36 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -51,19 +51,7 @@ sub list { ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 500, - openapi => { - error => - "Something went wrong, check Koha logs for details." - } - ); - } - return $c->render( - status => 500, - openapi => { error => "$_" } - ); + $c->unhandled_exception($_); }; } @@ -77,24 +65,18 @@ Controller function that handles retrieving a single Koha::Item sub get { my $c = shift->openapi->valid_input or return; - my $item; try { - $item = Koha::Items->find($c->validation->param('item_id')); + my $item = Koha::Items->find($c->validation->param('item_id')); + unless ( $item ) { + return $c->render( + status => 404, + openapi => { error => 'Item not found'} + ); + } return $c->render( status => 200, openapi => $item->to_api ); } catch { - unless ( defined $item ) { - return $c->render( status => 404, - openapi => { error => 'Item not found'} ); - } - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs."} ); - } + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Libraries.pm b/Koha/REST/V1/Libraries.pm index c63b823b0c..9025adbe7c 100644 --- a/Koha/REST/V1/Libraries.pm +++ b/Koha/REST/V1/Libraries.pm @@ -49,16 +49,7 @@ sub list { return $c->render( status => 200, openapi => $libraries ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check Koha logs for details." } - ); - } - return $c->render( - status => 500, - openapi => { error => "$_" } - ); + $c->unhandled_exception($_); }; } @@ -71,18 +62,23 @@ Controller function that handles retrieving a single Koha::Library sub get { my $c = shift->openapi->valid_input or return; - my $library_id = $c->validation->param('library_id'); - my $library = Koha::Libraries->find( $library_id ); + return try { + my $library_id = $c->validation->param('library_id'); + my $library = Koha::Libraries->find( $library_id ); - unless ($library) { - return $c->render( status => 404, - openapi => { error => "Library not found" } ); - } + unless ($library) { + return $c->render( status => 404, + openapi => { error => "Library not found" } ); + } - return $c->render( - status => 200, - openapi => $library->to_api - ); + return $c->render( + status => 200, + openapi => $library->to_api + ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 add @@ -105,24 +101,14 @@ sub add { ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check Koha logs for details." } - ); - } - if ( $_->isa('Koha::Exceptions::Object::DuplicateID') ) { + if ( blessed $_ && $_->isa('Koha::Exceptions::Object::DuplicateID') ) { return $c->render( status => 409, openapi => { error => $_->error, conflict => $_->duplicate_id } ); } - else { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); - } + + $c->unhandled_exception($_); }; } @@ -154,17 +140,7 @@ sub update { ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check Koha logs for details." } - ); - } - - return $c->render( - status => 500, - openapi => { error => "$_" } - ); + $c->unhandled_exception($_); }; } @@ -189,17 +165,7 @@ sub delete { return $c->render( status => 204, openapi => ''); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check Koha logs for details." } - ); - } - - return $c->render( - status => 500, - openapi => { error => "$_" } - ); + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index 8350f85312..8ff645c46e 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -94,18 +94,7 @@ sub list { return $c->render( status => 200, openapi => $patrons->to_api ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( - status => 500, - openapi => { error => $_->{msg} } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } @@ -119,14 +108,19 @@ Controller function that handles retrieving a single Koha::Patron object sub get { my $c = shift->openapi->valid_input or return; - my $patron_id = $c->validation->param('patron_id'); - my $patron = Koha::Patrons->find($patron_id); + return try { + my $patron_id = $c->validation->param('patron_id'); + my $patron = Koha::Patrons->find($patron_id); - unless ($patron) { - return $c->render( status => 404, openapi => { error => "Patron not found." } ); - } + unless ($patron) { + return $c->render( status => 404, openapi => { error => "Patron not found." } ); + } - return $c->render( status => 200, openapi => $patron->to_api ); + return $c->render( status => 200, openapi => $patron->to_api ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 add @@ -185,10 +179,7 @@ sub add { ); } else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check Koha logs for details." } - ); + $c->unhandled_exception($_); } }; } @@ -267,13 +258,7 @@ sub update { ); } else { - return $c->render( - status => 500, - openapi => { - error => - "Something went wrong, check Koha logs for details." - } - ); + $c->unhandled_exception($_); } }; } @@ -304,13 +289,7 @@ sub delete { ); } else { - return $c->render( - status => 500, - openapi => { - error => - "Something went wrong, check Koha logs for details." - } - ); + $c->unhandled_exception($_); } }; } @@ -347,13 +326,7 @@ sub guarantors_can_see_charges { } } catch { - return $c->render( - status => 500, - openapi => { - error => - "Something went wrong, check Koha logs for details. $_" - } - ); + $c->unhandled_exception($_); }; } @@ -389,13 +362,7 @@ sub guarantors_can_see_checkouts { } } catch { - return $c->render( - status => 500, - openapi => { - error => - "Something went wrong, check Koha logs for details. $_" - } - ); + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Patrons/Account.pm b/Koha/REST/V1/Patrons/Account.pm index 4dd9ee2bc4..aee36497ce 100644 --- a/Koha/REST/V1/Patrons/Account.pm +++ b/Koha/REST/V1/Patrons/Account.pm @@ -48,26 +48,31 @@ sub get { return $c->render( status => 404, openapi => { error => "Patron not found." } ); } - my $account = $patron->account; - - # get outstanding debits and credits - my $debits = $account->outstanding_debits; - my $credits = $account->outstanding_credits; - - return $c->render( - status => 200, - openapi => { - balance => $account->balance, - outstanding_debits => { - total => $debits->total_outstanding, - lines => $debits->to_api - }, - outstanding_credits => { - total => $credits->total_outstanding, - lines => $credits->to_api - } - } - ); + return try { + my $account = $patron->account; + + # get outstanding debits and credits + my $debits = $account->outstanding_debits; + my $credits = $account->outstanding_credits; + + return $c->render( + status => 200, + openapi => { + balance => $account->balance, + outstanding_debits => { + total => $debits->total_outstanding, + lines => $debits->to_api + }, + outstanding_credits => { + total => $credits->total_outstanding, + lines => $credits->to_api + } + } + ); + } + catch { + $c->unhandled_exception($_); + }; } =head3 add_credit @@ -141,19 +146,7 @@ sub add_credit { return $c->render( status => 200, openapi => { account_line_id => $credit->id } ); } catch { - if ( blessed $_ && $_->can('rethrow') ) { - return $c->render( - status => 400, - openapi => { error => "$_" } - ); - } - else { - # Exception, rely on the stringified exception - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs" } - ); - } + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/Patrons/Password.pm b/Koha/REST/V1/Patrons/Password.pm index af7af57ac4..82531d9d49 100644 --- a/Koha/REST/V1/Patrons/Password.pm +++ b/Koha/REST/V1/Patrons/Password.pm @@ -66,12 +66,14 @@ sub set { return $c->render( status => 200, openapi => "" ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( status => 500, openapi => { error => "$_" } ); + if ( blessed $_ and $_->isa('Koha::Exceptions::Password') ) { + return $c->render( + status => 400, + openapi => { error => "$_" } + ); } - # an exception was raised. return 400 with the stringified exception - return $c->render( status => 400, openapi => { error => "$_" } ); + $c->unhandled_exception($_); }; } @@ -117,7 +119,10 @@ sub set_public { return try { my $dbh = C4::Context->dbh; unless ( checkpw_internal($dbh, $user->userid, $old_password ) ) { - Koha::Exceptions::Authorization::Unauthorized->throw("Invalid password"); + return $c->render( + status => 400, + openapi => { error => "Invalid password" } + ); } ## Change password @@ -126,12 +131,14 @@ sub set_public { return $c->render( status => 200, openapi => "" ); } catch { - unless ( blessed $_ && $_->can('rethrow') ) { - return $c->render( status => 500, openapi => { error => "$_" } ); + if ( blessed $_ and $_->isa('Koha::Exceptions::Password') ) { + return $c->render( + status => 400, + openapi => { error => "$_" } + ); } - # an exception was raised. return 400 with the stringified exception - return $c->render( status => 400, openapi => { error => "$_" } ); + $c->unhandled_exception($_); }; } diff --git a/Koha/REST/V1/ReturnClaims.pm b/Koha/REST/V1/ReturnClaims.pm index 4fd8527abd..d33a326e55 100644 --- a/Koha/REST/V1/ReturnClaims.pm +++ b/Koha/REST/V1/ReturnClaims.pm @@ -86,12 +86,8 @@ sub claim_returned { openapi => { error => "Mandatory attribute created_by missing" } ); } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + + $c->unhandled_exception($_); }; } @@ -137,18 +133,7 @@ sub update_notes { ); } catch { - if ( $_->isa('Koha::Exceptions::Exception') ) { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } @@ -196,18 +181,7 @@ sub resolve_claim { ); } catch { - if ( $_->isa('Koha::Exceptions::Exception') ) { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } @@ -237,18 +211,7 @@ sub delete_claim { ); } catch { - if ( $_->isa('Koha::Exceptions::Exception') ) { - return $c->render( - status => 500, - openapi => { error => "$_" } - ); - } - else { - return $c->render( - status => 500, - openapi => { error => "Something went wrong, check the logs." } - ); - } + $c->unhandled_exception($_); }; } -- 2.20.1