Bugzilla – Attachment 102307 Details for
Bug 25032
Generic unhandled exception handling
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25032: Make existing controllers use unhandled_exception
Bug-25032-Make-existing-controllers-use-unhandlede.patch (text/plain), 39.38 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-04-02 19:25:44 UTC
(
hide
)
Description:
Bug 25032: Make existing controllers use unhandled_exception
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-04-02 19:25:44 UTC
Size:
39.38 KB
patch
obsolete
>From ff7e5d3fdf84e17e1fa5ae3c6f8bc06b6a991cbd Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 >--- > 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.26.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25032
:
102160
|
102161
|
102305
|
102306
|
102307
|
102523
|
102524
|
102525
|
102947
|
102971
|
102972
|
102973
|
102974
|
102975
|
103752
|
103753
|
103754
|
103755
|
103756