Bug Tracker

Opened 11 years ago

Closed 11 years ago

#11743 closed bug (fixed)

jQuery silently ignores errors during script tag ajax request in $.appendTo()

Reported by: softwareelves@… Owned by: gibson042
Priority: low Milestone: 1.8
Component: ajax Version: 1.7.2
Keywords: Cc: jaubourg
Blocked by: Blocking:

Description

In the test case I'm including a javascript file via appendTo(). Allow errors that happen during the initial execution of the javascript are ignored because jQuery.ajax in jQuery.domManip is missing the error case to rethrow the error. For reference I was expecting it to act the same as if the script was loaded normally (uncomment to see).

http://pastebin.com/ZTHGF15k <~ test.html

http://pastebin.com/1vRfndNj <~ test.js

Change History (18)

comment:1 Changed 11 years ago by dmethvin

Resolution: worksforme
Status: newclosed

Looks like it works fine to me. http://jsfiddle.net/QYDZW/

I see an error on the console in IE9, Firefox, Safari and Chrome.

You mentioned "rethrow the error" but jQuery goes to great lengths to avoid any try/catch that might mask the browser details of script errors. If you have code that you think may throw an error as part of its normal behavior, you should wrap it in a try/catch yourself or use a window.onerror handler as a last resort.

comment:2 Changed 11 years ago by anonymous

Your jsfiddle does work as intended for me (not sure why it's different). I've hosted my pastebin on dropbox for easy testing and it does still fail for me.

http://dl.dropbox.com/u/9535/jquery/test.html

Thanks

comment:3 in reply to:  1 Changed 11 years ago by anonymous

Replying to dmethvin:

Looks like it works fine to me. http://jsfiddle.net/QYDZW/

I see an error on the console in IE9, Firefox, Safari and Chrome.

You mentioned "rethrow the error" but jQuery goes to great lengths to avoid any try/catch that might mask the browser details of script errors. If you have code that you think may throw an error as part of its normal behavior, you should wrap it in a try/catch yourself or use a window.onerror handler as a last resort.

I added a comment a little over a week ago showing that this bug still exists, but this ticket is still marked as closed. Should I reopen another ticket?

comment:4 Changed 11 years ago by dmethvin

The difference between yours and mine is that I'm using jQuery (edge) which is the latest development version. Can you try with http://code.jquery.com/jquery-git.js and see if it is fixed there? Since the fiddle works I suspect it does.

comment:5 in reply to:  4 Changed 11 years ago by anonymous

Replying to dmethvin:

The difference between yours and mine is that I'm using jQuery (edge) which is the latest development version. Can you try with http://code.jquery.com/jquery-git.js and see if it is fixed there? Since the fiddle works I suspect it does.

I've updated my example (http://dl.dropbox.com/u/9535/jquery/test.html) with your edge version, and can confirm that the bug still exists in that version (I'm on mac chrome). I've included a screenshot of where the exception is getting eaten: http://i.imgur.com/0sebs.png.

I know your time as a jQuery is very valuable, thanks for taking the time to look at my issue again. Let me know if there is anything else you'd like me to do.

comment:6 Changed 11 years ago by dmethvin

Resolution: worksforme
Status: closedreopened

Does this problem only occur on Chrome with the Mac? That would be very important to know.

comment:7 Changed 11 years ago by dmethvin

Owner: set to softwareelves@…
Status: reopenedpending

comment:8 in reply to:  6 Changed 11 years ago by anonymous

Replying to dmethvin:

Does this problem only occur on Chrome with the Mac? That would be very important to know.

It's also broken on Safari 5.1.7 Mac, FF3.6 Mac, FF12 Mac/PC, Chrome Win7, IE9 Win7 (which is all I've tested).

I've changed the example script slightly (http://dl.dropbox.com/u/9535/jquery/test.html) to display an alert after the appendTo call that shouldn't show (due to the error bubbling up), but in this case since the error is eaten, and the alert happens.

