Skip to main content

Bug Tracker

Side navigation

#14683 closed bug (fixed)

Opened January 09, 2014 02:08PM UTC

Closed January 29, 2014 01:20PM UTC

ajax: Exceptions thrown synchronously by xhr.send are not caught

Reported by: jakob@gillich.me Owned by: jaubourg
Priority: blocker Milestone: 2.1.1
Component: ajax Version: 2.0.3
Keywords: Cc: jaubourg
Blocked by: Blocking:
Description

When an invalid url is passed to $.get, Chrome throws an NetworkError and no callbacks are called.

Expected result: Both fail() and always() should get called.

Code sample:

$.get('//example.com:80q').then(function () {
    console.log('not called');  
}).fail(function () {
    console.log('not called');  
}).always(function () {
    console.log('not called');  
});

http://jsfiddle.net/4FxSt/

Full error message:

XMLHttpRequest cannot load http://example.com:80q/. Cross origin requests are only supported for HTTP. jquery-2.0.2.js:7858
Uncaught NetworkError: A network error occurred. jquery-2.0.2.js:7321

Tested versions: 2.0.2, 2.0.3

Reproduced in: Chrome 31.0.1650.63

Couldn't reproduce in: IE10, Firefox 26

This does work as expected using jQuery 1.10.2, which makes me think it's an regression. http://jsfiddle.net/4FxSt/1/

Attachments (0)
Change History (14)

Changed January 09, 2014 11:33PM UTC by dmethvin comment:1

cc: → jaubourg
component: unfiledajax
priority: undecidedhigh
status: newopen

Agreed, this seems like it should call the error callback.

Changed January 09, 2014 11:34PM UTC by dmethvin comment:2

milestone: None2.next

Changed January 16, 2014 02:35PM UTC by njhamann comment:3

This appears because of the added onerror listener to xhr at https://github.com/jquery/jquery/blob/master/src/ajax/xhr.js#L110

Because this successfully fires (in chrome and safari) when trying an invalid url the try catch at https://github.com/jquery/jquery/blob/master/src/ajax.js#L649 throws an error instead of calling done() when chrome triggers a networkerror. Unfortunately throwing this error prevents the deferred callback from being called.

This can be fixed by delaying throwing the error by adding a setTimeout around throw e.

The alternative fix for this issue would be to remove the on error callback that is not consistently being called with all browsers or remove throw e line.

I will submit a pull request soon. Any feedback would be appreciated.

Changed January 16, 2014 03:00PM UTC by dmethvin comment:4

Awesome, thanks for the diagnosis!

Changed January 24, 2014 04:47PM UTC by njhamann comment:5

Changed January 24, 2014 06:14PM UTC by jaubourg comment:6

That's as edgy a case as I can imagine... and we tend to go by the "garbage in, garbage out" motto. The likelihood of such an invalid url not resulting from a programming error is very slim to say the least.

I'd vote against the "fix" but I'll let other weight in on this.

Changed January 24, 2014 06:22PM UTC by jakob@gillich.me comment:7

My use case was that to let the user configure the URL to an API and verify it by sending a simple GET request. Of course I could make sure the URL is valid first, but I expected jQuery to handle that (and I assume others do, too).

I can't comment on the fix though because I don't know a lot about the internals of jQuery.

Changed January 24, 2014 06:58PM UTC by jaubourg comment:8

Oh I wasn't implying your code was wrong or anything. It's just that it's a hack on top of a hack, albeit unavoidable given the goal you set here.

I'd need to check in the minutia of the XHR spec but if Chrome is going against the standard, then we can report a bug there and expect a fix in a timely fashion.

For the time being, you can catch the exception yourself (in parallel of an error handler) to cover your bases. Correct?

Changed January 24, 2014 07:18PM UTC by jaubourg comment:9

I can't reproduce in Chrome 32.0.1700.76 m nor in Chrome 34.0.1803.2 canary using this fiddle: http://jsfiddle.net/6Kjhr/1/

Am I "doing it wrong"(tm)?

Changed January 24, 2014 07:28PM UTC by scottgonzalez comment:10

_comment0: That fiddle is using 1.10.2, which the jakob said works. Look at the first fiddle (remove the /1).1390591771746553

That fiddle is using 1.10.2, which jakob said works. Look at the first fiddle (remove the /1).

Changed January 24, 2014 07:33PM UTC by jaubourg comment:11

/slapforehead :|

Yep, I can reproduce now: http://jsfiddle.net/6Kjhr/2/

Thanks Scott.

