Bug 27557 - Add the ability to display the error that caused a 500
Summary: Add the ability to display the error that caused a 500
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 26048
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-27 10:14 UTC by Jonathan Druart
Modified: 2023-11-27 19:24 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 27557: Display stacktrace on 500 for superlibrarian (5.79 KB, patch)
2021-03-16 10:26 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-01-27 10:14:00 UTC
From bug 26048 comment 19:
"""
Yesterday I was thinking we could have a syspref to display/hide the error (not the stacktrace). Or enable it by default for superlibrarian. What do you think (on its own bug report of course)?
"""


From bug 26048 comment 30:
"""
But in most production situations I don't think that we'd want to show system generated error messages. It's not a good look.
"""
Comment 1 Fridolin Somers 2021-01-29 14:03:31 UTC
I'd like to see this.
We could add a syspref in Logs > Debugging
Comment 2 Jonathan Druart 2021-03-16 10:26:44 UTC
Created attachment 118293 [details] [review]
Bug 27557: Display stacktrace on 500 for superlibrarian

From pod for Plack::Middleware::StackTrace:
"""
Gathering the information for a stack trace via Devel::StackTrace is slow, and Devel::StackTrace::WithLexicals is significantly slower still. This is not usually a concern in development and when exceptions are rare. However, your application may include code that's throwing and catching exceptions that you're not aware of. Such code will run significantly slower with this module.
"""
Maybe we should avoid the stacktrace and prefer to retrieve the error
only, without the stacktrace? (but how?)

cp C4/Auth.pm /usr/share/koha/lib/C4/Auth.pm
cp debian/templates/plack.psgi /etc/koha/sites/kohadev/plack.psgi
cp errors/500.pl /usr/share/koha/intranet/cgi-bin/errors/500.pl
cp koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/modules/errors/errorpage.tt
restart_all
Comment 3 Jonathan Druart 2021-03-16 10:27:14 UTC
Any ideas how we could retrieve the original error (vs the whole stacktrace)?
Comment 4 David Cook 2021-03-17 00:21:00 UTC
(In reply to Jonathan Druart from comment #3)
> Any ideas how we could retrieve the original error (vs the whole stacktrace)?

Based on my comment from https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26048#c29, I think we would need to replace Plack::Middleware::HTTPException with our own custom error handling middleware. 

Looking at the patch here, I would remind folk that the StackTrace middleware is only recommended for development and not production: "You're recommended to use this middleware during the development and use Plack::Middleware::HTTPExceptions in the deployment mode as a replacement" (https://metacpan.org/pod/Plack::Middleware::StackTrace).

But the concept used by StackTrace (ie $env->{'plack.stacktrace.text'} = $text;) is what I would suggest.

Ultimately, write a middleware that catches the error using eval{} or try/catch, save the error to an environmental variable, print the error to psgi.errors, and then move on to the ErrorDocument middleware.