git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: "J.H." <warthog9@eaglescrag.net>,
	"John 'Warthog9' Hawley" <warthog9@kernel.org>
Subject: [RFC PATCH v7 2/9] gitweb: use eval + die for error (exception) handling
Date: Thu, 23 Dec 2010 00:55:26 +0100	[thread overview]
Message-ID: <20101222235525.7998.99816.stgit@localhost.localdomain> (raw)
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'});
 }
 

  parent reply	other threads:[~2010-12-22 23:56 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-22 23:54 [RFC PATCH v7 0/9] gitweb: Output caching, with eval/die based error handling Jakub Narebski
2010-12-22 23:55 ` [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error Jakub Narebski
2010-12-23  1:55   ` Jonathan Nieder
2010-12-25 22:14     ` Jakub Narebski
2010-12-26  9:07       ` [RFC/PATCH] diff: funcname and word patterns for perl Jonathan Nieder
     [not found]         ` <201012261143.33190.trast@student.ethz.ch>
2010-12-26 10:54           ` Jonathan Nieder
     [not found]             ` <201012261206.11942.trast@student.ethz.ch>
2010-12-26 11:22               ` Jonathan Nieder
2010-12-26 23:14         ` Jakub Narebski
2010-12-27 17:18           ` Junio C Hamano
2010-12-27 22:44             ` Jakub Narebski
2010-12-28  3:52             ` Jeff King
2010-12-26  9:50       ` [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error Jonathan Nieder
2010-12-26 22:25         ` Jakub Narebski
2010-12-22 23:55 ` Jakub Narebski [this message]
2010-12-23  2:08   ` [RFC PATCH v7 2/9] gitweb: use eval + die for error (exception) handling Jonathan Nieder
2010-12-25 23:17     ` Jakub Narebski
2011-01-04  0:35   ` [RFC PATCH v7 2.5/9] gitweb: Make die_error just die, and use send_error to create error pages Jakub Narebski
2010-12-22 23:55 ` [RFC PATCH v7 3/9] gitweb: Introduce %actions_info, gathering information about actions Jakub Narebski
2010-12-22 23:56 ` [RFC PATCH v7 4/9] gitweb: Prepare for splitting gitweb Jakub Narebski
2010-12-24  9:29   ` Jonathan Nieder
2010-12-26 22:54     ` Jakub Narebski
2010-12-22 23:56 ` [RFC PATCH v7 5/9] t/test-lib.sh: Export also GIT_BUILD_DIR in test_external Jakub Narebski
2010-12-22 23:57 ` [RFC PATCH v7 6/9] gitweb/lib - Simple output capture by redirecting STDOUT to file Jakub Narebski
2010-12-24  9:49   ` Jonathan Nieder
2010-12-26 23:03     ` Jakub Narebski
2010-12-22 23:57 ` [RFC PATCH v7 7/9] gitweb/lib - Very simple file based cache Jakub Narebski
2010-12-22 23:57 ` [RFC PATCH v7 8/9] gitweb/lib - Cache captured output (using compute_fh) Jakub Narebski
2010-12-22 23:58 ` [RFC PATCH v7 9/9] gitweb: Add optional output caching Jakub Narebski
2010-12-31 18:03 ` [RFC PATCH v7 10/9] gitweb: Background cache generation and progress indicator Jakub Narebski
2011-01-03 21:33 ` [RFC PATCH v7 11/9] [PoC] gitweb/lib - tee, i.e. print and capture during cache entry generation Jakub Narebski
2011-01-03 23:31   ` J.H.
2011-01-04  0:28     ` Jakub Narebski
2011-01-04 13:20       ` Jakub Narebski
2011-01-05  2:26 ` [RFC PATCH 11/9] [PoC] gitweb/lib - HTTP-aware output caching Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101222235525.7998.99816.stgit@localhost.localdomain \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=warthog9@eaglescrag.net \
    --cc=warthog9@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).