Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#10251 closed bug (wontfix)

an uncaught exception in one ready handler prevents all others from executing

Reported by: al_9x@… Owned by:
Priority: low Milestone: None
Component: misc Version: 1.6.3
Keywords: Cc: jaubourg
Blocked by: Blocking:

Description (last modified by Rick Waldron)

<script>
$(function() {
	alert('1st ready');
	throw "forced error";
});
</script>
<script>
$(function() {
	alert('2nd ready');
});
</script>

whereas when dealing with DOM directly, an uncaught exception in one DOMContentLoaded does not abort the others.

<script>
document.addEventListener("DOMContentLoaded", function() {
	alert('1st DOMContentLoaded');
	throw "forced error";
}, false);
</script>
<script>
document.addEventListener("DOMContentLoaded", function() {
	alert('2nd DOMContentLoaded');
}, false);
</script>

this forces dependencies, that need not exists, between often unrelated bits of code, making the page more brittle

Change History (17)

comment:1 Changed 7 years ago by addyosmani

Component: unfiledmisc
Priority: undecidedlow
Status: newopen

Confirmed. This also occurs with $.error: http://jsfiddle.net/st3Kj/1/, however I wonder if the reasoning behind this behaviour is that you're more likely to use a try/catch block with your throws (eg. this works fine http://jsfiddle.net/st3Kj/2/). Leaving open for further comment.

comment:2 Changed 7 years ago by dmethvin

Cc: jaubourg added
Keywords: 1.8-discuss added

I don't think we can avoid this unless we run all the handlers asynchronously and independently via setImmediate/setTimeout. If we try/catch then we destroy important error information.

comment:3 Changed 7 years ago by timmywil

-1, This is desired behavior imho.

comment:4 Changed 7 years ago by jaubourg

Description: modified (diff)

-1, This is prefect for debugging. If on handler throw an exception and it is expected behaviour, then it's easy enough to wrap it into a function with a try/catch.

comment:5 Changed 7 years ago by dmethvin

Description: modified (diff)

-1, Yeah, I agree with the OP that this is less than ideal, but the alternatives are less than idealer.

comment:6 Changed 7 years ago by mikesherov

Description: modified (diff)

-1, This sounds like a feature, not a bug. Uncaught exceptions should be handled eventually by the developer, no? What's the situation in which you would want the exception to go uncaught all the way up?

comment:7 Changed 7 years ago by timmywil

-1

comment:8 Changed 7 years ago by Rick Waldron

Description: modified (diff)

-1

comment:9 Changed 7 years ago by dmethvin

Keywords: 1.8-discuss removed
Resolution: wontfix
Status: openclosed

The best of the possible choices, unfortunately.

comment:10 Changed 7 years ago by nr@…

I do not know if my comment will be read because the bug is already closed. But there is a big draw back for "wontfix". You might consider the case where the executed function from the readyList unwillingly, unintentionally creates an exception. eg. due to a "null" dereferencing.

Unfortunatedly you will never notice this exception! NO error message is being created! eg. FF failes completely silent because the function is executed in the background - within the context of an event. Any exception thrown intentionally by the function is silently discarded, too.

Rather than forcing every added function to be wrapped inside a try{}catch{} block you might consider wrapping added function inside such a block on adding them: (jQuery 1.7)

--- jquery-1.7.1.js.orig	2012-02-21 13:44:42.313393019 +0100
+++ jquery-1.7.1.js	2012-02-21 14:01:02.683393031 +0100
@@ -276,7 +276,15 @@
 		jQuery.bindReady();
 
 		// Add the callback
-		readyList.add( fn );
+		readyList.add( function() { 
+                    try {
+                        fn.apply(this, arguments); 
+                    } catch(e) {
+                        // write to console log within another try{}catch{} because on IE using it failes 
+                        // whenever "developer tools" window is not open.
+                        try{console.log("error while executing document.ready function: " + e.message);} catch(e2){}
+                    }
+                });
 
 		return this;
 	},

Therefore I vote for fixing instead of ignoring to give the developer the chance to detect exception in his document.ready functions.

comment:11 in reply to:  10 Changed 7 years ago by jaubourg

Have you opened the console?

A very simple jsfiddle snippet actually does notify the exception in the console!

Replying to nr@…:

