Thread (29 messages) flat view 29 messages, 3 authors, 2016-06-15

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:17

Jakub Narebski wrote:
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).
Hmm, why not throw an object of new type Gitweb::Exception?
quoted hunk ↗ jump to hunk
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -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);
-
Hoorah!
quoted hunk ↗ jump to hunk
 # 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);
+		}
The !ref($@) seems overzealous, which is why I am wondering if it
would be possible to use bless() for a finer-grained check.
quoted hunk ↗ jump to hunk
 
 	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'});
Is the DONE_REQUEST label still needed?

Thanks, I am happy to see the semantics becoming less thorny.
Jonathan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help