Skip to content
This repository has been archived by the owner on May 13, 2019. It is now read-only.

Protect against nil messages, which happen very infrequently. #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rrh
Copy link
Contributor

@rrh rrh commented Nov 12, 2015

The root cause of nil messages has not been diagnosed.

Run "go fmt"

@wvanbergen
Copy link
Owner

The only explanation I have for nil messages is the channel being closed outside of the consumers control, after which we end up reading zero values from the channel.

What do you think of this:

case messages, ok <- message:
  if ok {
    lastOffset = message.Offset
  else {
    // log something
  }

This would only protect against this particular case instead of nil values actually being present on the channel (which should never happen). Also, maybe we can log something that would allow us to investigate further?

cc @andremedeiros

@rrh
Copy link
Contributor Author

rrh commented Nov 23, 2015

I'm confused. Your counter proposal is to use:
case messages, ok <- message
but this is a case on a send statement, and AFAICT send statements don't allow for a way to assign "success/failure" to an auxiliary bool.

@rrh
Copy link
Contributor Author

rrh commented Dec 1, 2015

ping?

Your proposal doesn't compile:
./consumer_group.go:412: select cases cannot be lists

Further, even if the send succeeds (ok == true), it may still succeed with a nil message, and we'll then still redirect through nil causing a NPE.

@rrh
Copy link
Contributor Author

rrh commented Dec 1, 2015

[[Adding a log message is easy..]]

The root cause of nil messages has not been diagnosed.

Run "go fmt"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants