if an event is an error, pass the exception object

[cxreg]

Events emitted due to socket monitoring may represent an error, and currently only return the numeric errno. The binding has a method to translate these to the string that zmq_strerror provides but this is not exposed. Add one, and use it to provide the exception it generates to the callback.

I don't love the function signature to the event callback with this change, but I wrote it that way to be backwards compatible. If you have a better suggestion, I'm open to ideas.

[cxreg]

Any thoughts about this?

[cxreg]

ping

[ronkorving]

Thank you for the contribution. I'm just wondering, can't we just send the exception straight from C++ land into onMonitorEvent?

[cxreg]

You mean as an extra argument here:

https://github.com/cxreg/zeromq.node/blob/660e4bea5404a8519fc52cd39831ddc7307d98fa/binding.cc#L451

as opposed to asking for it conditionally? Sure that is possible, if you think that's a better interface. It would probably still need to be conditional (in the C++ side) so we aren't constructing error objects for non-errors

[ronkorving]

Yeah, we would still need the conditional, and that's fine. I just prefer reducing JS-C++ roundtrips (which imho also make it a bit harder to reason about). Sorry to be so picky... Does it make sense though?

[cxreg]

@ronkorving reworked it as you proposed, does this look better?

[reqshark]

Much of the original purpose of zmq socket patterns was about bridging endpoints in a clean way so that the programmer wouldn't have to worry about stuff like TCP errors, connect errors.

Monitoring to me feels more like silent logging, so for those reasons I think we should we should just convert the error numbers to their string mappings without necessarily generating JS exceptions over it.

[cxreg]

An unthrown Error object is not an exception. It's a nice way to include multiple pieces of information, including both a string and a numeric code (although this library is only including the string currently). Additionally, many logging libraries recognize an Error object making them semantically useful even for the purpose you just mentioned.

[reqshark]

An unthrown Error object is not an exception

ok, cool, as long as we're not crashing procs for not handling connection errors during monitoring this change LGTM. Thanks for clarifying @cxreg! 👍

[ronkorving]

Wonderful, thanks! And thank you @reqshark for having a look as well 👍

[ronkorving]

Travis is not happy, but I doubt that's related to this PR. I'm doing some reruns to see if it'll pass.