[RFC PATCH 2/2] gitweb: use eval + die for error (exception) handling
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Dec 13, 2010, 00:49 UTC
- Message-ID
- <20101213004916.9475.77561.stgit@localhost.localdomain>
- In-Reply-To
- <20101213004259.9475.87376.stgit@localhost.localdomain>
In gitweb code it is assumed that calling die_error() subroutine would end request, just like running "die" would. Up till now it was done by having die_error() jump to DONE_REQUEST (earlier DONE_GITWEB), or in earlier version just 'exit' (for mod_perl via ModPerl::Registry it ends request instead of exiting worker).
Instead of using 'goto DONE_REQUEST' for longjmp-like nonlocal jump, or using 'exit', gitweb uses now native for Perl exception handlingin the form of eval / die pair ("eval BLOCK" to trap exceptions, "die LIST" to raise/throw them).
Up till now the "goto DONE_REQUEST" solution was enough, but with the coming output caching support and it adding modular structure to gitweb, it would be difficult to continue to keep using this solution (e.g. interaction with capturing output).
Because gitweb now traps all exceptions occuring run_request(), the handle_errors_html handler (set via set_message from CGI::Carp) is not needed; gitweb can call die_error in -error_handler mode itself. This has the advantage that we can now set correct HTTP header (handler from CGI::Carp::set_message was run after HTTP headers were already sent).
Gitweb assumes here that exceptions thrown by Perl would be simple strings; die_error() throws hash reference (if not for minimal extrenal dependencies, it would be probable object of Class::Exception or Throwable class thrown).
Note: in newer versions of CGI::Carp there is set_die_handler(), where handler have to set HTTP headers to the browser itself, but we cannot rely on new enough CGI::Carp version to have been installed. Also set_die_handler interferes with fatalsToBrowser.
Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- This is main part of this series.
Comments?
gitweb/gitweb.perl | 26 ++++++++------------------ 1 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index af45daa..ab85c53 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -20,7 +20,7 @@ use lib __DIR__ . '/lib'; use CGI qw(:standard :escapeHTML -nosticky); use CGI::Util qw(unescape); -use CGI::Carp qw(fatalsToBrowser set_message); +use CGI::Carp qw(fatalsToBrowser); use Encode; use Fcntl ':mode'; use File::Find qw(); @@ -1034,21 +1034,6 @@ sub configure_gitweb_features { } } -# custom error handler: 'die <message>' is Internal Server Error -sub handle_errors_html { - my $msg = shift; # it is already HTML escaped - - # to avoid infinite loop where error occurs in die_error, - # change handler to default handler, disabling handle_errors_html - set_message("Error occured when inside die_error:\n$msg"); - - # you cannot jump out of die_error when called as error handler; - # the subroutine set via CGI::Carp::set_message is called _after_ - # HTTP headers are already written, so it cannot write them itself - die_error(undef, undef, $msg, -error_handler => 1, -no_http_header => 1); -} -set_message(\&handle_errors_html); - # dispatch sub dispatch { if (!defined $action) { @@ -1145,7 +1130,11 @@ sub run { $pre_dispatch_hook->() if $pre_dispatch_hook; - run_request(); + eval { run_request() }; + if (defined $@ && !ref($@)) { + # some Perl error, but not one thrown by die_error + die_error(undef, undef, $@, -error_handler => 1); + } DONE_REQUEST: $post_dispatch_hook->() @@ -3670,7 +3659,8 @@ EOF print "</div>\n"; git_footer_html(); - goto DONE_REQUEST + + die {'status' => $status, 'error' => $error} unless ($opts{'-error_handler'}); }