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

Only throw RabbitMQ.Client-specific exceptions. #1439

Closed
paulomorgado opened this issue Dec 5, 2023 · 4 comments
Closed

Only throw RabbitMQ.Client-specific exceptions. #1439

paulomorgado opened this issue Dec 5, 2023 · 4 comments
Assignees
Labels
Milestone

Comments

@paulomorgado
Copy link
Contributor

Describe the bug

From Using Standard Exception Types:

❌ DO NOT throw System.Exception or System.SystemException.

RabbitMQ code should return RabbitMQ exceptions. If there was another exception causing the one being thrown, it should be added as an InnerException.

Reproduction steps

ServiceBase.Notify is throwing System.Exception at https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/main/projects/RabbitMQ.Client/client/impl/SessionBase.cs#L128

Expected behavior

a RabbitMQ-specific exception is thrown.

Additional context

No response

@lukebakken
Copy link
Contributor

lukebakken commented Dec 5, 2023

Thank you @paulomorgado for taking the time to open this issue with great details.

Relevant comments - 169a6d1#r134239817

@lukebakken lukebakken changed the title Do not throw System.Exception Only throw RabbitMQ.Client-specific exceptions. Dec 28, 2023
@lukebakken
Copy link
Contributor

Exceptions thrown by this library could include the associated connection and channel as exception data to allow library users to take appropriate action. Thoughts? @paulomorgado

@paulomorgado
Copy link
Contributor Author

All the information that can be provided is always helpful.

Exceptions have a Data property that can be used to add information.

But this is not very discoverable and people usually choose to add the data as custome exception properties.

If you are creating scenario specific exceptions, you can create properties for that.

But I wouldn't mind having them as data.

@lukebakken
Copy link
Contributor

Closing! See this comment.

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

No branches or pull requests

2 participants