node-amqp10 icon indicating copy to clipboard operation
node-amqp10 copied to clipboard

AMQPError and friends are not exposed through the public API

Open damonbarry opened this issue 10 years ago • 6 comments

There are many places in the amqp10 code where AMQPError objects are returned to the user (e.g., when the user sets up a listener for 'errorReceived' on the client, or on a sender/receiver link). But AMQPError doesn't appear to be exposed through the public API, i.e., I can't reach the object definition through "require('amqp10')". I'd like to be able to access the definition of this class because I'm writing a library that wraps amqp10, and I need to translate AMQPErrors into my library-specific errors. In my tests, I want to be able to create simple AMQPError objects and pass them to my translation function. It would also be useful to access the definition of AMQPSymbol

There is an object at require('amqp10').Errors, but that's different. Perhaps the plan is that all AMQPErrors will eventually be abstracted into transport-agnostic errors under require('amqp10').Errors? If so that's fine. Whatever the official plan is, I just want to be able to map amqp10 errors to my library-specific errors.

damonbarry avatar Dec 02 '15 21:12 damonbarry

This makes sense - I'll try and get a new release out today with those enhancements.

noodlefrenzy avatar Dec 03 '15 17:12 noodlefrenzy

@noodlefrenzy seeing that the AMQPErrors are the actual descriptors, does it make sense to make a wrapper in lib/errors.js for an AMQPError object (to keep exported error consistency)? Not really sure what the best approach is here

mbroadst avatar Dec 03 '15 17:12 mbroadst

@mbroadst yes, that's exactly what I was planning :)

noodlefrenzy avatar Dec 03 '15 18:12 noodlefrenzy

@damonbarry I have a PR out for the change, all the code is in https://github.com/noodlefrenzy/node-amqp10/tree/expose-amqp-error-symbol if you want to try it before you buy.

noodlefrenzy avatar Dec 04 '15 17:12 noodlefrenzy

Hey there, published. @2.1.3 resolves this.

NOTE: There is a change in the returned error in that I now unpack the condition symbol (so you no longer need condition.contents to get the string). Since this was not part of the public API I didn't feel the need for a 2.2 bump, sorry if I'm a bad person for not doing so.

Obviously let me know (and reopen this) if you run into any issues.

noodlefrenzy avatar Dec 04 '15 19:12 noodlefrenzy

I misunderstood the need here - he'd like an isolated constructor to avoid having to take dependencies on internal classes while mocking for tests, so will modify to provide that.

noodlefrenzy avatar Dec 07 '15 18:12 noodlefrenzy