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.

Sign up for GitHub

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

Jump to bottom

L105: Python Add New Error Types #388

Merged
merged 4 commits into from
Sep 27, 2023
Merged

L105: Python Add New Error Types #388

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2132578
L105: Python Add New Error Types
XuanWang-Amos Aug 21, 2023
71f4455
Change error name
XuanWang-Amos Aug 22, 2023
4be3256
Add discussion link
XuanWang-Amos Aug 31, 2023
459053a
Add part for BaseError
XuanWang-Amos Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 42 additions & 0 deletions L105-python-expose-new-error-types.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
L105: Python Add New Error Types
----
* Author(s): XuanWang-Amos
* Approver: gnossen
* Status: In Review
* Implemented in: Python
* Last updated: 08/31/2023
* Discussion at: https://groups.google.com/g/grpc-io/c/pG34X9nAa3c

## Abstract

* Add two new errors in grpc public API:
* BaseError
* AbortError
* Also changed RpcError to be a subclass of BaseError.

## Background

In case of abort, currently we don't log anything, exposing those error types allows user to catch and handle aborts if they want.

## Proposal

1. Add AbortError and BaseError in public API.
2. Change RpcError to be a subclass of BaseError.


## Rationale

The Async API [has similar errors](https://github.com/grpc/grpc/blob/v1.57.x/src/python/grpcio/grpc/aio/__init__.py#L23,L24). We're refactoring code so those errors will also be used in Sync API. Adding them to the Sync API will help us keep the two stacks in sync and allow users of the Sync implementation to catch and handle aborts.

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to mention that adding this will allow users to type:

try:
  do_grpc_stuff()
except grpc.BaseError as e:
  # handle error

and be confident that they're catching all gRPC exceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We also have RpcError exposed to user, I think we should mention their differences, when should user expect RpcError?

Copy link
Contributor

Choose a reason for hiding this comment

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

If RpcError is not currently a subclass of BaseError, I think we should make it a subclass of BaseError. Ditto for all other exception types. The ultimate goal is to enable the snippet above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sense, I'll change RpcError to be a subclass of BaseError and add it to this gRFC.

We also plan to change RpcError to be a subclass of BaseError so that all grpc errors are a subclass of BaseError, this will allow users to catching all gRPC exceptions use code like this:

```Python
try:
do_grpc_stuff()
except grpc.BaseError as e:
# handle error
```

## Implementation

And and check AbortError while abort : https://github.com/grpc/grpc/pull/33969