added more protections for errors in callback

[zakagan]

added more protections for errors in callback, plus an option for the user to control their own error handling as part of the callback

[jfmesse]

this only works when onMessageCallback is synchronous. The response in this instance isn't really a callback. If onMessageCallback was a standard callback function, I'd have thought the structure of the method would be more like:

const onMessageUserHandling = (msg) => {
  onMessageCallback(msg, err => {
    if (err) {
      console.error(`Error from callback: ${err}`);
      msg.nack();
      process.exit(1);
    } else {
      msg.ack();
    }
  });
}

[zakagan]

Hi @jfmesse, I took a look at your proposed solution, but it hasn't clicked for me. Apologies, as I am not an experienced Node developer. However, the callback is intended to be provided by the user. It's what you want to be executed when a message is received. I'm not sure how passing an arrow function as an input in the callback addresses this issue.

Since this is an open repo, if you like you may submit your desired changes as a PR and I will evaluate and test them out. Otherwise I'll take a second pass and see what can be done. Thanks for bearing with us.

[zakagan]

Hi again @jfmesse I took a second look at this, adding the error handling as a callback the way you suggested. I was confused initially, since it's a callback in a callback. Can you see if this works for you?

[jfmesse]

Hi @zakagan , sorry I missed this. Your changes look good to me. We have a developer working on the ingest component now who will be looking to use. Thank you.

[jfmesse]

it's of lesser concern, but process.exit(1) feels a little brutal way of failing. It takes away the ability of the application to cleanly handle the client having a fatal error.

[jfmesse]

note the try catch behaviour only works with onMessageCallback being a synchronous method. If it was a promise or standard callback function, the try catch would not be triggered