[Bug 25504] New: Wrong API spec breaks plack without meaningul error
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Bug ID: 25504 Summary: Wrong API spec breaks plack without meaningul error Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Without the patches from 25482, if you start plack under D10, you get the output from bug 25482 comment 11. We should have something more meaningful to help the debug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25482 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104884&action=edit Bug 25504: Crash with the original error if API spec are invalid Bug 24700 put the whole spec validation into a try catch block that hides the original error raised by the JSON validator. If you don't have the patches from bug 25482 and start plack under D10 you will see a huge stacktrace with, lost in the middle: Koha::REST::V1::catch {...} ("Duplicate keys not allowed, at character offset 4789 (before "...) called at /usr/share/perl5/Try/Tiny.pm line 123 The complete output is at bug 25482 comment 11. With this patch, you will get a single line: Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load application from file "/kohadevbox/koha/api/v1/app.pl": Duplicate keys not allowed, at character offset 4789 (before "editcatalogue": "cre...") at /usr/share/perl5/Mojo/JSON.pm line 39. I am all for stacktraces, but not if the error is hidden ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Wrong API spec breaks plack |Wrong API spec breaks plack |without meaningul error |without meaningful error CC| |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=25504 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Should we have regression tests? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104884|0 |1 is obsolete| | --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 104894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104894&action=edit Bug 25504: Crash with the original error if API spec are invalid Bug 24700 put the whole spec validation into a try catch block that hides the original error raised by the JSON validator. If you don't have the patches from bug 25482 and start plack under D10 you will see a huge stacktrace with, lost in the middle: Koha::REST::V1::catch {...} ("Duplicate keys not allowed, at character offset 4789 (before "...) called at /usr/share/perl5/Try/Tiny.pm line 123 The complete output is at bug 25482 comment 11. With this patch, you will get a single line: Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load application from file "/kohadevbox/koha/api/v1/app.pl": Duplicate keys not allowed, at character offset 4789 (before "editcatalogue": "cre...") at /usr/share/perl5/Mojo/JSON.pm line 39. I am all for stacktraces, but not if the error is hidden ;) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- The use of $validator->bundle breaks Stretch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #4)
The use of $validator->bundle breaks Stretch
It is always falling back to the catch block in stretch........ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Jonathan, I didn't manage to make things work on JSON::Validator 0.97. I give up on this one. Lets discuss using local::lib as everyone else, and bundle our dependencies... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It means that the improvement speed bug 24700 was supposed to bring, never actually worked? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24700 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24700 [Bug 24700] Improve Mojo startup speed for REST APIs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #7)
It means that the improvement speed bug 24700 was supposed to bring, never actually worked?
It means not in stretch. Unless you cpan some libs to buster version -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Stretch is the only OS we really support right now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #9)
Stretch is the only OS we really support right now.
I tried by all means to make the validator fail on the broken spec (the permissions problem in advancededitor_macros) without luck. I think this means the 'feature' doesn't work on Stretch, but is not breaking anything either. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED Assignee|jonathan.druart@bugs.koha-c |koha-bugs@lists.koha-commun |ommunity.org |ity.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_11_target CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ere.maijala@helsinki.fi --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- This is really problematic as it hides the original error. Ere could you have a look? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I think we should at least log the error from the try block. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #13 from Ere Maijala <ere.maijala@helsinki.fi> --- I'll take a look. I can't see right away how Bug 24700 broke this, since the try block is just trying to take a shortcut, and if validation fails, it should do the very same as before the patch. I'm probably missing a detail here, but I'll try with D10 to see what happens. Letting it crash with the failed bundle might be confusing since the result is an amalgamation of the basic spec and any REST plugins. That's why it's swallowed and the one-by-one validation takes place in the catch block. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Ere, the problem I am having is for debugging purpose. Say you have a syntax problem in one of your json files, the error from the first try block will not be logged. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104894|0 |1 is obsolete| | --- Comment #15 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 106011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106011&action=edit Bug 25504: Improve REST API spec loading errors Test plan: 1. Introduce a typo in swagger.json or another spec json file. 2. Restart plack if used or try to access the REST APIs 3. Without the patch, verify that an incomplete error message and potentially lots of stack trace are logged. 4. With the patch, verify that much more meaningful error messages are logged and stack trace is omitted. 5. Fix the problem introduced in step 1 and verify that no messages are logged, or only warning about bundle is logged with Debian Stretch. 6. Repeat with a REST API plugin. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |ere.maijala@helsinki.fi |ity.org | --- Comment #16 from Ere Maijala <ere.maijala@helsinki.fi> --- Would the attached patch suffice? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- At first glance it looks great, thanks Ere! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Could we add: my $logger = Koha::Logger->get({ interface => 'api' }); $logger->error("$_"); instead of carp? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It's now pretty verbose but at least we have the full stacktrace! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106011|0 |1 is obsolete| | --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 109227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109227&action=edit Bug 25504: Improve REST API spec loading errors Test plan: 1. Introduce a typo in swagger.json or another spec json file. 2. Restart plack if used or try to access the REST APIs 3. Without the patch, verify that an incomplete error message and potentially lots of stack trace are logged. 4. With the patch, verify that much more meaningful error messages are logged and stack trace is omitted. 5. Fix the problem introduced in step 1 and verify that no messages are logged, or only warning about bundle is logged with Debian Stretch. 6. Repeat with a REST API plugin. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 109228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109228&action=edit Bug 25504: Use Koha::Logger and display the full stacktrace -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109227|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109228|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 109233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109233&action=edit Bug 25504: Improve REST API spec loading errors Test plan: 1. Introduce a typo in swagger.json or another spec json file. 2. Restart plack if used or try to access the REST APIs 3. Without the patch, verify that an incomplete error message and potentially lots of stack trace are logged. 4. With the patch, verify that much more meaningful error messages are logged and stack trace is omitted. 5. Fix the problem introduced in step 1 and verify that no messages are logged, or only warning about bundle is logged with Debian Stretch. 6. Repeat with a REST API plugin. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 109234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109234&action=edit Bug 25504: Use Koha::Logger and display the full stacktrace Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 109235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109235&action=edit Bug 25504: (QA follow-up) Remove unused include Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #26 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 109420 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109420&action=edit Bug 25504: Fix failing tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #26)
Created attachment 109420 [details] [review] Bug 25504: Fix failing tests
Pushed to master for 20.11. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_11_target, | |RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #28 from Lucas Gass <lucas@bywatersolutions.com> --- doesnt apply clean to 20.05.x, no backport if needed please rebase -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25504 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27190 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org