Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#11193 closed bug (duplicate)

jQuery.Callbacks left in inconsistent state after exception

Reported by: arnaud.lb@… Owned by: arnaud.lb@…
Priority: low Milestone: None
Component: event Version: 1.7.1
Keywords: Cc:
Blocked by: Blocking:

Description

If an exception occurs in a callback, the jQuery.Callback instance is left in an inconsistent state; as a result fire() and add() won't call anything after that, because 'firing' is true.

It seems that exceptions could be caught and re-thrown after firing has been set to false.

Change History (8)

comment:1 Changed 7 years ago by addyosmani

Component: unfiledevent
Owner: set to arnaud.lb@…
Priority: undecidedlow
Status: newpending

Thanks for submitting a ticket to the jQuery bug tracker. Could you please include a test case on jsfiddle.net that reproduces the issue you're experiencing with $.Callbacks?

comment:2 Changed 7 years ago by anonymous

Sure, here is the jsfiddle: http://jsfiddle.net/2G23g/2/

comment:3 Changed 7 years ago by jaubourg

Resolution: duplicate
Status: pendingclosed

It's not really a duplicate of #9033 per se, but all the info and discussion are there: Callbacks and Deferred objects are locked because:

  1. Catching rethrowing makes debugging impossible in IE
  2. try/finally is not supported in all IE
  3. the error should be notified and as vocal as possible (you do want the exception to halt the program somehow in order to be able to debug).
  4. If you want to treat the exception, do it in the callback itself (basic locality principle).

comment:4 Changed 7 years ago by jaubourg

Duplicate of #9033.

comment:5 Changed 6 years ago by dmethvin

#12884 is a duplicate of this ticket.

comment:6 Changed 6 years ago by dmethvin

#14201 is a duplicate of this ticket.

comment:7 Changed 5 years ago by spicyj

I just ran into this. Any chance we could reopen and fix this for jQuery 2.x? IE8+ supports try/finally properly so there shouldn't be a problem. In React we have a similar wrapper for processing a list of callbacks where we run every callback but make sure the first error bubbles up to aid debugging:

https://github.com/facebook/react/blob/master/src/utils/Transaction.js#L230-L260

If it would be helpful I'd be happy to submit a patch to fix this.

comment:8 Changed 5 years ago by dmethvin

Can you open a new ticket and reference this one? We will be dropping IE 6/7 support soon so we could make a case for it now. Thanks!

Note: See TracTickets for help on using tickets.