Skip to content

fix(pubsub): complete publish futures on unexpected RPC errors - #18751

Open
maximilliangrand wants to merge 1 commit into
googleapis:mainfrom
maximilliangrand:fix/pubsub-complete-unexpected-publish-errors
Open

maximilliangrand wants to merge 1 commit into
googleapis:mainfrom
maximilliangrand:fix/pubsub-complete-unexpected-publish-errors

Conversation

@maximilliangrand

Copy link
Copy Markdown

When the publish RPC raises an exception outside GoogleAPIError or TransportError, the batch thread exits with its futures still pending. For example, retry="invalid" raises a TypeError in the GAPIC wrapper, but future.result() waits indefinitely.

Route ordinary publishing exceptions through the existing batch failure path so every future receives the original exception and the batch completion callback reports failure. This also covers credential refresh errors. Regression tests exercise the real threaded GAPIC wrapper with invalid retry/timeout values and verify the error state, callback, and exception identity for every future.

Fixes #15642.

Validation:

  • 1,764 unit tests pass in each of Python 3.14/upb, Python 3.14/pure-Python protobuf, and Python 3.10/upb with the repository's minimum dependency constraints.
  • Package Ruff import/format checks and flake8 pass.
  • The changed batch module has 100% line and branch coverage. Matching pristine/patched Python 3.14 runs have identical coverage in every unchanged file; overall coverage increases slightly from 99.429576% to 99.429820%. The local matrix does not reach the repository-wide 100% gate because of existing uncovered branches elsewhere.
  • No live-cloud system tests were run; the regression fails before sending a request.

@maximilliangrand
maximilliangrand requested a review from a team as a code owner October 17, 2026 20:15

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the publisher batch commit thread to catch all exceptions instead of only specific API and transport errors. This ensures that any unexpected publishing errors are propagated to the futures, preventing callers from waiting indefinitely. Additionally, the unit tests have been updated to cover more exception types and to verify that invalid RPC options correctly fail and propagate exceptions. There are no review comments, so I have no feedback to provide.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing catch-all try-except in batch thread, and missing publish() argument validation

1 participant