Bug Tracker

Ticket #8744 (closed bug: cantfix)

Opened 4 years ago

Last modified 21 months ago

.ajax() jsonp requests are not handled correctly when hitting timeout

Reported by: Dynalon Owned by: jaubourg
Priority: high Milestone: 1.next
Component: ajax Version: 1.5.2
Keywords: jsonp timeout Cc: jaubourg
Blocking: Blocked by:

Description

The $.ajax() call does not handle cross-domain JSONP requests correctly if the request fails because of a timeout.

As soon as the timeout was reached, the corresponding callbacks error/complete/success are fired correctly. However, the xhr is not canceled and after response (even if its too late now) jQuery jumps into the callback handler of the jsonp, even so the function does not exists (anymore). You will hit an error like "jQuery152008986116735648775_1301675055297 is not a function" in your error console.

(quite) minimal testcase which will always timeout:

function doTest() {
    $.ajax({
        url: 'http://jsfiddle.net/echo/jsonp/',
        timeout: 5,
        dataType: "jsonp",
        data: { echo: "Hello World!" },
        error: function(jqxhr, textStatus) {
            console.log("failed with error: " + textStatus);
        },
        success: function(data, jqxhr, textStatus) {
            alert(data.echo);   
        }
    });
}
$("document").ready(doTest);

// attention: you cannot run that  testcase on jsFiddle because
// it will then not be a cross-domain request!

If you set the timeout in the code above to a reasonable timeframe, i.e. 5000, the cross-domain request will finish properly.

Expected behaviour: jQuery should correctly cancel the XHR when hitting the timeout.

Tested Browser: Firefox 4 and Opera 11 are both affected, Safari 5 however seems not to be (all tested on Mac).

Change History

comment:1 Changed 4 years ago by timmywil

  • Cc jaubourg added
  • Keywords jsonp timeout added
  • Component changed from unfiled to ajax

Unfortunately, and jaubourg can correct me on this, but I don't think it can be cancelled since jsonp is not xhr.

comment:2 Changed 4 years ago by Dynalon

I've digged into this a little more and yes, it seems the problem is not easily solveable. The JSONP request is done via injection of a <script> tag into the DOM and I don't think (?) there is a way to cancel the request once its triggered. The problem is, the callback function is removed from the window context by jquery right after hitting the timeout. If the request does finish later, the jsonp response is run by the browser as a standalone-script and thus first hits an undefined function, causing the "unknown function" exception.

However, it turns out that this bug is a really minor one. As the exception is thrown in the standalone script of the jsonp response, it should not affect any other scripts running, causing no damage at all except for the error messge in the error console.

I've come up with a simple hack in jquery so that the callback function is not cleaned up and remains within the window object. This resolves the problem but is surely no feasible solution for upstream since the window object would be cluttered with (afterwards useless) functions over time when doing lots of jsonp requests.

To have this completeley fixed a better way for callback handling would be needed, but considering that the bug itself doesn't raise serious issues I think its not worth the effort.

comment:3 Changed 4 years ago by jaubourg

  • Owner set to jaubourg
  • Status changed from new to assigned

I find this behaviour very strange. I'm not on my dev box right now but afaik, jqXHR.abort (which is used by timeout) will remove the script tag from the document... so is it that FF4 and Opera 11 don't cancel script loading in that case or is something wrong in the script tag removal code?

I'll look into this on my dev box once I'm back on it.

Last edited 4 years ago by jaubourg (previous) (diff)

comment:4 Changed 4 years ago by jaubourg

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixes #8744. Makes sure script transport abort method actually removes the script tag even if readyState exists.

Changeset: 2ed81b44be958b5f2b5569ab15f22bde262b4eb6

comment:5 Changed 4 years ago by jaubourg

Dynalon, can you confirm this fixes your issue?

comment:6 Changed 4 years ago by jaubourg

  • Milestone changed from 1.next to 1.6

comment:7 Changed 4 years ago by Dynalon

Yes, I confirm with the recent git version the problem is gone, bug is fixed. Thanks!

comment:8 Changed 4 years ago by Dynalon

Stop, that one was to quick: The Bugfix in Git works for Opera 11 (Mac & Linux tested), but is still present in Firefox 3.6.13 (Linux) and Firefox 4 (Mac). So the Fix does not seem to work on Firefox at all.

comment:9 Changed 4 years ago by timmywil

I think jaubourg had the right idea about firefox script loading. Once the script tag is injected, I believe it loads the script regardless of whether the tag gets removed before completion. Another thought I had was that I think async is set to true by default for injected script tags in Firefox 4. I'm not sure if that could have an impact on removals. Again, I defer to jaubourg on these issues.

comment:10 Changed 4 years ago by jaubourg

