-
Notifications
You must be signed in to change notification settings - Fork 696
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
[FIXED] Panic in ordered consumer #1686
Conversation
In `orderedConsumer.reset()`, `retryWithBackoff()` is returned from with `false, nil` in case the subscription is closed. In this case, `var cons Consumer` is still `nil`, and the code crashes in the attempt to cast it to a `pullConsumer` later on. Fix this by catching the `nil` case explicitly. Signed-off-by: Daniel Mack <daniel.mack@holoplot.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,
one question to make sure its ok.
@@ -142,6 +145,9 @@ func (c *orderedConsumer) Consume(handler MessageHandler, opts ...PullConsumeOpt | |||
select { | |||
case <-c.doReset: | |||
if err := c.reset(); err != nil { | |||
if errors.Is(err, errOrderedConsumerClosed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only case for errOrderedConsumerClosed
is when its stopped, so there is no point to try resetting it again, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there is no point. I used continue
here because when the consumer is stopped we'll eventually end up in <-sub.done
case where the actual cleanup is performed.
Signed-off-by: Piotr Piotrowski <piotr@synadia.com> Co-authored-by: Daniel Mack <daniel@zonque.org>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com> Co-authored-by: Daniel Mack <daniel@zonque.org>
Signed-off-by: Piotr Piotrowski piotr@synadia.com