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

IConnection Dispose can throw OperationInterruptedException #133

Closed
matburton opened this issue Nov 11, 2015 · 6 comments
Closed

IConnection Dispose can throw OperationInterruptedException #133

matburton opened this issue Nov 11, 2015 · 6 comments
Assignees
Milestone

Comments

@matburton
Copy link

A while back you closed issue #119 that I'd previously worked-around by catching and swallowing IOException that could be thrown when calling Dispose on an IConnection.

I've just upgraded to version 3.5.6 but it seems that Dispose can still throw OperationInterruptedException which, judging from the description in #119, isn't intended behaviour?

The exception message I sometimes receive is The AMQP operation was interrupted. Am I calling Dispose at an inappropriate time perhaps?

(Sorry if this is more of a question than an issue)

@michaelklishin
Copy link
Member

Questions belong to rabbitmq-users.

I can't say anything without seeing your code and a stack trace. Connections should not be closed with Dispose but Close or Abort instead.

@michaelklishin
Copy link
Member

@micdenny what do you think about making Dispose ignore all exceptions for both connections and channels? Currently at least IModel doesn't, and relies on Abort to do that, which I believe still can throw AlreadyClosedException.

@matburton
Copy link
Author

Hey @michaelklishin. Apologies for the question style wording.

So it looks like Dispose calls Abort and that in turn can throw an exception from the ShutdownReports or an OperationInterruptedException if there are only ShutdownReports that don't have an exception.

I wouldn't really expect these to be thrown from Dispose either, but it doesn't look like the exceptions stored in ShutdownReports are thrown during Close or Abort when it might be more reasonable to throw exceptions? I guess another alternative would be to raise these exceptions through the CallbackException event during disposal.

The exception I was getting was the new OperationInterruptedException(null) thrown from the bottom of Dispose by the looks of it. Hopefully I can try and write some minimal code at some-point to reliably reproduce this.

@micdenny
Copy link
Contributor

@michaelklishin in the previous PR (#121) I did a conservative development, to avoid swallowing to many kind of exception, in fact I will remain of this idea, just adding the catch of OperationInterruptedException.

What other kind of exception do you/we should expect is thrown by the underline connection? I think we must be conservative and avoid catching the generic Exception so we have the control of what's going on, in other words I would prefer a github issue as this one, instead of, it took me a day to figure out that RabbitMQ client was swallowing an exception that is not belong to rabbitmq itself (memory corruption, strange network failures, etc...).

Is that make sense for you?

@michaelklishin
Copy link
Member

Sure. I think AlreadyClosedException and OperationInterruptedException are two key ones. If I come up with something else, I'll add them.

So, should I go ahead and make both Dispose methods ignore common library-specific exceptions? This should help with @matburton's case without introducing a lot of risk.

@micdenny
Copy link
Contributor

should I go ahead and make both Dispose methods ignore common library-specific exceptions?

to me it's ok 👍 leaving the same behavior, if Close is called with abort=false, the exception should be throw

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

No branches or pull requests

3 participants