On Sun, 12 Dec 2010, J.H. wrote: > > Hmm... anuthing that happens after 'use CGI::Carp;' is parsed should > > have STDERR redirected to web server logs, see CGI::Carp manpage > > > > [...] > > > > use CGI::Carp > > > > And the standard warn(), die (), croak(), confess() and carp() calls will > > automagically be replaced with functions that write out nicely time-stamped > > messages to the HTTP server error log. > > > > [...] > > > > REDIRECTING ERROR MESSAGES > > > > By default, error messages are sent to STDERR. Most HTTPD servers direct > > STDERR to the server's error log. > > > > [...] > > > > Especially the second part. > > That was not what I was seeing, so either something I was doing was > horking how CGI::Carp works, or their claim that "most HTTPD server > direct STDERR to the server's error log" is false. > > > Could you give us example which causes described misbehaviour? > > While I was working on the trapping of the error pages I started getting > 500 errors when going to a non-existent sha1. Running the command from > the cli revealed that a message from a git command was making it out to > the console. Redirecting STDERR masked the error from git, and stopped > premature data being sent out before the headers were sent. Generally if something worked, and stopped working, don't you think that you should concentrate on fixing your code, and not papering over the issue? The fact that "Running the command from the cli revealed that a message from a git command was making it out to the console." doesn't mean anything, because when running gitweb from commandline both stdout and stderr are redirected to terminal, by default. So you should worry only if there is premature data being sent to standard output, with standard error redirected to /dev/null (2>/dev/null). What CGI::Carp does is (re)define 'die' and 'warn' to support fatalsToBrowser and warningsToBrowser, and to add timestamp and other auxiliary information: in the end 'die' calls 'CORE::die', and 'warn' calls 'CORE::warn' - both of which write to STDERR. This means that warnings from git commands sent to standard error do not get timestamp appended. Note that standard output from git commands run by gitweb is always captured. > > I have nothing against this patch: if you have to have it, then you > > have to have it. I oly try to understand what might be core cause > > behind the issue that this patch is to solve... > > I've re-tried this, if you remove this patch and attempt to visit a > non-exist sha1, *boom* > > I can only speculate that CGI::Carp only redirects the output inside of > perl, and does not handle the case when called programs (like git) write > more directly to STDERR. CGI::Carp doesn't redirect output: it adds timestamp and prints it to STDERR (unless one use 'carpout') to the result of 'die' and 'warn' calls. *Without your series* when I visit non-existing sha1, or non-existing file I get correctly 404 error from gitweb. So you have borked something. The CGI standard (http://tools.ietf.org/html/rfc3875) doesn't talk about 'standard error' stream at all; on the other hand it talks only about 'standard input' and 'standard output'. I have checked with simple CGI script in Perl, that neither using die or warn (both before any HTTP headers are send), neither with plain CGI or with mod_perl (ModPerl::Registry), with CGI::Carp I never get the error you see. Without CGI::Carp I get '500 Internal Server Error' instead of nicer one formatted by CGI::Carp, but I don't get it even without CGI::Carp with 'warn' and printing to STDERR directly. The standard error stream either gets discarded (mod_cgid), or is written to /var/log/httpd/error_log (mod_perl). -- Jakub Narebski Poland -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html