bunny icon indicating copy to clipboard operation
bunny copied to clipboard

Channel does not handle server-initiated closes.

Open jmalloc opened this issue 6 years ago • 3 comments

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

jmalloc avatar Jul 01 '18 08:07 jmalloc

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.

jakubkulhan avatar Jul 03 '18 18:07 jakubkulhan

Hi, did these exceptions ever get handled?

brandonsimpson avatar Dec 21 '18 21:12 brandonsimpson

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 avatar Jan 12 '19 07:01 Donatello-za