Consuming from a non-existing queue leads to an unrecoverable state

[adrianbielewicz]

There is an exception being thrown inside consumers of subscribe methods in consumeNoAck and consumeManualAck. This leads to un unrecoverable state. The exception is not emitted as error.

I used BaseSubscriber in Mono.subscribe() method of each consume method so the exception thrown in consumer is then handled by errorConsumer.

The problem was discovered in our project when Receiver was consuming from a queue that didn't exist. This is the same issue as described in https://github.com/reactor/reactor-core/issues/1995 and usage of BaseSubscriber is a workaround.

[pivotal-issuemaster]

@adrianbielewicz Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[simonbasle]

I think another more viable solution is to avoid throwing in the handler, and directly propagate the RabbitFluxException to the emitter on line 150 inside the catch block.

[acogoluegnes]

@adrianbielewicz I confirm that @simonbasle's suggestion works in both cases. I'd suggest to go this way. Thanks for the tests, they're really useful.

[adrianbielewicz]

No problem. I'm glad I could help.

I was actually considering both approaches and as only IOException is catched there I decided to do it using BaseSubscriber. Not sure if there is any possibility for a different exception to be thrown, but if it does happen, it will lead to the same state as now.

[acogoluegnes]

You can go with emitter.error(new RabbitFluxException(e)) to keep the same exception, I should be fine I guess.

[simonbasle]

@acogoluegnes you mean turn the catch (IOException e) into catch (Throwable e) ? (yes it should be fine)

[acogoluegnes]

@simonbasle @adrianbielewicz Sorry, I got confused. Nothing else than IOException should be thrown, but catching even Exception would be safer indeed. Feel free to update your PR with this change @adrianbielewicz. Thanks!

[acogoluegnes]

@adrianbielewicz Please sign the CLA so we merge this!

[adrianbielewicz]

@acogoluegnes I am waiting for the approval at Opera. I will sign it as soon as I get it.

[adrianbielewicz]

I've got the approval, but I have a problem signing the CCLA. Is GitHub Organization necessary?

[acogoluegnes]

@adrianbielewicz Honestly, I don't know. @simonbasle any clue?

You may have a look at the FAQ https://cla.pivotal.io/about.

@simonbasle Or is it possible to add a comment with a blurb "I have read the CLA and adhere to it completely"?

[simonbasle]

@adrianbielewicz Honestly, I don't know. @simonbasle any clue?

You may have a look at the FAQ cla.pivotal.io/about.

@simonbasle Or is it possible to add a comment with a blurb "I have read the CLA and adhere to it completely"?

We need to go through the CLA tool, a blurb won't do I think.

The FAQ states it should be possible to sign the CCLA linking it to accounts by email address:

A corporate signature is either associated to the domain of a verified email address or a GitHub organization.

You'll need to make sure the email address of the same domain is linked to your github account @adrianbielewicz, as explained in the FAQ (and you might need to reconnect to the CLA tool, etc...

The organization-linking looks simpler.

[pivotal-issuemaster]

@adrianbielewicz Thank you for signing the Contributor License Agreement!

[adrianbielewicz]

@acogoluegnes I've just signed CLA.

[acogoluegnes]

Thanks!