Re: [PATCH] gitweb: return correct HTTP status codes

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 16, 2008, 22:34 UTC
Message-ID: <200806170034.42621.jnareb@gmail.com>
In-Reply-To: <4856DFE8.9010809@gmail.com>

On Mon, 16 Jun 2008, Lea Wiemann wrote:

Show 7 quoted lines

> Jakub Narebski wrote:
>>
>> Well, we could, perhaps, examine stderr (or redirect it to stdout and
>> examine upon error) to check what was the error.
> 
> We don't have to -- gitweb's current (suboptimal) error checking is 
> because it doesn't interface with git very well.

The "examine stderr" was a bit tongue-in-cheek, i.e. solution which would require least changes... but I guess very impractical.

> The API I'm writing  
> will fix this (i.e. provide proper feedback in all cases) so we'll have 
> more specific status codes.  IOW, we'll be able to differentiate between 
> 500 and 404.  Trust me on this one. ;-)

I have thought that Git.pm API together with catching (and examining) Error, perhaps with redirecting STDERR somewhere (but it would be best if it would not be needed) would be enough.

Show 10 quoted lines

>> But I think in all, or almost all cases, the source is wrong parameters
>> in URL.  Now, returning 5xx _server_ error would make me want to email
>> webmaster about error on his/her server, while 4xx _user_ error would
>> make me examine my input
> 
> Since the status codes will get better (more accurate) anyway, I care 
> more about correctness than practicalities right now (and I'm convinced 
> that only 500 is correct in the cases we're talking about).  That said, 
> if you really want 404s in there, go ahead and send a follow-up patch, I 
> won't object.

If the source of error is some misconfiguration on server, then 5xx is appropriate (for example git binary is not found, something which perhaps we should check upfront at the beginning). But I think it should be very, very rare, and result of misconfigured gitweb, or error installing git... or corrupted repository.

If source of error is mistake in URL, I would certainly want 4xx error. So the user knows that he/she has to look at the URL.

That said, perhaps I am worrying over nothing, and
  or die_error(undef, "Open <git command> failed");
can happen only on some serious server error (like corrupted
repository).

>From RFC 2616 (http://tools.ietf.org/html/rfc2616)

 10.4 Client Error 4xx

   The 4xx class of status code is intended for cases in which the
   client seems to have erred.

 [...]

 10.5 Server Error 5xx

   Response status codes beginning with the digit "5" indicate cases in
   which the server is aware that it has erred or is incapable of
   performing the request.

Show 5 quoted lines

>> BTW. I got three copies of this email: was it you fighting VGER
>> anti-spam filter?
> 
> Yup.  Apparently it simply greps for Content-TypXe: text/hXtml.  *shakes 
> head* :-)

It is unfortunately very simple pattern based filter, not Markovian, spam/ham Bayesian, or even simple Bayesian.

-- 
Jakub Narebski
Poland