Skip to content

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

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

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

Conversation

@plamut
Copy link
Contributor

@plamut plamut commented Dec 16, 2019

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=lambda msg: None)

try:
    future.result()
except KeyboardInterrupt:
    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.
@plamut plamut added the api: pubsub Issues related to the Pub/Sub API. label Dec 16, 2019
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 16, 2019
Copy link
Contributor

@pradn pradn left a comment

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:master Dec 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: pubsub Issues related to the Pub/Sub API. cla: yes This 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