Bug Tracker

Modify

Ticket #11743 (closed bug: fixed)

Opened 2 years ago

Last modified 23 months ago

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
Blocking: Blocked by:

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

comment:1 follow-up: ↓ 3 Changed 2 years ago by dmethvin

  • Status changed from new to closed
  • Resolution set to worksforme

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 2 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 2 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 follow-up: ↓ 5 Changed 2 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 2 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 follow-up: ↓ 8 Changed 2 years ago by dmethvin

  • Status changed from closed to reopened
  • Resolution worksforme deleted

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

comment:7 Changed 2 years ago by dmethvin

  • Owner set to softwareelves@…
  • Status changed from reopened to pending

comment:8 in reply to: ↑ 6 Changed 2 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 follow-up: ↓ 10 Changed 2 years ago by dmethvin

  • Cc jaubourg added
  • Owner softwareelves@… deleted
  • Status changed from pending to new

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 2 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 follow-up: ↓ 12 Changed 2 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 2 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 2 years ago by dmethvin

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

comment:14 follow-up: ↓ 15 Changed 2 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 2 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 2 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 2 years ago by mikesherov

  • Owner set to gibson042
  • Priority changed from undecided to low
  • Status changed from new to assigned
  • Component changed from unfiled to ajax
  • Milestone changed from None to 1.8

comment:18 Changed 23 months ago by Richard Gibson

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

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

Changeset: 742872984e000ff8e13b9a23e74852d1b549f161

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.