Dynalon: could you set a test case on jsFiddle (calling the twitter public api or whatnot). I'd like to see this with my own eyes because, if that's the case and what timmywil says is right then it's clearly a bug in Firefox. I highly doubt it though, so I'll need a test case clearly showing the behaviour to re-open this (are you sure you didn't have some caching issues when you tested in FF?).

comment:11 Changed 4 years ago by Dynalon

Yes, actually it seems if you enforce dataType:jsonp as ajax option, the bug appears no matter if cross-domain or not. So I now have set up a testcase on jsFiddle:

 http://jsfiddle.net/XtVWT/

Please note the HTML Commentin jsFiddle, it seems there is a problem with git-edge on jsFiddle / code.jquery.com/jquery-git.js

As already said, with you fix in jquery-git the Bug does not occur in Opera, but is still present in FF when testing with this fiddle.

Last edited 4 years ago by Dynalon (previous) (diff)

comment:12 Changed 4 years ago by jaubourg

  • Keywords needsdocs added

OK, so I checked this under FF and it seems timmy is right: once the script tag has been appended, there seems to be no way to cancel loading. This is quite daunting seeing as it means there is no mean to abort cross-domain script requests in FF. I tried with and without async to no avail :(

This is clearly a bug in the browser. We need to document the issue.

comment:13 Changed 4 years ago by john

  • Priority changed from undecided to high

comment:14 Changed 3 years ago by addyosmani

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone 1.6 deleted

comment:15 Changed 3 years ago by addyosmani

@jaubourg: as this ticket has been open for two months, can you re-confirm whether it's been fixed or if any fixes are currently being worked on for it? thanks!

comment:16 Changed 3 years ago by jaubourg

It's a FF bug. The script tag is removed, yet the script is still loaded and executed... didn't check if this was fixed in FF5.

comment:17 Changed 3 years ago by timmywil

  • Status changed from reopened to open

comment:18 Changed 3 years ago by jaubourg

#9687 is a duplicate of this ticket.

comment:19 Changed 3 years ago by addyosmani

  • Keywords neededdocs added; needsdocs removed

comment:20 Changed 3 years ago by john

  • Milestone set to 1.next

comment:21 Changed 3 years ago by jaubourg

#9782 is a duplicate of this ticket.

comment:22 Changed 3 years ago by timmywil

#9853 is a duplicate of this ticket.

comment:23 Changed 3 years ago by blair

When i load this jsfiddle code:  http://jsfiddle.net/XtVWT/ I always get an error in IE9 stating "Object expected". Clicking debug error points me to this line: jQuery1604361509936638372_1322095521345({"_": "1322095521358", "echo": "Hello World!"});

This is a huge issue for us and hope it can get fixed soon.

comment:24 follow-up: ↓ 25 Changed 3 years ago by Dynalon

@blair: the jsFiddle you posted uses an old version of jquery, because at the time i reported the bug, jsFiddle has had issues with using jquery-edge, which is was likely fixed weeks ago.

I set up a new fiddle here:  http://jsfiddle.net/HEsL2/ which you can use to test against jquery-edge. Can you confirm IE9 is affected, and if so, whether or not you are getting the "failed with error: timeout" message?

If you get the "right" error message that means the error() event is correctly triggered when hitting the timeout. If thats the case, the bug should not be a huge issue as the code will work as expected - the only drawback is the somewhat confusing error message that is displayed on the error console.

comment:25 in reply to: ↑ 24 Changed 3 years ago by blair

@Dynalon

With your new example I still get the "Object Expected" error in IE9:

jQuery1725116137489204307943349415308069850127750664_1322114277554({"_": "1322114277629", "echo": "Hello World!"});

Also note that IE9 does not print its errors to the console, the error comes in the form of a modal alert dialog. This is a critical error that should be fixed asap as users with error messaging enabled in IE9 will always see this issue if the timeout hits, providing a less than ideal user experience.

comment:26 Changed 3 years ago by Dynalon

so you say IE9 will fail completely if the timeout hits? I agree, in this case this is a major issue. However, the whole thing isn't really jquery's fault, but specific to the browser implementations. To really fix the bug, one must nag the IE & FF developers to fix this in their code.

I don't think there is a *good* way to work around this in jquery for those browsers. The only workaround I came up with, is to prevent jquery from deleting the jsonp callback function when the timeout is encountered. Instead, upon timeout hit, the callback function is replaced by an empty dummy function which does nothing. So if - at a later time- the ajax request completes (even if the timeout was hit already) the browser actually finds the function.

This is UGLY AS HELL, as references to the callback functions are stored PERMANENTLY in the window object, thus cluttering the heap more and more everytime you do a jsonp request that exceeds its timeout.

If you can live with that and only do a limited amount of requests on a page, I've uploaded a patch that implements this workaround, grab it here:  http://www.stud.uni-karlsruhe.de/~urblr/jquery-ugly/workaround-bug8744.diff

To test jquery with applied patch see:  http://jsfiddle.net/D5YUZ/ At least it fixes the issues in FF for me (can't test in IE since I don't have a windows box around).

Once again, this is not a good solution and the patch is not meant to be put into upstream jquery, so you've been warned!

comment:27 Changed 3 years ago by anonymous

Dynalon thanks for the reply, to clarify, the error callback is called into, but in addition, there is the "Object expected" error alert.

comment:28 Changed 3 years ago by anonymous

Dynalon, your patch also fixes IE9 from throwing the "Object expected" error. It also correctly executes the ajax error callback.

I agree that cluttering the global namespace with callback functions is not a good solution, wondering if there is a better way.

comment:29 Changed 3 years ago by jaubourg

Yeah, this is a nasty nasty bug but Dynalon is spot on: the right fix is browser-side :(

comment:30 Changed 3 years ago by Dynalon

Turns out the bug is known since 2007 in FF but due to lack of reporter corresponding it never got fixed:  https://bugzilla.mozilla.org/show_bug.cgi?id=394908 Also, the Dojokit is aware of this bug, see:  http://archive.dojotoolkit.org/nightly/dojotoolkit/dojo/tests/io/scriptTimeout.html

I've resurrected the bugreport in mozillas bugzilla and hopefully it can be worked out in FF.

Since I have no access to a windows machine to verify IE8/9 behaviour, could someone with windows report that bug to Microsoft and have them work it out?

comment:31 follow-up: ↓ 32 Changed 3 years ago by BennyTheSnitch

The following work-around was suggested a few months back in a different post:

window.jQuery16207753935731721285_1310374972748 = function() {

    delete window.jQuery16207753935731721285_1310374972748;

}

Although the global namespace would still be littered with redundant callbacks at least the ones that DO eventually return would be removed.

comment:32 in reply to: ↑ 31 Changed 3 years ago by jaubourg

Replying to BennyTheSnitch:

The following work-around was suggested a few months back in a different post:

window.jQuery16207753935731721285_1310374972748 = function() {

    delete window.jQuery16207753935731721285_1310374972748;

}

Although the global namespace would still be littered with redundant callbacks at least the ones that DO eventually return would be removed.

On browsers that do handle removing a script tag properly or, in all browsers when you have a server error or the jsonp response is illformed and never calls the function, these would stay in the global namespace.

So it's still not an acceptable solution.

comment:33 Changed 3 years ago by anonymous

Hi, what is the best workaround for this issue?

comment:34 Changed 3 years ago by e.marin.izquierdo@…

My solution:

 // JSON Request
   var auxTime = new Date();
   var jQueryCallbackRandom = auxTime.getTime();
		
   var callParameters = {
      url: myUrl,
      dataType: "jsonp",			
      jsonp: "myServerParamCallback",
      jsonpCallback: "jQueryRandom_" + jQueryCallbackRandom,
      crossDomain : true,
      success: f_success,
      timeout: myCriticTimeOut,
      error: function(jqXHR, textStatus){
         console.log("failed with error: " + textStatus);
         window["jQueryRandom_" + jQueryCallbackRandom] = function( {window["jQueryRandom_" + jQueryCallbackRandom] = null;};
      },
      data: params			
   }
   
   $.ajax(callParameters);
	

comment:35 Changed 2 years ago by anonymous

I've found this to still be an issue on at least IE9 and Windows Chrome. Replies #26, #32, and #34 successfully address it, but all will leave empty functions... Because I don't want to amend jQuery as in #26, I modified the answers in #32 and #34, see the following jsfiddle. Also, this doesn't require pre-defined jsonpCallback as in #34. Note this still leaves empty functions in the namespace...

 http://jsfiddle.net/2qnrU/

Note that there's probably a there a better way to grab the pre-defined AJAX callback? I just found the callback as a property of the object, so I pulled the name with a for-in loop.

comment:36 Changed 2 years ago by anonymous

Sorry I didn't realize using a # sign triggered links to tickets. I was referring to comment numbers.

comment:37 Changed 23 months ago by dmethvin

  • Keywords neededdocs removed
  • Status changed from open to closed
  • Resolution set to cantfix

It's pretty clear we can't make this work cross-browser, it will have to be solved by the browser makers. some workarounds are listed above. The related Firefox ticket is here:

 https://bugzilla.mozilla.org/show_bug.cgi?id=707154

Docs ticket is here:

 https://github.com/jquery/api.jquery.com/issues/235

Please don't add more comments to the ticket unless they contribute significantly to the discussion.

comment:38 Changed 21 months ago by markelog

#13693 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.