304980 - nsIXMLHttpRequest throws exception on access of status member when the request failed due to a timeout

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Willem Joosten]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

When a request is done using xmlhttprequest the callback is called with a
request status going from 1 to 4 (finished) When the called server times out on
the call status 4 is reached but accessing the status property produces this
exception:

Error: [Exception... "Component returned failure code: 0x80040111
(NS_ERROR_NOT_AVAILABLE) [nsIXMLHttpRequest.status]"  nsresult: "0x80040111
(NS_ERROR_NOT_AVAILABLE)"  location: "JS frame ::
http://freeby/ajax/v2/formdemo.js :: handleAddressSaved :: line 192"  data: no]
Source File: http://freeby/ajax/v2/formdemo.js
Line: 192

This happens in 'alert(req.status)' as wel as testing for the existince of the
property using if(req.status)

This happens both in the Firefox version above as well as version 0.9 on FreeBSD
4.10.

Reproducible: Always

Steps to Reproduce:
1. create a XMLHttpRequest
2. make the server timeout (in Apache change the line 'TimeOut 300' in the
config file to something smaller
3. try to access the status property of the request object 

Actual Results:  
Javascript throws an exception.

Expected Results:  
The status en statusText property should be defined, maybe with a undefined value.

Comment 1

[Willem Joosten]

The status also can't be read when the server is not reachable. Also the
readyState goes through status 2 (statusLoaded) and 3 (statusInteractive) before
reaching 4.

Comment 2

[Gabriel Kneisley]

I am experiencing the same phenomenon.  A workaround is to catch the exception,
although the exception should not have been thrown, as request.readyState should
not have had a value of 4.

Comment 3

[Wojciech Polak]

This bug still occurs in FireFox 1.5 Beta 1.

Comment 4

[Stephen C]

Still happens on FF1.7 when the server is terminated before the request is sent. A status field like this needs to handle this situation.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible fix

We can't return "undefined" for an integer, but we can do the same thing as for a non-HTTP request -- return 0.

Comment 6

[Darin Fisher]

Comment on attachment 204399 [details] [diff] [review]
Possible fix

r=darin

Please document this in nsIXMLHttpRequest.  Is this documented in the Web Apps spec?  Should it be?  What do other browsers do?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 204399 [details] [diff] [review]
Possible fix

Per conversation with hixie, something like a 5xx code would make more sense here...

Stephen, Gabriel, Willem, what status code does IE return in this case?

Comment 8

[Darin Fisher]

> Per conversation with hixie, something like a 5xx code would make more sense
> here...

Really?  5xx seems to implicate the server, but that's not accurate.  Also, don't we need to worry about changing the API in an incompatible way?  What should the status codes be for 'file:' URLs?

Comment 9

[Boris Zbarsky [:bzbarsky]]

I'd be aiming to not change anything for file: urls -- those would continue to return 0.

502 doesn't necessarily implicate the server you were trying to contact...  Unfortunately, all the HTTP status codes do assume the server was contacted; none of them really mean "server didn't respond".  502 is the closest.

Comment 10

[Darin Fisher]

> I'd be aiming to not change anything for file: urls -- those would continue to
> return 0.

OK


> 502 doesn't necessarily implicate the server you were trying to contact... 
> Unfortunately, all the HTTP status codes do assume the server was contacted;
> none of them really mean "server didn't respond".  502 is the closest.

502 means there was a bad gateway.  That doesn't really sound accurate to me.

The first sentence of section 10.5 says:

  "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."

That really doesn't describe the case of a server response that is still pending.

Comment 11

[Boris Zbarsky [:bzbarsky]]

For still pending we want to keep throwing.  I think we should only change behavior for cases where our readyState is 3 or 4 (which means either OnDataAvailable or OnStopRequest has been called).  But network timeout will call OnStopRequest, as will connection refused, etc, right?

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

onStopRequest will always be called if asyncOpen returned success. so, yes.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated patch

This makes sure we still throw if readyState is not 3 or 4 (so if we're still waiting on initial server response).  I sort of randomly decided that the generic 500 status code is good enough here, but I really don't care that much what we make it.  IE uses 12029, which is ERROR_INTERNET_CANNOT_CONNECT (see http://msdn.microsoft.com/library/default.asp?url=/library/en-us/wininet/wininet/wininet_errors.asp).  I don't really think we want to be following IE to the extent of using that status code; if we do decide to go that way, we should use whatever error status our OnStopRequest got.... ;)

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 205440 [details] [diff] [review]
Updated patch

sr=jst

Comment 15

[Darin Fisher]

>  I don't really think we want to be following IE to the extent of using that
> status code; if we do decide to go that way, we should use whatever error
> status our OnStopRequest got.... ;)

Perhaps an internal error code would be more useful than a made-up 500 response that looks like it came from the server.  Synthesizing a 500 response is bound to confound someone, who may end up blaming the server for the wrong thing, look in the server logs, wonder why the server logs show no 500 response, and so on...

Comment 16

[Boris Zbarsky [:bzbarsky]]

I could just return NS_ERROR_NOT_AVAILABLE as the int, sure.  Want me to?

Comment 17

[Boris Zbarsky [:bzbarsky]]

The problem there is whether WHATWG can interoperably specify this.  It seems like that would be desirable, but with NS_ERROR_NOT_AVAILABLE it's tough.

Comment 18

[Darin Fisher]

A WG could easily just say that the useragent will throw an error and not specify the error, right?

Comment 19

[Boris Zbarsky [:bzbarsky]]

But we're not throwing an error.  We're returning an integer.  The error case we're going to throw NOT_AVAILABLE and be done with it; the problem is the non-error case.

Comment 20

[Darin Fisher]

I think we should consider doing what IE does.... looks like we haven't figured that out yet.  Do we have a testcase?  Probably not, right?...  because it is difficult to simulate a network timeout?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Hixie wrote a testcase for this, actually, so we do know exactly what IE does.  See comment 13 -- it returns an "HTTP status" of 12029, which corresponds to a WinAPI status code.

Comment 22

[Darin Fisher]

Sorry, I missed that while scrolling back through the comments.  So, do you really think that faking a legit server response code is the best solution here?  It doesn't feel right to me... perhaps we should have our own internal status code as well.  Otherwise, we are overloading the meaning of the HTTP 5xx status codes.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Yeah, I have to admit that 500 does make me somewhat unhappy... ;)

Our internal status code would be NS_ERROR_NOT_AVAILABLE, I guess.

Comment 24

[Darin Fisher]

Yes, let's just return that then.  I think that's more reasonable.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 26

[Boris Zbarsky [:bzbarsky]]

Fixed.  If someone feels a need for this to land on the 1.8 branch (so for Firefox 2), please request approval.

Comment 28

[Anne (:annevk)]

The specification http://dev.w3.org/cvsweb/~checkout~/2006/webapi/XMLHttpRequest/Overview.html?content-type=text/html;%20charset=utf-8 currently says that you have to set all members of the object to their initial values (excluding readyState). If you want this to change, by setting status to 500 for instance you should probably raise this on public-webapi.

Comment 29

[Boris Zbarsky [:bzbarsky]]

I don't think we really care so much what we return here.  I'm happy to return 0, as the spec would require.

We're going to need so many changes to implement that spec that it'll be more or less a wholesale rewrite of this file, though.  I figure once the spec is in CR and there are tests (so we can test said rewrite) we'll file a bug to implement that spec.

Comment 30

[Anne (:annevk)]

Ah, thanks for the heads up!