Subject: Re: [PATCH] gitweb.js: Harden setting blamed commit info in incremental blame

From: Jakub Narebski <[email-masked]>
Date: Wed, 25 Nov 2009 21:55:58 +0100

On Wed, 25 Nov 2009, Jakub Narebski wrote:
> On Wed, 25 Nov 2009 05:01, Stephen Boyd wrote:
> > Jakub Narebski wrote:
> > >
> > > Debugging this is serious PITA.  After fix below it appears that this bug
> > > is some intermittent bug, depending on XMLHttpRequest timing.  It more
> > > often than not (at least when I tried to debug it using build-in IE8
> > > debugger) works correctly for the folowing files: README, GIT-VERSION-GEN,
> > > revision.c (once even it did fail when first running for given file, and
> > > then running correctly when reloading from debugger; fun it is not).
> > >
> > > It does consistently fail for gitweb/gitweb.perl... but when I tried
> > > to debug it IE8 would hang up when trying to use debugger (with around
> > > 600MB available RAM).  Perhaps somebody else would have more luck...
> > 
> > Interesting. I don't have time to look into this until early December, 
> > but if it's still around then I'll take a look. I wonder if IE6 or IE7 
> > works (I don't think everyone is on version 8 yet).
> 
> Well, the one time I was able to run debugger (F12, select 'Script', select
> 'gitweb.js') with error occurring

The error was "Unspecified error", char:2 in the mentioned line

> and without IE hanging (for README file) it did show an error for the
> following line: 
> 
>   if (xhr.readyState === 3 && xhr.status !== 200) {
> 
> When I checked 'xhr' object, it has "Unknown error" as contents of 
> xhr.statusText field and as contents of xhr.status (sic!), which should
> be a number: HTTP status code.

It was 'Unspecified error.' shown in xhr watch.  Accessing xhr.status
causes an error.

This might be cause by the fact that xhr (XMLHttpRequest object, or as IE8
shows it in JScript debugger DispHTMLXMLHttpRequest object) is not fully
initialized, or something; gitweb.js calls handleResponse() also from
a timer, to work around the fact that some web browsers onreadystatechange
handler is called only once for each state, and not as soon as new data
is available from server.

Longer term solution would be to use onprogress handler if it is available;
if it is available we don't need trick with calling handleResponse from
timer, as XMLHttpRequest 2.0 proposed specification ensures calling callback
as soon as new data is available.

Short term solution would be to wrap access to xhr.status in try ... catch
block for IE8... although I am a bit reluctant to implement this workaround
bugs in IE8.

-- 
Jakub Narebski
Poland