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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18206
Bug 18206 depends on bug 18205, which changed state.

Bug 18205 Summary: Use Koha::Logger/Log4Perl using Mojolicious app log method
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18205

   What|Removed |Added

 Status|Failed QA   |RESOLVED
 Resolution|--- |DUPLICATE

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Tomás Cohen Arazi  changed:

   What|Removed |Added

 Status|BLOCKED |RESOLVED
 Resolution|--- |DUPLICATE

--- Comment #19 from Tomás Cohen Arazi  ---
Some ideas from this bug could be reused on top of bug 25032. Marking as
duplicate for now.

*** This bug has been marked as a duplicate of bug 25032 ***

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Martin Renvoize  changed:

   What|Removed |Added

 CC||martin.renvoize@ptfs-europe
   ||.com

--- Comment #18 from Martin Renvoize  ---
I'd love to see this one start moving again..

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Tomás Cohen Arazi  changed:

   What|Removed |Added

 Status|Signed Off  |BLOCKED

--- Comment #17 from Tomás Cohen Arazi  ---
I like the idea of a more generic fallback exception handling method.
I'm not sure how it fits what we already do in Koha::REST::V1::Auth::under.

I will mark it as blocked until bug 18205 moves to avoid confusion.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #69229|0   |1
is obsolete||
  Attachment #69230|0   |1
is obsolete||
  Attachment #69231|0   |1
is obsolete||

--- Comment #14 from Josef Moravec  ---
Created attachment 85715
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85715=edit
Bug 18206: Default exception handling for REST API

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

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

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

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

What does this patch actually do?

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

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

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

--- Comment #16 from Josef Moravec  ---
Created attachment 85717
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85717=edit
Bug 18206: Fix missing use in api/v1.t test

Test plan:
prove t/d_dependent/api/v1.t

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

--- Comment #15 from Josef Moravec  ---
Created attachment 85716
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85716=edit
Bug 18206: Use this feature for cities

To test:
1. prove t/db_dependent/api/v1/cities.t

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

 Status|Needs Signoff   |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 Status|BLOCKED |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #68243|0   |1
is obsolete||

--- Comment #13 from Lari Taskula  ---
Created attachment 69231
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69231=edit
Bug 18206 - REST API: Default exception handling - Squashable, fix regexp

t/db_dependent/api/v1.t .. Unescaped left brace in regex is deprecated, passed
through in regex; marked by <-- HERE in m/ERROR - Unknown::Exception::OhNo => {
<-- HERE "what":"test unknown exception"}/ at t/db_dependent/api/v1.t line 100.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61171|0   |1
is obsolete||

--- Comment #12 from Lari Taskula  ---
Created attachment 69230
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69230=edit
Bug 18206: Use this feature for cities

To test:
1. prove t/db_dependent/api/v1/cities.t

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61170|0   |1
is obsolete||

--- Comment #11 from Lari Taskula  ---
Created attachment 69229
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69229=edit
Bug 18206: Default exception handling for REST API

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

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

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

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

What does this patch actually do?

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

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

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 Blocks||19652


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19652
[Bug 19652] Debug-level logging for response JSON
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Katrin Fischer  changed:

   What|Removed |Added

 CC||katrin.fisc...@bsz-bw.de
 Status|Needs Signoff   |BLOCKED

--- Comment #10 from Katrin Fischer  ---
Depends on a patch that doesn't apply, marking BLOCKED for now. Please fix
dependency tree from the top and reset status!

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Olli-Antti Kivilahti  changed:

   What|Removed |Added

 CC||olli-antti.kivila...@jns.fi

--- Comment #9 from Olli-Antti Kivilahti  ---
Wow!

This feature is orgastic.

Looking at the codebase, it is obvious Perl has way too many types of
Exceptions. This will turn this incredible frustration to something manageable.

>> Lari, do you think we could call the method 'rethrow'?

rethrow() is already reserved by Exception::Class?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

--- Comment #8 from Olli-Antti Kivilahti  ---
Created attachment 68243
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68243=edit
Bug 18206 - REST API: Default exception handling - Squashable, fix regexp

t/db_dependent/api/v1.t .. Unescaped left brace in regex is deprecated, passed
through in regex; marked by <-- HERE in m/ERROR - Unknown::Exception::OhNo => {
<-- HERE "what":"test unknown exception"}/ at t/db_dependent/api/v1.t line 100.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 CC||jonathan.dru...@bugs.koha-c
   ||ommunity.org
  Component|Web services|REST api
 QA Contact|testo...@bugs.koha-communit |
   |y.org   |

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

--- Comment #7 from Tomás Cohen Arazi  ---
Lari, do you think we could call the method 'rethrow'?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 Blocks||14843


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14843
[Bug 14843] Notifications and messages via REST API
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61088|0   |1
is obsolete||

--- Comment #6 from Lari Taskula  ---
Created attachment 61171
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61171=edit
Bug 18206: Use this feature for cities

To test:
1. prove t/db_dependent/api/v1/cities.t

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61087|0   |1
is obsolete||

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

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

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

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

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

What does this patch actually do?

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

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

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 CC||tomasco...@gmail.com

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61072|0   |1
is obsolete||

--- Comment #3 from Lari Taskula  ---
Created attachment 61087
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61087=edit
Bug 18206: Default exception handling for REST API

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

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

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

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

What does this patch actually do?

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

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

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61073|0   |1
is obsolete||

--- Comment #4 from Lari Taskula  ---
Created attachment 61088
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61088=edit
Bug 18206: Use this feature for cities

To test:
1. prove t/db_dependent/api/v1/cities.t

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 Status|NEW |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

--- Comment #2 from Lari Taskula  ---
Created attachment 61073
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61073=edit
Bug 18206: Use this feature for cities

To test:
1. prove t/db_dependent/api/v1/cities.t

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 Depends on||18205


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18205
[Bug 18205] REST interface to Koha::Logger
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

--- Comment #1 from Lari Taskula  ---
Created attachment 61072
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61072=edit
Bug 18206: Default exception handling for REST API

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

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

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

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

What does this patch actually do?

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

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

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

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

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

   Assignee|koha-b...@lists.koha-commun |lari.task...@jns.fi
   |ity.org |

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/