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

[fix][client] Fix NPE when acknowledging multiple messages #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BewareMyPower
Copy link
Owner

Motivation

For a multi-topics consumer, when it acknowledges a single message, it will first find the owner topic from its message ID. If the owner topic is not subscribed by the consumer, NotConnectedException will be thrown.

However, when it acknowledges multiple messages, if any of them is the message whose owner topic is not subscribed by the consumer, NPE will happen instead.

Modifications

When acknowledging multiple messages, ignore the message IDs whose owner topic is not subscribed. testAckMessageInAnotherTopic is added to cover this case.

TODO

There are many other places that do not check if consumers.get returns null, like doReconsumeLater, negativeAcknowledge, etc. This patch does not cover them.

### Motivation

For a multi-topics consumer, when it acknowledges a single message, it
will first find the owner topic from its message ID. If the owner topic
is not subscribed by the consumer, `NotConnectedException` will be
thrown.

However, when it acknowledges multiple messages, if any of them is the
message whose owner topic is not subscribed by the consumer, NPE will
happen instead.

### Modifications

When acknowledging multiple messages, ignore the message IDs whose owner
topic is not subscribed. `testAckMessageInAnotherTopic` is added to
cover this case.

### TODO

There are many other places that do not check if `consumers.get` returns
`null`, like `doReconsumeLater`, `negativeAcknowledge`, etc. This patch
does not cover them.
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant