Skip to content
Permalink
Browse files
Add catch block to try/finally in deferred. Fixes #9033. Test case ne…
…eded.
  • Loading branch information
timmywil committed Jun 8, 2011
1 parent d66c3b6 commit 0a80be67f4fe968d99777564a02aeddbde1fbf35
Showing 1 changed file with 3 additions and 2 deletions.
@@ -58,8 +58,9 @@ jQuery.extend({
while( callbacks[ 0 ] ) {
callbacks.shift().apply( context, args );
}
}
finally {
} catch( e ) {
throw e;
} finally {
fired = [ context, args ];
firing = 0;
}

6 comments on commit 0a80be6

@jaubourg
Copy link
Member

@jaubourg jaubourg commented on 0a80be6 Jun 8, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed on purpose. Adding a catch block makes it impossible to debug in any browser. If IE8- complains, so be it.

See 5d9db48 and http://bugs.jquery.com/ticket/8353

The problem only occurs when an exception is thrown and only in IE. Better have an issue in IE rather than in every other browser. Would be a good idea to revert this.

@timmywil
Copy link
Member

@timmywil timmywil commented on 0a80be6 Jun 8, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't see it was added in the past. So even with throwing the error, other browsers won't see it?

@timmywil
Copy link
Member

@timmywil timmywil commented on 0a80be6 Jun 8, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it works ok. http://jsfiddle.net/timmywil/zWrJJ/1/

@jaubourg
Copy link
Member

@jaubourg jaubourg commented on 0a80be6 Jun 8, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, "it works". Even the try/finally construct will work flawlessly in anything but IE8- :P

Like I said, the issue is debugging: you loose context (original file and line of the exception) and everything seems to originate from the catch/throw block. It makes it a nightmare to debug and even confuses users who think the exception actually really occured within the deferred code itself.

@timmywil
Copy link
Member

@timmywil timmywil commented on 0a80be6 Jun 8, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yea, didn't think about line of exception. Tickets for this keep coming so I would love to have a solution by 1.6.2. Maybe we can do some brainstorming.

@dmethvin
Copy link
Member

@dmethvin dmethvin commented on 0a80be6 Jun 9, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the solution has to be that if you expect your callback to throw exceptions then you try/catch it inside the callback. If the problem is due to programming errors then it's debugger time, or you can just leave the try/catch in for production. I know it's an unfortunate problem since IE6/7 are browsers most likely to fail. But like @jaubourg says if we add the catch we totally screw up debugging in every other browser and hide problems.

/me shakes fist at IE7

Please sign in to comment.