Event emitter calls listeners immediately, rather than on nextTick

[toejough]

This seems to have been an intentional design decision, but if so, I've been unable to find any commentary. Also, if so, then this is really a request for a new API, or at least some documentation of best practices for event emitters.

My mental model for emitting an event is that "emitting" is independent of the listener "hearing" and acting. In concrete terms, when code looks like this:

foo.on('event', function() { console.log('got event'); });
console.log('emitting event');
foo.emit('event');
console.log('event emitted');

I expect the outcome to be:

emitting event
event emitted
got event

Instead, it's

emitting event
got event
event emitted

Rather than being "emit my event", the present behavior is actually "execute arbitrary code". That seems like broken behavior, and I know from conversations with other developers and inspection of library code that I'm not alone in my expectation of "emit" being independent from listener execution.

The only place I've seen the current behavior become a problem is when you are expecting state to be safe on the other side of an 'emit' call:

/* user code */
foo.on('announcement', function () { //trigger complex action which eventually destroys foo.state });

/* library code */
foo.state = { key: value};
foo.emit('announcement', 'I have state!');
// all I did was announce - expecting state to be the same
console.log(foo.state.key); // EXPLODE (because a user listener caused the state to be destroyed)

This is a trivial example, but I've actually run into a more complex situation crossing multiple libraries and emitted events which eventually boiled down to this. The library I was using got a socket error, seemingly in the middle of executing a synchronous function, which I didn't think was possible. In the socket.on('error', ...) callback, the library cleaned up its state before returning to the prior execution flow (which depended on that state). I was totally confused until I debugged the actual execution flow and realized that it was all due to events emitting events emitting events, and that all of that happens on this tick.

I'll be submitting a bug/PR to the library's maintainers, but this behavior was so counter-intuitive to me that I had to submit a bug with node, as well.

So, is this purposeful behavior? If not, could it be changed, or a new "emitNext" be added? Either way, could some documentation be added explaining the risks and providing some best-practices?

[chrisdickinson]

Edit: my finger slipped on submit -- apologies! Hold on a second, I have a bit more to write up :)

Emitting an event on an emitter instance means, in part, to execute all of the emitter's registered listeners for that event, sequentially. This is one of the oldest patterns Node uses, predating streams, the modern require function, and even narrowly predating promises (before node standardized again around what would become "node style callbacks".) The behavior of event emitters has been relatively (remarkably) stable compared to all other abstractions in Node. They're powerful, and as the oldest stable abstraction, have reached all throughout Node. It is unlikely that the behavior will change.

Rather than being "emit my event", the present behavior is actually "execute arbitrary code". That seems like broken behavior, and I know from conversations with other developers and inspection of library code that I'm not alone in my expectation of "emit" being independent from listener execution.

"emitting" means to "execute all listeners for this topic on this emitter with these arguments". Event emitters are not a flow control pattern -- they don't involve the event loop at all. It's sort of an application of the Observer pattern in JavaScript.

[chrisdickinson]

Sorry again for the accidental early submission. Continuing...

Re: emitNext, the current accepted way of doing performing that action would be to write the following:

setImmediate(function() {
  eventEmitter.emit('topic', args)
})

---

I vehemently agree that there should be more documentation on EventEmitters practices -- they are a powerful footgun. At present, however, Node's documentation is heavily (nearly exclusively!) reference-focused. Reference docs are random-access. There's no guarantee that the reader is consuming the document linearly, and thus it's hard to ensure that necessary information is conveyed. I'm leery of interleaving best-practice documentation with the existing reference docs -- there's a good chance people who need them won't see them, and people who don't need to see them will get bogged down by them. I'd like to revisit this when we get a place for non-reference docs, which I believe some folk are working towards.

[toejough]

Thanks for the quick response!

Heh, I like the 'vehement' agreement about the documentation. I don't mind where it goes, so much as I am looking for more than I've found. I did go digging (maybe not well?), and came up with a really nice article on error handling best practices, but not on emitted events. Do you know of any? If people are working on this...fantastic!

I've started to use process.nextTick, rather than setImmediate - is there a good reason to use one over the other in this scenario?

[trevnorris]

EventEmitter was made to be used as an abstraction around asynchronous events. Thus, when an event is emitted it will have happened on a different tick than when the event was set. So it would be disadvantageous if the asynchronous event was then again asynchronously emitted. Unnecessary resources would be used, and too many events remain in-flight.

A specific case is in the emitting of 'error'. Generally when an error is emitted it should be immediately dealt with before execution continues. Also by immediately executing the callbacks in the events queue the user does not loose the call stack. Again, important for errors.

As for the differences between process.nextTick() and setImmediate():

process.nextTick() should be used only when a callback needs to be called before the event loop continues. For example, when setting up an event emitter and its listeners nothing should be emitted immediately, but likewise it may be necessary to have an event handled before the event loop continues.

setImmediate() is probably the correct choice 99% of the time. This will not call the callback until a later phase of the event loop. It has the added advantage of accepting optional arguments that are passed to the callback. Making it easier to hoist functions.

Another reason is to ensure resources are properly cleaned up. This is actually an edge case issue in Node that still needs to be fixed. Here's an example:

(function createServer() {
  require('net').createServer()
    .on('close', createServer)
    .listen(8080, function() { this.close(); });
}());

Because the 'listen' and 'close' events are emitted in a process.nextTick(), the event loop is never allowed to continue. This prevents libuv from being able to clean up the resources allocated when the server was created. Hence it will cause memory to fill, since the memory retained is not within the V8 heap.

Hopefully that sheds some light. @chrisdickinson Feel free to throw this into some documentation somewhere. ;-)

[toejough]

Ah, yes it does, thank you!

[cjihrig]

@chrisdickinson think this can be closed now?

[chrisdickinson]

Yep! Closing now.