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

Channel does not handle server-initiated closes. #70

Open
jmalloc opened this issue Jul 1, 2018 · 3 comments
Open

Channel does not handle server-initiated closes. #70

jmalloc opened this issue Jul 1, 2018 · 3 comments

Comments

@jmalloc
Copy link

jmalloc commented Jul 1, 2018

Channel::onFrameReceived() has no path that handles an inbound MethodChannelCloseFrame.

This means that if the server closes a channel, we get the generic exception message Message: Unhandled method frame Bunny\Protocol\MethodChannelCloseFrame., thrown from https://github.com/jakubkulhan/bunny/blob/master/src/Bunny/Channel.php#L623.

It would be great if the exception thrown here included the AMQP error information from the close frame, such as the error code constant and the error message. I'm not sure if it's appropriate to add this to ChannelException, or whether a new exception type should be added.

Aside from providing more information to the developer I'd like to add that there are programatic use cases for having access to the error code. For example, being able to check if a resource-locked code is returned when attempting to start an exclusive consumer.

I'm not very familiar with Bunny yet, so please forgive me if I've missed something here. I can put together a PR if you could provide some direction as to what you think should be changed :)

/cc @act28

@jakubkulhan
Copy link
Owner

Hi, this really is missing 😇 I'd be glad for a PR:

  • I think 2 subclasses of ChannelException/ClientException should be created SoftError*Exception/HardError*Exception - see spec/amqp-rabbitmq-0.9.1.json -> constants.
  • For soft errors (e.g. resource-locked), before throwing and exception, Channel should send channel.close-ok reply to server (I haven't checked with the spec, however, that is the behavior I'd expect) and change its internal state to ChannelStateEnum::CLOSED.
  • For hard errors, before throwing and exception, Channel should change its internal state to ChannelStateEnum::ERROR.

@brandonsimpson
Copy link

Hi, did these exceptions ever get handled?

@Donatello-za
Copy link
Contributor

Donatello-za commented Jan 12, 2019

I was effected by this issue when I tried publishing messages to a non existing exchange. If the RabbitMQ response error message was provided it would have been much quicker in picking up what the actual problem was. I'm going to try and submit a PR in the next week or two if I get enough time.

Donatello-za added a commit to Donatello-za/bunny that referenced this issue Jan 12, 2019
- Generator will generate exception classes and error fetcher from amqp spec.

Todo:
- Support AbstractFrame also, currently only MethodFrame frames are supported.
- Implement usage of the new error fetching class everywhere.
Donatello-za added a commit to Donatello-za/bunny that referenced this issue Jan 13, 2019
… exceptions.

- This commit is only for discussion on the PR and is not final.
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

4 participants