comment:9 Changed 11 years ago by dmethvin

Cc: jaubourg added
Owner: softwareelves@… deleted
Status: pendingnew

OIC what is going on, I'm sure jaubourg would have caught it immediately. My test case grabbed the script from another domain so it used a script transport (script tag). Yours uses the same domain so it's XHR. And yes, when an XHR script is processed the error is caught.

I've included jaubourg for some thoughts on how we can make this a bit more consistent and/or better ways to accomplish this. You could always create a script tag and set its src property.

comment:10 in reply to:  9 Changed 11 years ago by anonymous

Replying to dmethvin:

I've included jaubourg for some thoughts on how we can make this a bit more consistent and/or better ways to accomplish this.

Many thanks, I look forward to this issue being resolved :-)

You could always create a script tag and set its src property.

In my case I have arbitrary HTML I'm injecting into the <head> tag using jQuery, so that isn't an option for me. I just simplified the test case so that it'd be easier to diagnose.

comment:11 Changed 11 years ago by dmethvin

In my case I have arbitrary HTML I'm injecting into the <head> tag using jQuery

Do you mean you're expecting the script injection and execution to be synchronous? That won't happen, the request is async.

Unfortunately, the behavior of injected scripts isn't very well documented, but it's something we're trying to clarify for 1.8.

comment:12 in reply to:  11 Changed 11 years ago by anonymous

Replying to dmethvin:

In my case I have arbitrary HTML I'm injecting into the <head> tag using jQuery

Do you mean you're expecting the script injection and execution to be synchronous? That won't happen, the request is async.

Unfortunately, the behavior of injected scripts isn't very well documented, but it's something we're trying to clarify for 1.8.

It appears to be synchronous according to my example (http://dl.dropbox.com/u/9535/jquery/test.html). The first alert ("hi") is from the script being included and the second ("this should not show") is in the <script> tag directly following the appendTo call.

I believe this synchronous behavior is from script fetching code in domManip:

jQuery.ajax({
	type: "GET",
	global: false,
	url: elem.src, //this is the url from <script> tag in appendTo call
	async: false,
	dataType: "script"
});

It seems that this ticket bug actually steams from this code incorrectly handling the errors (silently ignoring them).

comment:13 Changed 11 years ago by dmethvin

Right, sync for XHR calls. I'll be quiet now and wait for professional help to arrive. :)

comment:14 Changed 11 years ago by jaubourg

We don't have much choice here... adding an error handler and rethrowing will be of no use (obfuscation).

We need yet another option in ajax in order not to catch exceptions in converters so that the exception in the text to script converter "bubbles up" so to speak. We could not catch when async is false but we would break existing code.

Seems like a good addition while handling the sourceURL issue.

comment:15 in reply to:  14 Changed 11 years ago by anonymous

Replying to jaubourg:

We don't have much choice here... adding an error handler and rethrowing will be of no use (obfuscation).

We need yet another option in ajax in order not to catch exceptions in converters so that the exception in the text to script converter "bubbles up" so to speak. We could not catch when async is false but we would break existing code.

Seems like a good addition while handling the sourceURL issue.

I like that solution, how would it work in the case of async or would it only be allowed for sync requests?

comment:16 Changed 11 years ago by gibson042

The script converter is a weird case anyway; it's not so much converting as doing work that elsewhere would be set up in a prefilter or transport. So what if we just delineate its special behavior and catch true conversion errors closer to their source?

https://github.com/jquery/jquery/pull/795

comment:17 Changed 11 years ago by mikesherov

Component: unfiledajax
Milestone: None1.8
Owner: set to gibson042
Priority: undecidedlow
Status: newassigned

comment:18 Changed 11 years ago by Richard Gibson

Resolution: fixed
Status: assignedclosed

Fix #11743: Don't mask script errors in jQuery.ajax, closes gh-795.

Changeset: 742872984e000ff8e13b9a23e74852d1b549f161

Note: See TracTickets for help on using tickets.