Skip to content

fix(pubsub): handle None in on response callback - #9982

Merged
pradn merged 1 commit into
googleapis:masterfrom
plamut:iss-9975
Dec 16, 2019
Merged

fix(pubsub): handle None in on response callback#9982
pradn merged 1 commit into
googleapis:masterfrom
plamut:iss-9975

Conversation

@plamut

@plamutplamut commented Dec 16, 2019

Copy link
Copy Markdown
Contributor

Fixes#9975.

This PR fixes an error that can occur when the streaming pull manager is being shut down when the underlying RPC gets shut down.

When that happens, the StreamingPullManager's _on_response() method is invoked with None (as opposed to a StreamingPullResponse instance). This commit handles this case gracefully.

How to test

(this is how I managed to reproduce the error on my machine consistently)

  • Use PubSub v1.1.0 (the bug does reportedly not occur in an earlier version, e.g. v1.0.2)
  • Create a topic and a subscription to that topic.
  • (optional) Set logging level to DEBUG.
  • Start pulling messages from the subscription using the subscriber's streaming pull.
  • While pulling messages, generate a keyboard interrupt, and cancel the streaming pull future as a response to the interrupt.
    (alternative: close the channel manually: subscriber.api.transport.channel.close()

Actual result (before the fix):
An error occurs during the shutdown ("'NoneType' object has no attribute 'received_messages'") in one of the background threads.

Expected result (after the fix):
No error occurs, and the streaming pull shuts down cleanly (check the DEBUG log output for details).

A sketch of the test script:

subscriber=pubsub_v1.SubscriberClient()
future=subscriber.subscribe(SUBSCRIPTION_PATH, callback=lambdamsg: None)
try:
future.result()
exceptKeyboardInterrupt:
future.cancel()

PR checklist

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

If the underlying RPC is shut down while pulling the messages with a
streming pull, the StreamingPullManager's _on_response() method is
invoked with None (as opposed to a StreamingPullResponse instance).
This commit handles this case and prevents an error in a background
thread on streaming pull manager shutdown.
@plamutplamut added the api: pubsub Issues related to the Pub/Sub API. label Dec 16, 2019
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 16, 2019

@pradnpradn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@pradn
pradn merged commit d372f66 into googleapis:masterDec 16, 2019
@plamut
plamut deleted the iss-9975 branch December 16, 2019 22:49
parthea pushed a commit that referenced this pull request Mar 2, 2026
If the underlying RPC is shut down while pulling the messages with a
streming pull, the StreamingPullManager's _on_response() method is
invoked with None (as opposed to a StreamingPullResponse instance).
This commit handles this case and prevents an error in a background
thread on streaming pull manager shutdown.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: pubsubIssues related to the Pub/Sub API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PubSub: AttributeError: 'NoneType' object has no attribute 'received_messages'

4 participants

@plamut@pradn@googlebot@anguillanneuf