git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH 4/5] gitweb: Add custom error handler using die_error
Date: Sat, 24 Apr 2010 16:00:04 +0200	[thread overview]
Message-ID: <20100424135627.30511.70847.stgit@localhost.localdomain> (raw)
In-Reply-To: <20100424132255.30511.98829.stgit@localhost.localdomain>

Change the default message for errors (for fatalsToBrowser) to use
die_error() subroutine.  This way errors (and explicitely calling 'die
MESSAGE') would generate 'Internal Server Error' error message.

Note that call to set_message is intentionally not put in BEGIN block;
we set error handler to use die_error() only after we are sure that we
can use it, after all needed variables are set.

Due to the fact that error handler set via set_message() subroutine
from CGI::Carp (in the fatalsToBrowser case) is called after HTTP
headers were already printed (with exception of MOD_PERL), gitweb
cannot return 'Status: 500 Internal Server Error'.

Thanks to the fact that die_error() no longer uses 'exit', errors
would be logged by CGI::Carp, independent on whether default error
handler is used, or handle_errors_html which uses die_error is used.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Because of the fact that with CGI::Carp::set_message and die_error
you cannot return proper HTTP status code, I have decided that
caching engine needs support for custom error handler anyway, a la
'on_get_error' and 'on_set_error' in CHI.

With this patch most error messages, from fatal errors in gitweb or
Perl, should get look of the rest of gitweb.  This is good idea even
without caching support.

 gitweb/gitweb.perl |   27 +++++++++++++++++++++++----
 1 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ed92dca..e579c14 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -11,7 +11,7 @@ use strict;
 use warnings;
 use CGI qw(:standard :escapeHTML -nosticky);
 use CGI::Util qw(unescape);
-use CGI::Carp qw(fatalsToBrowser);
+use CGI::Carp qw(fatalsToBrowser set_message);
 use Encode;
 use Fcntl ':mode';
 use File::Find qw();
@@ -952,6 +952,21 @@ if ($git_avatar eq 'gravatar') {
 	$git_avatar = '';
 }
 
+# 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
 if (!defined $action) {
 	if (defined $hash) {
@@ -3167,6 +3182,7 @@ sub blob_contenttype {
 sub git_header_html {
 	my $status = shift || "200 OK";
 	my $expires = shift;
+	my %opts = @_;
 
 	my $title = "$site_name";
 	if (defined $project) {
@@ -3194,7 +3210,8 @@ sub git_header_html {
 		$content_type = 'text/html';
 	}
 	print $cgi->header(-type=>$content_type, -charset => 'utf-8',
-	                   -status=> $status, -expires => $expires);
+	                   -status=> $status, -expires => $expires)
+		unless ($opts{'-no_http_headers'});
 	my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : '';
 	print <<EOF;
 <?xml version="1.0" encoding="utf-8"?>
@@ -3411,6 +3428,7 @@ sub die_error {
 	my $status = shift || 500;
 	my $error = esc_html(shift) || "Internal Server Error";
 	my $extra = shift;
+	my %opts = @_;
 
 	my %http_responses = (
 		400 => '400 Bad Request',
@@ -3419,7 +3437,7 @@ sub die_error {
 		500 => '500 Internal Server Error',
 		503 => '503 Service Unavailable',
 	);
-	git_header_html($http_responses{$status});
+	git_header_html($http_responses{$status}, undef, %opts);
 	print <<EOF;
 <div class="page_body">
 <br /><br />
@@ -3433,7 +3451,8 @@ EOF
 	print "</div>\n";
 
 	git_footer_html();
-	goto DONE_GITWEB;
+	goto DONE_GITWEB
+		unless ($opts{'-error_handler'});
 }
 
 ## ----------------------------------------------------------------------

  parent reply	other threads:[~2010-04-24 14:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-24 13:46 [PATCH 0/5] gitweb: Miscellaneous improvements, in preparation for caching support Jakub Narebski
2010-04-24 13:50 ` [PATCH 1/5] Export more test-related variables when running external tests Jakub Narebski
2010-04-25 11:47   ` Jakub Narebski
2010-04-24 13:53 ` [PATCH 2/5] gitweb: href(..., -path_info => 0|1) Jakub Narebski
2010-04-24 13:56 ` [PATCH 3/5] gitweb: Use nonlocal jump instead of 'exit' in die_error Jakub Narebski
2010-05-04 11:39   ` Jakub Narebski
2010-04-24 14:00 ` Jakub Narebski [this message]
2010-04-24 14:01 ` [PATCH 5/5] gitweb: Move generating page title to separate subroutine Jakub Narebski
2010-04-25  9:20 ` [PATCH 0/5] gitweb: Miscellaneous improvements, in preparation for caching support Petr Baudis
2010-04-25 11:45   ` 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=20100424135627.30511.70847.stgit@localhost.localdomain \
    --to=jnareb@gmail.com \
    --cc=git@vger.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).