[RFC PATCH v7 2/9] gitweb: use eval + die for error (exception) handling

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 22, 2010, 23:55 UTC
Message-ID: <20101222235525.7998.99816.stgit@localhost.localdomain>
In-Reply-To: <20101222234843.7998.87068.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>
---

 gitweb/gitweb.perl |   26 ++++++++------------------
 1 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 724287b..c7a1892 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -12,7 +12,7 @@ use strict;
 use warnings;
 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();
@@ -1045,21 +1045,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) {
@@ -1167,7 +1152,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->()
@@ -3768,7 +3757,8 @@ EOF
 	print "</div>\n";
 
 	git_footer_html();
-	goto DONE_REQUEST
+
+	die {'status' => $status, 'error' => $error}
 		unless ($opts{'-error_handler'});
 }