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

IModel.WaitForConfirmsOrDie* methods don't document that they close #1234

Closed
lukealbao opened this issue Jul 22, 2022 · 2 comments
Closed

IModel.WaitForConfirmsOrDie* methods don't document that they close #1234

lukealbao opened this issue Jul 22, 2022 · 2 comments

Comments

@lukealbao
Copy link

lukealbao commented Jul 22, 2022

Hi there, thanks for this library.

I suppose many people could infer from the name (OrDie) that these methods close the channel. I however assumed that the documentation was clear enough to be complete:

Waits until all messages published since the last call have
been ack'd by the broker.  If a nack is received or the timeout
elapses, throws an OperationInterrupedException exception immediately.

I consider it extra dangerous because failures are rare enough that automated testing doesn't catch it, as ours did not. With long-lived channels, the first nack caused all subsequent publish attempts to fail.

I'd be happy to submit a PR, but I'm unclear on (1) if this is actually part of the interface or if it just so happens that the implementations do this, and (2) how would one submit a PR for documentation that would apply to all supported versions/branches? (We are using 4.x. I just noticed that the copied doc above also describes the wrong exception type; that's from the 4.x branch.)

Thank you again!

@michaelklishin
Copy link
Member

4.x is no longer maintained. This change can be back ported to other branches just like any other.

michaelklishin added a commit that referenced this issue Jul 22, 2022
(cherry picked from commit dcbc380)
@lukebakken
Copy link
Contributor

FWIW, using WaitForConfirmsOrDie should be avoided and you should deal with publisher confirms this way:

https://www.rabbitmq.com/tutorials/tutorial-seven-dotnet.html

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

No branches or pull requests

3 participants