Bug 25032: Make existing controllers use unhandled_exception
authorTomas Cohen Arazi <tomascohen@theke.io>
Thu, 2 Apr 2020 17:43:27 +0000 (14:43 -0300)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Wed, 29 Apr 2020 15:24:18 +0000 (16:24 +0100)
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 <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

15 files changed:
Koha/Exceptions/ClubHold.pm
Koha/REST/V1/Acquisitions/Funds.pm
Koha/REST/V1/Acquisitions/Orders.pm
Koha/REST/V1/Acquisitions/Vendors.pm
Koha/REST/V1/Biblios.pm
Koha/REST/V1/Checkouts.pm
Koha/REST/V1/Cities.pm
Koha/REST/V1/Clubs/Holds.pm
Koha/REST/V1/Holds.pm
Koha/REST/V1/Items.pm
Koha/REST/V1/Libraries.pm
Koha/REST/V1/Patrons.pm
Koha/REST/V1/Patrons/Account.pm
Koha/REST/V1/Patrons/Password.pm
Koha/REST/V1/ReturnClaims.pm

index 429f89c..e787c27 100644 (file)
@@ -12,4 +12,4 @@ use Exception::Class (
     },
 );
 
-1;
\ No newline at end of file
+1;
index 533d3e2..ee07046 100644 (file)
@@ -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($_);
     };
 }
 
index 989cb6b..a9ed512 100644 (file)
@@ -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($_);
     };
 }
 
index 974491b..8fcf715 100644 (file)
@@ -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;
index 121d66f..80c7200 100644 (file)
@@ -95,10 +95,7 @@ sub get {
         }
     }
     catch {
-        return $c->render(
-            status  => 500,
-            openapi => { error => "Something went wrong, check the logs ($_)" }
-        );
+        $c->unhandled_exception($_);
     };
 }
 
@@ -134,18 +131,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($_);
     };
 }
 
index 3a5f448..9d5e5b4 100644 (file)
@@ -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;
index 4c49e1a..6b76daa 100644 (file)
@@ -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($_);
     };
 }
 
index 5851e15..6acaced 100644 (file)
@@ -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($_);
     };
 }
 
index a806d67..2a8c055 100644 (file)
@@ -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($_);
     };
 }
 
index 3213fad..4781024 100644 (file)
@@ -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($_);
     };
 }
 
index c63b823..9025adb 100644 (file)
@@ -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($_);
     };
 }
 
index 8350f85..8ff645c 100644 (file)
@@ -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($_);
     };
 }
 
index 4dd9ee2..aee3649 100644 (file)
@@ -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($_);
     };
 }
 
index af7af57..82531d9 100644 (file)
@@ -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($_);
     };
 }
 
index 4fd8527..d33a326 100644 (file)
@@ -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($_);
     };
 }