Explicit way of setting up the connection

[d2201]

Hey, as I was setting up some tests in one of my projects - I had some issues with rabbitmq server. The problem though is with the logs my app would capture, they were just generic node AggregateError without any insightful stack.

Then I looked at what's happening, and noticed that in the constructor you're initiating the call that is handled async. So when my app is getting initialized & the error happens, this can throw off whole node process.

Could there be possibility for extending the class Connection to perhaps include a static method? e.g.:

class Connection {
  static create(propsOrUrl?: string|ConnectionOptions): Promise<Connection> {
     ... initialization that waits for the connection to succeed
  }
}

It would really help with catching all errors in app setup.

Let me know if you're opened to outside contributions, so I could help you a little bit with this amazing project 👍

[cody-greene]

If I understand you correctly, your application is crashing because of an uncaught exception? Attach an error handler immediately after you create the connection (or more generally: before the end of the current event-loop tick):

const con = new Connection()
con.on('error', (err) =>{ console.log('RabbitMQ connection:', err) })

But I do see value in a giving users a promise that resolves when the connection is stable. Right now you've got just the event con.on('connection', ...) and con.ready: boolean. In the test suite I have a helper function do something like:

await new Promise((resolve) => connection.once('connection', resolve))

This, of course, has a downside in that if the connection is never established then the promise never resolves. Users need a more polished method with timeout, etc. I suspect you'd also want this to handle race-conditions like starting the rabbit broker at the same time as your application, where it might take a few retries before the connection is established. So rejecting the promise with the first error is probably not a good idea. All that to say I would rather have:

class Connection {
  // name TBD
  resolveWhenReady(timeout = 0) :Promise<void>
}

[d2201]

Users need a more polished method with timeout

💯

So rejecting the promise with the first error is probably not a good idea

I could argue on the bare concept of this approach, but given you've worked with raw details and you got much better experience there, I'll assume this is the right approach.

I do like the proposal, it's definitely a step in the right direction.

[cody-greene]

Any thoughts on https://github.com/cody-greene/node-rabbitmq-client/pull/58 ?