"TypeError: Handler is not a function" when unsubscribing listeners inside of a callback #1

davethegr8 · 2014-03-03T19:46:11Z

in src/pubsub.js at line 30, you are assigning i and l before the initialization of the triggering loop, leading to a case where you can cause a fatal TypeError if your handlers unsubscribe function from the same event that cause them. This can be problematic if you use logic control to define which callbacks to execute when.

    for (var i = 0, l = handlers[event].length; i < l; i++) {
        handlers[event][i].apply(ctx, ctx.args);
    }

This code reproduces the issue. Notice that "Done." is never logged.

PubSub.subscribe('test', function () {
    console.log('function 1 called');
});

PubSub.subscribe('test', function () {
    console.log('function 2 called');
    console.log('removing listeners for "test"');
    PubSub.unsubscribe('test');
});

PubSub.subscribe('test', function () {
    console.log('function 3 called');
});

PubSub.publish('test');

console.log('Done.');

The text was updated successfully, but these errors were encountered:

bdadam · 2014-03-04T19:28:10Z

Hey Dave,

Thanks for your report. Actually it is not a bug in the module per se. Everything works as it was designed.

What here happens is when you call PubSub.unsubscribe('test'); it throws a TypeError. Since it is never handled, this Exception bubbles up to PubSub.publish('test'); and prohibits the code to run any further. So the last line never gets called.

Where this behaviour might be bad is at the publisher's side. Why should the publisher get an exception, when one of the subscribers supplies a crappy function...

I'll take another look at this issue tomorrow. I'm not really sure right now what should be done in this situation.

davethegr8 · 2014-03-05T23:25:58Z

What here happens is when you call PubSub.unsubscribe('test'); it throws a TypeError.

Oh strange... I hadn't noticed that.

This is definitely more a concern for people using the library. I check for this kind of behavior in pubsub systems since it seems to cause more gotchas than other parts.

I've seen two ways of handling this kind of behavior: either using setTimeout(..., 0) to queue up the callbacks or not initializing l = handlers[event].length. I personally prefer the former since you guarantee that all of the callbacks that existed when the event was triggered will get called.

bdadam · 2014-03-07T06:50:22Z

setTimeout was also my first thought. But I think it would be a breaking change for the lib.
But let's see. I hope I can come to this at the weekend.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"TypeError: Handler is not a function" when unsubscribing listeners inside of a callback #1

"TypeError: Handler is not a function" when unsubscribing listeners inside of a callback #1

davethegr8 commented Mar 3, 2014

bdadam commented Mar 4, 2014

davethegr8 commented Mar 5, 2014

bdadam commented Mar 7, 2014

"TypeError: Handler is not a function" when unsubscribing listeners inside of a callback #1

"TypeError: Handler is not a function" when unsubscribing listeners inside of a callback #1

Comments

davethegr8 commented Mar 3, 2014

bdadam commented Mar 4, 2014

davethegr8 commented Mar 5, 2014

bdadam commented Mar 7, 2014