When an error thrown from ready handler all subsequent handlers are not being invoked #1823
7 participants
|
What would you want us to do with the error once it's caught? |
|
We could force the ready handlers to be called asynchronously, ie replace https://github.com/jquery/jquery/blob/master/src/core/ready.js#L12 with: jQuery.ready.promise().done( function() {
setTimeout( function() {
fn.call( document, jQuery );
} );
} );But it won't solve the problem if the functions are added on the promise directly. Note that using Standard Promises would be even worse, a handler throwing an exception would fail silently and nothing else would break making it quite tricky to know where the bug is. Also, the previous implementation (<1.5) which would go through an array of function and call them sequentially did behave exactly the same way except when adding a function after the document is ready. |
|
Agreed, this is far from an obvious fix. If we catch the error we have to report it somehow, otherwise we're masking the problem. |
|
Apologies for improper test case earlier..let me give you exact use case here Example with jquery2.1.1 Example with jquery1.5(where it works fine) yes, jquery should not suppress the error. It may be correct to not execute the pending handlers(in the queue at that time) when the first handler throws error. But that should not cause a problem for newly registered handlers. In the above example ready handler is added on click of a button. It works fine on jquery 1.5 but fails with jquery 2.1.1. it looks like 1.5 uses finally block to reset "firing" flag. Is it possible to have similar fix in 2.1.1?. Thank you for looking in to this. |
|
Yes, I think it should be possible to restore the |
|
Well, you know how it pained me we had to remove this |
|
How so? According to |
|
@mzgol the problem was that IE7 required |
|
Yeah and rethrowing in the catch ended up in losing the actual context of the original exception. |
|
Ah, right. It's good we're dropping IE<8 then! |
|
Nice! Have a beer on me with @changetip |
|
For posterity, the behavior change here is that ready handlers are now executed async. In other words, if any code after adding a ready handler expected the handler to be executed synchronously |
Jquery version: 2.1.1
Browser: on any browser:
Testcase:
Please take a look at this jsfiddle
Example with jquery2.1.1
http://jsfiddle.net/q7gbzrbu/2/
Example with jquery1.5(where it works fine)
http://jsfiddle.net/q7gbzrbu/1/
[Edit: OLD and improper test case] http://jsfiddle.net/8b3a92yo/1/
Problem:
When an error is thrown from one of the handlers registered through $(docuement).ready() then all subsequent handler are not being invoked.
Addl info:
This is what happening when an error is thrown from a handler function which is registered with $.ready(). jQuery.Callbacks.fire()(refer line 3073 in jquery-2.1.1.js) will invoke the user's ready Handler function. Below is related piece of code. When an error was thrown, "firing" flag will not be reset to false. Due to this any handlers registered after error will not be triggered.(refer line 3113 in jquery-2.1.1.js)
firing = true;
for ( ; list && firingIndex < firingLength; firingIndex++ ) {
if ( list[ firingIndex ].apply( data[ 0 ], data[ 1 ] ) === false && options.stopOnFalse ) { //this statement invokes our ready handler
memory = false; // To prevent further calls using add
break;
}
}
firing = false;
It looks like "firing" flag need to be reset in a finally block.
Workaround:
Obvious workaround is handler code need to be wrapped with try/catch to avoid this issue.