Changed January 24, 2014 08:34PM UTC by jaubourg comment:12

_comment0: tl;dr: this is a bug in Chrome but there's a clean workaround for the time being. \ \ Longer version... \ \ Confirmed this is a Chrome bug, see https://dvcs.w3.org/hg/xhr/raw-file/tip/xhr-1/Overview.html#the-open%28%29-method, point 9. \ \ Chrome fails to identify the URL as illegal and throws on `send` thinking it's a CORS violation. \ \ Interestingly, Chrome calls `onerror` before throwing, apparently not `onreadystatechange` hence the difference in behaviour between 1.x and 2.x. For the latter, `state` is too "advanced" when the desperate-try-catch-of-no-return is reached. jQuery thus thinks there's an exception in a callback attached to the internal Promise or in its own code, something we clearly don't wanna shadow. \ \ So the internal Deferred is resolved, but since everything happens synchronously, there's no time to attach the handlers. It works, however, if you use the callbacks options like so: \ \ {{{#!js \ $.ajax( "//example.com:80q", { \ success: function () { \ log( "success", arguments ); \ }, \ error: function () { \ log( "error", arguments ); \ }, \ complete: function () { \ log( "complete", arguments ); \ } \ } ); \ }}} \ \ Those are deprecated though but there's a way to use the Promise interface: `beforeSend`, which gives early access to the jqXHR instance: \ \ {{{#!js \ $.ajax( "//example.com:80q", { \ beforeSend: function( jqXHR ) { \ console.log( jqXHR ); \ jqXHR.done( function () { \ log( "done", arguments ); \ } ).fail( function () { \ log( "fail", arguments ); \ } ).always( function () { \ log( "always", arguments ); \ } ); \ } \ } ); \ }}} \ \ You can see it in action in http://jsfiddle.net/7LAVY/1390595713874596

tl;dr: this is a bug in Chrome but there's a clean workaround for the time being.

Longer version...

Confirmed this is a Chrome bug, see https://dvcs.w3.org/hg/xhr/raw-file/tip/xhr-1/Overview.html#the-open%28%29-method, point 9.

Chrome fails to identify the URL as illegal and throws on send thinking it's a CORS violation.

Interestingly, Chrome calls onerror before throwing, apparently not onreadystatechange hence the difference in behaviour between 1.x and 2.x. For the latter, state is too "advanced" when the desperate-try-catch-of-no-return is reached. jQuery thus thinks there's an exception in a callback attached to the internal Promise or in its own code, something we clearly don't wanna shadow.

So the internal Deferred is resolved, but since everything happens synchronously, there's no time to attach the handlers. It works, however, if you use the callbacks options like so:

$.ajax( "//example.com:80q", {
    success: function () {
        log( "success", arguments );  
    },
    error: function () {
        log( "error", arguments );  
    },
    complete: function () {
        log( "complete", arguments );  
    }
} );

Those are deprecated though but there's a way to use the Promise interface: beforeSend, which gives early access to the jqXHR instance:

$.ajax( "//example.com:80q", {
    beforeSend: function( jqXHR ) {
        jqXHR.done( function () {
            log( "done", arguments );  
        } ).fail( function () {
            log( "fail", arguments );  
        } ).always( function () {
            log( "always", arguments );  
        } );
    }
} );

You can see it in action in http://jsfiddle.net/7LAVY/

Changed January 25, 2014 07:31AM UTC by jaubourg comment:13

milestone: 2.next2.1.1
owner: → jaubourg
priority: highblocker
status: openassigned
summary: ajax: Invalid url triggers NetworkError in Chromeajax: Exceptions thrown synchronously by xhr.send are not caught

I re-qualified the bug.

There are other valid circumstances where xhr.send may throw synchronously (data that cannot be serialized being one of them) so, despite the fact there's a bug in Chrome in your specific use-case, we still have to protect against these exceptions.

Since this is an xhr issue, the fix should be in the xhr transport, see the following PR: https://github.com/jquery/jquery/pull/1497

Thanks a bunch for discovering this, Jakob.

Changed January 29, 2014 01:20PM UTC by jaubourg comment:14

resolution: → fixed
status: assignedclosed

Ajax: Protect against exceptions thrown synchronously by xhr.send

When xhr.send throws an exception synchronously, the onerror handler may have

been called already which, unchecked, makes the exception bubble up outside of

jQuery.ajax.

We now catch the exception pre-emptively and only rethrow if we know it hasn't

already been notified through the onerror handler.

Fixes #14683

Changeset: 01c360f96390ff16edfe65ef3b34e167087ef645