Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add dedicated exception forbasic.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
lukebakken merged 6 commits intomainfromrabbitmq-dotnet-client-1831
May 12, 2025

Conversation

@lukebakken
Copy link
Collaborator

Fixes#1831

This PR adds thePublishReturnException class that includes the originating exchange and routing key for abasic.return message. It should be backwards-compatible in the API.

ramonsmits reacted with thumbs up emojiansd reacted with hooray emoji
Fixes#1831This PR adds the `PublishReturnException` class that includes theoriginating exchange and routing key for a `basic.return` message. Itshould be backwards-compatible in the API.
@lukebakkenlukebakken self-assigned thisMay 8, 2025
@lukebakkenlukebakken added this to the7.2.0 milestoneMay 8, 2025
@bording
Copy link
Collaborator

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.

lukebakken reacted with thumbs up emoji

@lukebakken
Copy link
CollaboratorAuthor

@SzymonPobiega and@Gsantomaggio all set for a re-review, thanks.

@lukebakken
Copy link
CollaboratorAuthor

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

Feel free to suggest a patch! Or, modify my PR directly@bording ... I think you have write access here 😸

bording reacted with thumbs up emoji

@bordingbordingforce-pushed therabbitmq-dotnet-client-1831 branch from4a64a83 to39397a6CompareMay 9, 2025 18:25
@bordingbordingforce-pushed therabbitmq-dotnet-client-1831 branch from39397a6 to5b62dffCompareMay 9, 2025 18:30
@bording
Copy link
Collaborator

Feel free to suggest a patch! Or, modify my PR directly@bording ... I think you have write access here 😸

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.

lukebakken reacted with hooray emoji

Add more conditions to the testSigned-off-by: Gabriele Santomaggio <G.santomaggio@gmail.com>
Signed-off-by: Gabriele Santomaggio <G.santomaggio@gmail.com>
Copy link
CollaboratorAuthor

@lukebakkenlukebakken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks everyone!

@lukebakkenlukebakken merged commit1e6f8ab intomainMay 12, 2025
16 checks passed
@lukebakkenlukebakken deleted the rabbitmq-dotnet-client-1831 branchMay 12, 2025 15:44
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@GsantomaggioGsantomaggioGsantomaggio approved these changes

+1 more reviewer

@SzymonPobiegaSzymonPobiegaSzymonPobiega approved these changes

Reviewers whose approvals may not affect merge requirements

Labels

None yet

Projects

None yet

Milestone

7.2.0

Development

Successfully merging this pull request may close these issues.

Exception details lost when using async API

5 participants

@lukebakken@bording@SzymonPobiega@Gsantomaggio

[8]ページ先頭

©2009-2025 Movatter.jp