-
Notifications
You must be signed in to change notification settings - Fork 606
Add dedicated exception for basic.return
messages.
#1832
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
base: main
Are you sure you want to change the base?
Conversation
Fixes #1831 This PR adds the `PublishReturnException` class that includes the originating exchange and routing key for a `basic.return` message. It should be backwards-compatible in the API.
While it's good to add these properties to the exception for inspection, I'd also really like to see exception messages being added that display these property values in the message text. Those messages tend to get logged along with stack traces and provide useful information in logs. Custom properties on an exception are much less likely to make it into a log file. |
@SzymonPobiega and @Gsantomaggio all set for a re-review, thanks. |
Feel free to suggest a patch! Or, modify my PR directly @bording ... I think you have write access here 😸 |
4a64a83
to
39397a6
Compare
39397a6
to
5b62dff
Compare
I pushed up a change to add exception messages. The interesting one is for the return messages because there really isn't any useful information to include for a nack. |
Fixes #1831
This PR adds the
PublishReturnException
class that includes the originating exchange and routing key for abasic.return
message. It should be backwards-compatible in the API.