I do not know if my comment will be read because the bug is already closed. But there is a big draw back for "wontfix". You might consider the case where the executed function from the readyList unwillingly, unintentionally creates an exception. eg. due to a "null" dereferencing.

Unfortunatedly you will never notice this exception! NO error message is being created! eg. FF failes completely silent because the function is executed in the background - within the context of an event. Any exception thrown intentionally by the function is silently discarded, too.

Rather than forcing every added function to be wrapped inside a try{}catch{} block you might consider wrapping added function inside such a block on adding them: (jQuery 1.7)

--- jquery-1.7.1.js.orig	2012-02-21 13:44:42.313393019 +0100
+++ jquery-1.7.1.js	2012-02-21 14:01:02.683393031 +0100
@@ -276,7 +276,15 @@
 		jQuery.bindReady();
 
 		// Add the callback
-		readyList.add( fn );
+		readyList.add( function() { 
+                    try {
+                        fn.apply(this, arguments); 
+                    } catch(e) {
+                        // write to console log within another try{}catch{} because on IE using it failes 
+                        // whenever "developer tools" window is not open.
+                        try{console.log("error while executing document.ready function: " + e.message);} catch(e2){}
+                    }
+                });
 
 		return this;
 	},

Therefore I vote for fixing instead of ignoring to give the developer the chance to detect exception in his document.ready functions.

comment:12 Changed 7 years ago by anonymous

It's all a matter of timing. If the onload already has fired - as in your example - everything works find. But consider this: http://jsfiddle.net/uAHv6/2/

I have used the load event to even more delay the firing. Thus the function to execute onload is saved in the queue and then later executed on the background - that is the problem. Only functions executed from the ready queue are affected.

comment:13 Changed 7 years ago by nr@…

Sorry, again (i have the feeling to be not clear enough in my previous post) The timing is the key to trigger the error.

Using $(document).ready(): (a) If the onready event has already fired - the DOM-state is "ready" - then functions are not added to the onready-queue but executed immediately. (b) but if the DOM is NOT yet ready and the ready event has not yet been fired, the function is added to the ready-queue and then executed later, in the background.

Only (b) is affected by the described error - only in that case! So if you have a lightweight HTML page, you'll never fall over this error. Only if you have a heavy-weight page with a lot of HTML and JS you may suffer from it.

comment:14 Changed 7 years ago by anonymous

That sounds like an error in your code. The fiddle you provided uses $( document ).load which is incorrect - it should be using $( window ).load if you want to use the load event. http://jsfiddle.net/uAHv6/3/ throws an error as expected

Feel free to stop by the #jquery irc room on freenode to ask for help.

comment:15 Changed 7 years ago by anonymous

OK. It's hard to create a good test. So see this real-life page, where you can experience the error.

page A: https://transfer.nexxar.com/public/jquery-test/welcome-jquery-test.html page B: https://transfer.nexxar.com/public/jquery-test/welcome-jquery-test2.html

page B is correct and displays an allert. Secondly the accordeons at the right bottom are initialised and existing. page A is incorrect but neither display an error on the FF console nor does the alert appear. the document-ready function with the alert is never executed because the ready queue is abandoned.

Thze only difference between page A and page B is:

--- welcome-jquery-test.html	2012-04-03 23:59:56.000000000 +0200
+++ welcome-jquery-test2.html	2012-04-04 00:00:49.000000000 +0200
@@ -152,7 +151,7 @@
 $(document).ready(function() {
 
    var tmp = {};
-   tmp.dereference();
+//   tmp.dereference();
 
 });
 

The code with the alert is:

$(document).ready(function() {

   var tmp = {};
   tmp.dereference();

});


$(document).ready(function() {   
   alert("function executed after null dereference");      
});

comment:16 Changed 7 years ago by nr@…

Here is an improved jsfiddle test code that mimics the behaviour of the ready queue. The second function is NOT executed. http://jsfiddle.net/uAHv6/4/ Here it is executed http://jsfiddle.net/uAHv6/5/

comment:17 Changed 5 years ago by Igor Baltiyskiy

This is a bug. If uncontrolled code adds a handler before your code and fails, your handler will not be executed and you can't do anything about that.

The reason for assuming this bug a "feature" was that it is helpful during debugging. Isn't jQuery supposed to be a production-ready library? In my opinion, all debugging aids should be optional. How about turning the existing behaviour on with a switch?

Note: See TracTickets for help on using tickets.