Skip to content

Don't panic when cancelling an unknown consumer #525

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.

Sign up for GitHub

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

Jump to bottom
Closed
michaelklishin opened this issue Mar 19, 2019 · 1 comment
Closed

Don't panic when cancelling an unknown consumer #525

michaelklishin opened this issue Mar 19, 2019 · 1 comment
Assignees
@michaelklishin
Labels
bug effort-tiny usability
Milestone
4.11.0

Comments

@michaelklishin
Copy link
Contributor

michaelklishin commented Mar 19, 2019 •
edited
Loading

See https://github.com/spring-projects/spring-integration/issues/2658#issuecomment-474452945 for background. Kudos to @bojanv55 for solid investigative work.

When a user tries to cancel a consumer or when this client receives a basic.cancel for a consumer it doesn't recognize, instead of throwing an IOException it should log a warning and ignore the method. This is what works well for other clients.

Note that such scenarios are realistic and have to do with inherently racy consumer state management on server and client ends. Being more defensive and not panicking is the right thing to do.

Per discussion with @acogoluegnes.
@michaelklishin michaelklishin changed the title Don't panic when handling a basic.cancel method for an unknown consumer Don't panic when cancelling an unknown consumer Mar 19, 2019
@michaelklishin michaelklishin self-assigned this Mar 19, 2019
@michaelklishin
Copy link
Contributor Author

@acogoluegnes and I decided to consider making Channel#basicCancel idempotent for 6.0 but avoid throwing an exception for server-sent basic.cancel frames.

@garyrussell garyrussell mentioned this issue Mar 19, 2019
Closed
@acogoluegnes acogoluegnes added this to the 5.6.1 milestone Mar 20, 2019
@acogoluegnes acogoluegnes mentioned this issue Mar 20, 2019
Closed
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Log warning when receiving basic.cancel for unknown consumer
be13273 
Fixes #525
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add test for Channel#basicCancel with unknown consumer tag
261ffff 
References #525, #528
@acogoluegnes acogoluegnes mentioned this issue Mar 22, 2019
Merged
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add comment to explain decision tree
ad69686 
References #525
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Log warning when receiving basic.cancel for unknown consumer
6081628 
Fixes #525

(cherry picked from commit be13273)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add test for Channel#basicCancel with unknown consumer tag
365dd22 
References #525, #528

(cherry picked from commit 261ffff)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add comment to explain decision tree
e49f66d 
References #525

(cherry picked from commit ad69686)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Log warning when receiving basic.cancel for unknown consumer
d312014 
Fixes #525

(cherry picked from commit be13273)

Conflicts:
	src/test/java/com/rabbitmq/client/test/ClientTests.java
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add test for Channel#basicCancel with unknown consumer tag
b88b58f 
References #525, #528

(cherry picked from commit 261ffff)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add comment to explain decision tree
c2482bf 
References #525

(cherry picked from commit ad69686)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Log warning when receiving basic.cancel for unknown consumer
91d6f8d 
Fixes #525

(cherry picked from commit be13273)

Conflicts:
	src/test/java/com/rabbitmq/client/test/ClientTests.java
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add test for Channel#basicCancel with unknown consumer tag
1e21fa8 
References #525, #528

(cherry picked from commit 261ffff)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add comment to explain decision tree
8724b82 
References #525

(cherry picked from commit ad69686)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Log warning when receiving basic.cancel for unknown consumer
546269a 
Fixes #525

(cherry picked from commit be13273)

Conflicts:
	src/test/java/com/rabbitmq/client/test/ClientTests.java
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add test for Channel#basicCancel with unknown consumer tag
4041927 
References #525, #528

(cherry picked from commit 261ffff)
acogoluegnes added a commit that referenced this issue Mar 22, 2019
@acogoluegnes
Add comment to explain decision tree
3327e14 
References #525

(cherry picked from commit ad69686)
@acogoluegnes acogoluegnes modified the milestones: 5.6.1, 4.10.1 Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelklishin michaelklishin

Labels
bug effort-tiny usability
Projects
None yet
Milestone
4.11.0
Development

No branches or pull requests
2 participants
@michaelklishin @acogoluegnes