https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |jonathan.druart@bugs.koha-c | |ommunity.org Depends on| |22206 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22206 [Bug 22206] Add routes to enable suspension or resumption of holds -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 100200 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100200&action=edit Bug 24813: Prevent api/v1/holds.t to fail randomly Hold cannot be suspended if found=W: Koha::Hold->suspend_hold 90 if ( $self->is_found ) { # We can't suspend found holds So when TestBuilder generate a hold with found=W the test fail. A question however: Why did not we have the correct exception displayed somewhere? That would be much more helpful to find the problem! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100201|0 |1 is obsolete| | --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 100204 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100204&action=edit Bug 24813: Prevent api/v1/holds.t to fail randomly Hold cannot be suspended if found=W: Koha::Hold->suspend_hold 90 if ( $self->is_found ) { # We can't suspend found holds So when TestBuilder generate a hold with found=W the test fail. A question however: Why did not we have the correct exception displayed somewhere? That would be much more helpful to find the problem! Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #6)

Hoping to get an answer from Tomas on this one regarding Jonathan's question.. we would like to know if the API should really be returning the actual exception error in the 500 response.

The usual pattern is: 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 => "$_" } ); So if it is not showing the exception info it is because it is not an exception. In both cases I'd feel comfortable with a change to send the exception to the logs, unless debugging is set (i.e. no need to give the consumer details about the internal problems in Koha). So I vote for all unhandled exceptions to return a generic internal server error and warn the exception info. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It's not related to 500 but 400. There is a Koha::Exceptions::Hold::CannotSuspendFound that is raised and we need to see it somewhere. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #11)

(In reply to Jonathan Druart from comment #10)

...

It's not related to 500 but 400. There is a Koha::Exceptions::Hold::CannotSuspendFound that is raised and we need to see it somewhere.

My gut feeling is we should somehow overload Test::Mojo so it does ->or( sub { diag $t->tx->res->to_string } ), or just add it everywhere and enforce it as a coding guideline.

This would give us more light on handled exceptions, but we still need something for unhandled exceptions, in the lines of bug 18206 (probably making use of a helper instead, but you get the point).

That's not only API related, it's exception related. For instance I get that this morning: t/db_dependent/Koha/Object.t .. 9/20 # No tests run! # Failed test 'No tests run for subtest "attributes_from_api() tests"' # at t/db_dependent/Koha/Object.t line 568. [A bad parameter was given]# Looks like your test exited with 255 just after 14. It's impossible to track down where it comes from... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.05.00, 19.11.05 |20.05.00, 19.11.05, released in| |19.05.10 Status|Pushed to stable |Pushed to oldstable CC| |lucas@bywatersolutions.com --- Comment #14 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 19.05.x for 19.05.10 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24813 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25551 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25551 [Bug 25551] [OMNIBUS] Some tests are failing randomly -- You are receiving this mail because: You are watching all bug changes.