Nodejs-pubsub: Error Handling in the Subscription MessageHandler

[stoked10]

The Subscription extension of the EventEmitter class appears to ignore errors that happen within the messageHandler. The EventEmitter listener for 'error' and 'uncaughtException' on Subscription seems to be eaten or ignored. To test this:

const messageHandler = (message) => {
  throw new Error('always an error');
};

const errorHandler = (err) => {
  logger.error('caught an error, continuing on');
};

subscription.on('message', messageHandler);
subscription.on('error', errorHandler);

'error' doesn't catch and the app crashes, we found the only way around this is to emit the error to the app instead of throwing, then handle it at the app layer, but that is undesirable for being able to handle the errors gracefully from our pubsub codebase and is really hacky passing the app down to our subscription service.

[callmehiphop]

I'm not entirely sure I would agree that the client should catch this error and re-throw it in the form of an event. IMO the client should only emit client related errors and what you're describing sounds like a user error.

@bcoe @stephenplusplus do you have any feelings on this?

[stephenplusplus]

@stoked10 Would it work to emit the error from within the message handler? That should let you take the wheel when it comes to app-determined errors.

[stoked10]

@stephenplusplus How do you mean? Would we be able to do a listener on the subscriber for error? Or do you mean for us to create a secondary EventEmitter then emit to that within the messageHandler?

[stephenplusplus]

I think if you use the scope of the message handler, you can emit right there and your subscriber error handler will catch it. So, something like:

const messageHandler = function() {
  // ^__ note this doesn't use arrow functions, which
  // would persist the scope of the outer function 
  if (errorCondition) {
    this.emit('error', new Error('Message'))
  }
}

Or, you can just emit it right on the subscription instance, subscription.emit('error', new Error('Message')).

[stoked10]

@stephenplusplus the.emit worked perfectly and is a good solution for us, thanks!!

[esabelhaus]

Would it make sense to include this in example code to help folks using the library in the future? If so I'd be glad to submit a PR

[callmehiphop]

This seems pretty specific to @stoked10's implementation, so I don't think we need to add any samples for it.