Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null Message Exception for 404 Errors Skipped in amqplib >= 0.10.0 #301

Closed
lance-menard opened this issue Oct 11, 2022 · 3 comments · Fixed by #303
Closed

Null Message Exception for 404 Errors Skipped in amqplib >= 0.10.0 #301

lance-menard opened this issue Oct 11, 2022 · 3 comments · Fixed by #303
Labels

Comments

@lance-menard
Copy link

lance-menard commented Oct 11, 2022

It appears that amqplib versions >= 0.10.0 no longer have an isOperational flag on errors, causing the specific 404 exception linked below to no longer gracefully skip throwing an error. I'm unclear what that flag is intended to communicate - documentation is scarce - so I'm not sure what the impact might be of removing it for versions of amqplib < 0.10.0.

https://github.com/jwalton/node-amqp-connection-manager/blob/master/src/ChannelWrapper.ts#L762

Reproducing the issue is fairly straightforward: install amqplib >= 0.10.0 alongside the amqp-connection-manager, create a channel and bind and consume from a queue, then delete that queue via the RabbitMQ admin dashboard or API. In older versions of amqplib, the connection manager disconnects, then reconnects automatically and reruns the setup function to recreate the queue, as described in the comment. In newer versions, the error is instead thrown and bubbles up.

@luddd3
Copy link
Collaborator

luddd3 commented Oct 12, 2022

I think isOperational was used to communicate whether the channel was still usable or not.

@lance-menard
Copy link
Author

lance-menard commented Oct 12, 2022

Interesting. It looks to me like the only PR included in v0.10.0 of amqplib is this one. I can't see anything in there that would obviously have caused that property to vanish - but then again, I don't see anything at all related to that property, so I'm not entirely clear where it was coming from in the first place.

This does bring to light a related problem, which is that if the .catch() attached to _reconnectConsumer throws on line 770, the error becomes an unhandled promise rejection. As documented here, this can happen in a clustered scenario when the node hosting a queue is failing (and I've seen that exact situation happen in production). That's better than just swallowing the error entirely, but it seems to me it might be better to emit an error event on the channel and try to reconnect if the channel consumption is cancelled for any reason. Thoughts?

luddd3 added a commit to luddd3/node-amqp-connection-manager that referenced this issue Oct 24, 2022
github-actions bot pushed a commit that referenced this issue Oct 24, 2022
## [4.1.8](v4.1.7...v4.1.8) (2022-10-24)

### Bug Fixes

* error thrown when queue deleted in amqplib 0.10.0 ([60700ee](60700ee)), closes [#301](#301)
@jwalton
Copy link
Owner

jwalton commented Oct 24, 2022

🎉 This issue has been resolved in version 4.1.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants