Skip to content
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

fix: convert all RPC error types to exceptions #170

Merged
merged 3 commits into from
Sep 16, 2020

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Jul 24, 2020

Fixes #163.

This PR fixes a problem when the underlying gRPC channel is wrapped into something and terminates with a reason that is not an instance of grpc.RpcError. The streaming pull manager now converts all reason types into an Exception instance, fixing a TypeError when a StreamingPullFuture then tries to raise it.

Still waiting for the reporter's confirmation that this is indeed what happens, but I found no other place in the code that could call future.set_exception(...) with a non-Exception argument.

(probably also makes sense to merge #158 first so that we don't have to make yet another release before dropping Python 2)

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)

@plamut plamut added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 24, 2020
@plamut plamut requested a review from pradn July 24, 2020 13:16
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 24, 2020
@plamut plamut changed the title fix: Convert all RPC error types to Exception instances fix: convert all RPC error types to Exception instances Jul 24, 2020
@plamut plamut changed the title fix: convert all RPC error types to Exception instances fix: convert all RPC error types to exceptions Jul 24, 2020
@busunkim96 busunkim96 closed this Jul 31, 2020
@plamut plamut reopened this Jul 31, 2020
@pradn
Copy link
Contributor

pradn commented Aug 7, 2020

Looks good, thanks!

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/python-pubsub API. label Aug 21, 2020
@plamut plamut requested a review from cguardia August 28, 2020 10:00
Copy link
Contributor

@cguardia cguardia left a comment

Choose a reason for hiding this comment

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

I think this looks good. Thanks!

@plamut plamut removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 14, 2020
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 googleapis/python-pubsub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: exceptions must derive from BaseException
4 participants