We rushed into using Koha::Logger as a Mojo::Log implementation, but there are some differences that need to be addressed before that can actually happen. This is not *that* problematic for production sites, but some things like hitting an undefined route will raise weird errors we need to avoid: GET http://kohadev.myDNSname.org:8081/api/v1/hola The application raised the following error: Can't use an undefined value as an ARRAY reference at template mojo/debug.html.ep line 311. 306: %= $kv->(Time => scalar localtime(time)) 307: </table> 308: </div> 309: <div class="tap">tap for more</div> 310: </div> 311: % if (@{app->log->history}) { 312: <div id="log" class="box infobox spaced"> 313: <table> 314: % for my $msg (@{app->log->history}) { 315: <tr> 316: <td class="striped value wide"> and the StackTrace middleware couldn't catch its stack trace, possibly because your application overrides $SIG{__DIE__} by itself, preventing the middleware from working correctly. Remove the offending code or module that does it: known examples are CGI::Carp and Carp::Always. <<<<< We need to move back to using plain Koha::Logger, and rethink the proposal from bug 18205 to use MojoX::Log::Log4perl. That certainly belongs to another bug.
Created attachment 104852 [details] [review] Bug 25493: Make ->unhandled_exception use Koha::Logger To test: 1. Edit the Koha/REST/V1/Cities.pm 'list' method adding die("Nada"); before the render step. 2. Restart plack and try the endpoint => SUCCESS: The message is generic, but you see something is logged in /var/log/koha/kohadev/api-error.log 3. Change die("Nada"); for a real exception like: use Koha::Exceptions; Koha::Exceptions::DuplicateObject->throw("Nada"); 4. Repeat 2. => SUCCESS: The message is generic, but a meaningful text is added to the logs. 5. Point your browser to the /api/v1/hola route from your dev environment => FAIL: Wow, such a weird error 6. Apply this patch 7. Restart plack and repeat 2, 3 and 4 => SUCCESS: No behaviour change 8. Repeat 5 => SUCCESS: The regular Mojolicious 404 weird page 9. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 105016 [details] [review] Bug 25493: Make ->unhandled_exception use Koha::Logger To test: 1. Edit the Koha/REST/V1/Cities.pm 'list' method adding die("Nada"); before the render step. 2. Restart plack and try the endpoint => SUCCESS: The message is generic, but you see something is logged in /var/log/koha/kohadev/api-error.log 3. Change die("Nada"); for a real exception like: use Koha::Exceptions; Koha::Exceptions::DuplicateObject->throw("Nada"); 4. Repeat 2. => SUCCESS: The message is generic, but a meaningful text is added to the logs. 5. Point your browser to the /api/v1/hola route from your dev environment => FAIL: Wow, such a weird error 6. Apply this patch 7. Restart plack and repeat 2, 3 and 4 => SUCCESS: No behaviour change 8. Repeat 5 => SUCCESS: The regular Mojolicious 404 weird page 9. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 105100 [details] [review] Bug 25493: Make ->unhandled_exception use Koha::Logger To test: 1. Edit the Koha/REST/V1/Cities.pm 'list' method adding die("Nada"); before the render step. 2. Restart plack and try the endpoint => SUCCESS: The message is generic, but you see something is logged in /var/log/koha/kohadev/api-error.log 3. Change die("Nada"); for a real exception like: use Koha::Exceptions; Koha::Exceptions::DuplicateObject->throw("Nada"); 4. Repeat 2. => SUCCESS: The message is generic, but a meaningful text is added to the logs. 5. Point your browser to the /api/v1/hola route from your dev environment => FAIL: Wow, such a weird error 6. Apply this patch 7. Restart plack and repeat 2, 3 and 4 => SUCCESS: No behaviour change 8. Repeat 5 => SUCCESS: The regular Mojolicious 404 weird page 9. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
A simple pragmatic approach to resolving the bug before release, thanks guys Passing QA
Nice work everyone! Pushed to master for 20.05
missing dependencies-not backported to 19.11.x