Don't emit end events after error events

[evansolomon]

I always find it a little surprising/frustrating that Query objects (and maybe other types) can emit both 'error' and 'end' events. In most event emitters 'error' events are also effectively end events. I often end up with patterns like this.

var emitter = makeSomeEventEmitter()
emitter.on('error', callback)
emitter.on('end', function () {
  callback()
})

Because Query objects will emit both of those events in the case of an error, that becomes rather problematic and you end up having to do something like.

var emitter = makeSomeEventEmitter()
emitter.on('error', function (err) {
  callback(err)
  callback = function () {}
})
emitter.on('end', function () {
  callback()
})

Or maintain state about errors or something. In any case, it makes handling errors a little non-standard. I'd love to change the either 'error' or 'end' is emitted, but never both.

What do you think?

[evansolomon]

@brianc any interest?

[brianc]

If that's how node core event emitters behave, then I think this is definitely how node-postgres should behave as well. You want to whip up a PR or have me add it in? I'm trying to get the [email protected] release out and this is a breaking change, so I have it targeted for that release.

[brianc]

Just made that change & all the tests pass - sooo that's good. Adding a test for the change & pushing it up as a PR. Going to ship this today. 👊

[evansolomon]

Awesome, thank you! I would have been happy to make the change, thanks for taking care of it.

Leaving this open for now, but feel free to close it when you land that change.