Last Comment Bug 304980 - nsIXMLHttpRequest throws exception on access of status member when the request failed due to a timeout
: nsIXMLHttpRequest throws exception on access of status member when the reques...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XML (show other bugs)
: Trunk
: x86 All
: -- normal with 5 votes (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Ashish Bhatt
:
Mentors:
: 366930 (view as bug list)
Depends on: 238559
Blocks: 480200
  Show dependency treegraph
 
Reported: 2005-08-17 09:17 PDT by Willem Joosten
Modified: 2016-08-21 17:58 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Possible fix (1.21 KB, patch)
2005-11-28 17:23 PST, Boris Zbarsky [:bz] (still a bit busy)
darin.moz: review+
bzbarsky: superreview-
Details | Diff | Splinter Review
Updated patch (1.47 KB, patch)
2005-12-09 16:29 PST, Boris Zbarsky [:bz] (still a bit busy)
jst: superreview+
Details | Diff | Splinter Review
Updated to comments (1.42 KB, patch)
2006-01-20 08:36 PST, Boris Zbarsky [:bz] (still a bit busy)
darin.moz: review+
Details | Diff | Splinter Review

Description Willem Joosten 2005-08-17 09:17:44 PDT
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 2005-08-18 10:38:08 PDT
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 2005-08-26 14:23:27 PDT
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 2005-09-12 06:00:38 PDT
This bug still occurs in FireFox 1.5 Beta 1.
Comment 4 Stephen C 2005-11-25 06:06:08 PST
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 [:bz] (still a bit busy) 2005-11-28 17:23:12 PST
Created attachment 204399 [details] [diff] [review]
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 2005-11-29 17:01:29 PST
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 [:bz] (still a bit busy) 2005-11-29 17:49:57 PST
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 2005-11-29 18:11:58 PST
> 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 [:bz] (still a bit busy) 2005-11-29 18:20:52 PST
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 2005-11-29 18:28:42 PST
> 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 [:bz] (still a bit busy) 2005-11-29 18:36:29 PST
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) 2005-11-30 14:35:33 PST
onStopRequest will always be called if asyncOpen returned success. so, yes.
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2005-12-09 16:29:07 PST
Created attachment 205440 [details] [diff] [review]
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, jst@mozilla.com) 2005-12-09 16:44:08 PST
Comment on attachment 205440 [details] [diff] [review]
Updated patch

sr=jst
Comment 15 Darin Fisher 2005-12-13 10:07:52 PST
>  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 [:bz] (still a bit busy) 2005-12-23 11:38:16 PST
I could just return NS_ERROR_NOT_AVAILABLE as the int, sure.  Want me to?
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2005-12-23 11:39:00 PST
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 2006-01-10 16:01:57 PST
A WG could easily just say that the useragent will throw an error and not specify the error, right?
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2006-01-10 18:25:59 PST
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 2006-01-10 18:36:20 PST
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 [:bz] (still a bit busy) 2006-01-10 19:02:42 PST
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 2006-01-10 19:20:29 PST
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 [:bz] (still a bit busy) 2006-01-10 19:27:14 PST
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 2006-01-19 11:06:47 PST
Yes, let's just return that then.  I think that's more reasonable.
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2006-01-20 08:36:23 PST
Created attachment 209107 [details] [diff] [review]
Updated to comments
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2006-01-20 13:34:17 PST
Fixed.  If someone feels a need for this to land on the 1.8 branch (so for Firefox 2), please request approval.
Comment 27 Phil Ringnalda (:philor) 2007-01-13 15:03:35 PST
*** Bug 366930 has been marked as a duplicate of this bug. ***
Comment 28 Anne (:annevk) 2007-01-14 03:37:29 PST
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 [:bz] (still a bit busy) 2007-01-14 22:46:47 PST
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) 2007-01-15 04:39:42 PST
Ah, thanks for the heads up!

Note You need to log in before you can comment on or make changes to this bug.