From 12d4b3071524f4fed8ca173148b57e7f471380ca Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 7 Dec 2018 16:34:01 -0800 Subject: [PATCH 01/10] New RPC Status API for Python --- L44-python-rich-status.md | 394 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 394 insertions(+) create mode 100644 L44-python-rich-status.md diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md new file mode 100644 index 000000000..ce3e5caf6 --- /dev/null +++ b/L44-python-rich-status.md @@ -0,0 +1,394 @@ +New RPC Status API for Python +---- +* Author(s): lidizheng +* Approver: a11r +* Status: Draft +* Implemented in: Python +* Last updated: 12-06-2018 +* Discussion at: (filled after thread exists) + +## Abstract + +Current design of status API doesn't support rich status, also the concept of status "details" doesn't comply with gRPC spec. So, after discussion with @gnossen and @ericgribkoff, we think we should add a new set of status APIs to correct them. + + +## Background + +The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of a RPC call `Status` and `Status-Message`. But status code and a text message themselves are insufficient to express more complicate situation like the RPC call failed because of quota check of certain policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). + +The one major use case of this feature is Google Cloud API. The Google Cloud API needs it to transport rich status from server to client, also they are the owner of those proto files. + +So, the gRPC team introduced a new internal trailing metadata entry `grpc-status-details-bin` to serve this purpose. It takes in serialized proto `google.rpc.status.Status` message binary and transmits as a binary metadata. However, since gRPC is a ProtoBuf-agnostic framework, it can't enforce either the content of the binary data be set in the trailing metadata entry, nor the consistency of status code/status message with the additional status detail proto message. The result of this unsolvable problem is that for three major implementation of gRPC we have, each of them are slightly different about this behavior. + +### Behavior Different Across Implementations + +C++ implementation tolerates anything to be put in the `grpc-status-details-bin` entry; Java will check if the code of the proto status is matching the status code of the RPC call; Golang will enforce the both code and message of the proto status and the RPC call be the same. + +So, which paradigm should Python follow? + +### The Proto of Status + +The proto of status is well-defined and used in numerous framework. Here is the definition of `Status`, for full version see [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). +```proto +message Status { + int32 code = 1; + string message = 2; + repeated google.protobuf.Any details = 3; +} +``` + +### Current Python API + +```Python +# Client side +stub = ...Stub(channel) +try: + resp = stub.ARpcCall(...) +except grpc.RpcError as rpc_error: + code = rpc_error.code() + message = rpc_error.details() + # Unable to get rich status +``` + +```Python +# Server side +def ...Servicer(...): + ... + def ARPCCall(request, context): + ... + context.set_code(...) + context.set_details(...) + ... + +``` + +## Proposal + +### Concept Change + +**Status message** should be interpreted as a text (unicode allowed) field that indicates the status of the RPC call and it is intended to be read by the developers. + +**Status details** should be understood as a encoded `google.rpc.status.Status` proto message that serves as rich status error details about the RPC Call. + +### Client side API + +Currently, the documentation about getting status resides in [grpc.Call](https://grpc.io/grpc/python/grpc.html#client-side-context) section. We propose to add a new member function named `status()`, it will return a `namedtuple('Status', ['code', 'message', 'details'])`. + +The usage snippet will look like: + +```Python +stub = ...Stub(channel) + +try: + resp = stub.AMethodHandler(...) +except grpc.RpcError as rpc_error: + code = rpc_error.status().code + message = rpc_error.status().message + details = rpc_error.status().details + # or + code, message, details = rpc_error.status() +``` + +### Server side API + +gRPC Python team encountered lots of disagreements about how this part should be implemented. Available options will be listed in the `Rationale` section. Here I will post the final consensus for the team, but feel free to comment about other options. + +The new API will be added to [ServicerContext](https://grpc.io/grpc/python/grpc.html#service-side-context), and named `set_status`. It accept two positional arguments and a keyword argument, so setting the status code and status message are mandatory but not details. At the same time, current API `set_code`/`set_details` will be labeled as "deprecated". + +The function signature will be: + +```Python +def set_status(code, message, details=""): pass +``` + +The usage will look like: + +```Python +def ...Servicer(...): + def ARPCCall(request, context): + ... + context.set_status( + grpc.StatusCode.INVALID_ARGUMENT, + "Failed to recognize input variables", + ) +``` + +### Rich Status Usage + +Besides this proposal, there will be a new gRPC Python extension package that depends on ProtoBuf named `grpcio-status`. The new package will provide convenient functions to help developers transform from ProtoBuf instance to gRPC status. The usage will look like: + +```Python +# Client side +from grpc_status import rpc_status + +stub = ...Stub(channel) + +try: + resp = stub.AMethodHandler(...) +except grpc.RpcError as rpc_error: + rich_status = rpc_status.from_rpc_error(rpc_error) + # `rich_status` here is a ProtoBuf instance of `google.rpc.status.Status` proto message +``` +```Python +# Server side +from grpc_status import rpc_status +from google.protobuf import any_pb2 + +def ...Servicer(...): + def ARPCCall(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + rpc_status.error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.status_pb2.Status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail] + ) + + context.set_status(*rpc_status.convert(rich_status)) +``` + + +## Rationale + +There are four more alternative options of implementing this feature. + +### Option 1: A New Exception Handling Mechanism + +We expose a new public exception interface that developer can raise within their servicer method handler. The exception itself will contain information like status code, status message, and most importantly the rich status details. As for the extension package, we shall expose a function to help developer assemble that exception. + +Current server side abortion mechanism is rely on exception as well. When user called `context.abort(...)`, an designated exception will be raised. Then upstream function will catch that exception and perform abortion for the RPC call. + +PS: The custom error handler mechanism is needed for gRPC Python but unsupported yet. The correct way is to do it through fully-featured server-side interceptor in proposal [L13](https://github.com/grpc/proposal/pull/39). However, it is never got implemented. + +```Python +# Usage Snippet +import grpc_status +from google.protobuf import any_pb2 + +def ...Servicer(...): + def AMethodHandler(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.status_pb2.Status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail] + ) + + raise grpc_status.to_exception(rich_status) +``` + +#### Pros +* Follows the existing implementation mechanism (abort) +* Allows to raise exception from any layer of stack, also prevents servicer context be passed around + + +#### Cons +* Adds a new public interface +* Adds a new exception handling mechanism (@gnossen: it's magic!) +* No rich status for successful call + + +### Option 2: Server Context Status Class + +In C++/Java/Golang, the status is implemented in a cohesive class that handles status-related information. Unfortunately, in Python, current design doesn't support this. And as @ericgribkoff mentioned, Python API name mixed the concept of status message and status details. There is a `set_details` method for servicer context, but its used to set status message. So, we presumably can reorganize those information as a server-side status class as alternative method of existing methods, and deprecate those single-value setter in future. + +Also, the new interface will allow gRPC Python to validate the code, message and details provided by developer are matched. Even with the validation, developers still have the ability to abuse this API by providing arbitrary status details. + +```Python +# Add a new interface for ExtraDetails +class ExtraDetails(abc.ABCMeta): + @abc.abstractmethod + def code(self): pass + + @abc.abstractmethod + def message(self): pass + + @abc.abstractmethod + def details(self): pass + +# Usage Snippet +import grpc_status +from google.protobuf import any_pb2 + +def ...Servicer(...): + def AMethodHandler(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.assemble_status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail], + ) + + context.set_extra_status(grpc_status.convert_to_extra_details(rich_status)) + ... +``` + +#### Pros +* Expose the setting of status to developers like Java/Go +* Status-related information managed in one place +* Clean, plain design if `set_code`/`set_details` removed + +#### Cons +* Have to add a brand new `Status` class +* Ambiguity of responsibility of the new status class +* Asymmetric with client side channel design +* Needs to educate developer about priority + + +### Option 3: Call Server Context Method in New Extension Package + +In this design, the new extension package provides a single function `abort_with_status` that accepts the server context and status proto instance. It will set code, message and trailing metadata inside the function. The downside is developers have to pass a mutable instance to another package to change it. Also, the semantic left ambiguity about the abortion behavior during exception whether to continue abortion or not. + +```Python +# Usage Snippet +import grpc_status +from google.protobuf import any_pb2 + +def ...Servicer(...): + def AMethodHandler(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.status_pb2.Status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail] + ) + grpc_status.abort_with_status(context, rich_status) + # An exception will be raised. RPC call abort. +``` + +#### Pros +* Zero code change to the main package + +#### Cons +* Passing server context around (@ericgribkoff: ugly!) +* Ambiguous abortion behavior + + +### Option 4: Convert to Metadata + +The new package handles the conversion from ProtoBuf message instance to gRPC Python metadata, which is a double-value `tuple`. It is the most hands-off option, our main framework promised nothing to developers about the usage of the `grpc-status-details-bin`. + +```Python +# Usage Snippet +import grpc_status +from google.protobuf import any_pb2 + +def ...Servicer(...): + def AMethodHandler(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.status_pb2.Status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail] + ) + + context.set_code(rich_status.code) + context.set_details(rich_status.message) + context.set_trailing_metadata( + grpc_status.to_metadata(rich_status) + ) +``` + +#### Pros +* Zero code change to the main package + +#### Cons +* Verbosity +* No code/message validation at all +* Confusing definition of message/details + + +### Option 5: Add Status Details Setter + +Similar to option 4, this option is one-step further that the framework automatically set the string to the metadata entry `grpc-status-details-bin`. + +```Python +### Client side ### +stub = ...Stub(channel) +try: + resp = stub.AMethodHandler(...) +except grpc.RpcError as rpc_error: + binary_status = rpc_error.binary_status() + status = grpc_status.parse(binary_status) + # Do stuff with the status + +### Server side ### +def ...Servicer(...): + def AMethodHandler(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.status_pb2.Status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail] + ) + + context.set_code(rich_status.code) + context.set_details(rich_status.message) + context.set_binary_status(rich_status.serializeToString()) + ... +``` + +#### Pros +* Similar to C++'s strictness: code/message/details are independent +* Promised developers the usage of metadata entry `grpc-status-details-bin` + +#### Cons +* Verbosity +* No code/message validation at all +* Confusing definition of message/details + + +### Why we need a new package `grpcio-status`? + +Although we can't enforce the consistency of status code, status message and status details, there is still value for the new package to help developers validate that. It demonstrates our recommendation through the design. + +One more point is that in the future, the `google.rpc.status.Status` may not be the only status proto we accept. This package will provide easy mapping between different status proto message. + +## Implementation + +PR: https://github.com/grpc/grpc/pull/17384 + +## Open issues (if applicable) + +Issue: https://github.com/grpc/grpc/issues/16366 From 694a88d31c6cccfbd9d244b175d3be6939b2debd Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 7 Dec 2018 16:41:56 -0800 Subject: [PATCH 02/10] Learn how to count five... --- L44-python-rich-status.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index ce3e5caf6..0d50c5711 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -156,7 +156,7 @@ def ...Servicer(...): ## Rationale -There are four more alternative options of implementing this feature. +There are five more alternative options of implementing this feature. ### Option 1: A New Exception Handling Mechanism From b5f9b1a2a7b62648f56b292c75d48689e0e06843 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 10 Dec 2018 13:38:39 -0800 Subject: [PATCH 03/10] Add grpc-io thread --- L44-python-rich-status.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index 0d50c5711..0a139f852 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -4,8 +4,8 @@ New RPC Status API for Python * Approver: a11r * Status: Draft * Implemented in: Python -* Last updated: 12-06-2018 -* Discussion at: (filled after thread exists) +* Last updated: 12-10-2018 +* Discussion at: https://groups.google.com/forum/#!topic/grpc-io/8Ys6ba8gpLc ## Abstract From a6fb00448568b38613577fb08a1bce9200c87574 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 14 Dec 2018 11:15:04 -0800 Subject: [PATCH 04/10] Update the design doc * Update our final choice * Not going to swap the two concepts anymore * Mark the client-side API as optional * Correct my terrible grammar --- L44-python-rich-status.md | 181 +++++++++++++++++++++++++++----------- 1 file changed, 130 insertions(+), 51 deletions(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index 0a139f852..b873d4e92 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -9,26 +9,26 @@ New RPC Status API for Python ## Abstract -Current design of status API doesn't support rich status, also the concept of status "details" doesn't comply with gRPC spec. So, after discussion with @gnossen and @ericgribkoff, we think we should add a new set of status APIs to correct them. +The current design of status API doesn't support rich status. Also the concept of status "details" doesn't comply with the gRPC spec. So, after discussion with @gnossen and @ericgribkoff, we think we should add a new set of status APIs to correct them. ## Background -The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of a RPC call `Status` and `Status-Message`. But status code and a text message themselves are insufficient to express more complicate situation like the RPC call failed because of quota check of certain policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). +The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of an RPC call `Status` and `Status-Message`. However, status code and a text message themselves are insufficient to express more complicated situation like the RPC call failed because of quota check of specific policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). -The one major use case of this feature is Google Cloud API. The Google Cloud API needs it to transport rich status from server to client, also they are the owner of those proto files. +The one primary use case of this feature is Google Cloud API. The Google Cloud API needs it to transport rich status from the server to the client. Also, they are the owner of those proto files. -So, the gRPC team introduced a new internal trailing metadata entry `grpc-status-details-bin` to serve this purpose. It takes in serialized proto `google.rpc.status.Status` message binary and transmits as a binary metadata. However, since gRPC is a ProtoBuf-agnostic framework, it can't enforce either the content of the binary data be set in the trailing metadata entry, nor the consistency of status code/status message with the additional status detail proto message. The result of this unsolvable problem is that for three major implementation of gRPC we have, each of them are slightly different about this behavior. +So, the gRPC team introduced a new internal trailing metadata entry `grpc-status-details-bin` to serve this purpose. It takes in serialized proto `google.rpc.status.Status` message binary and transmits as binary metadata. However, since gRPC is a ProtoBuf-agnostic framework, it can't enforce either the content of the binary data be set in the trailing metadata entry, nor the consistency of status code/status message with the additional status detail proto message. The result of this unsolvable problem is that for three major implementations of gRPC we have, each of them is slightly different about this behavior. ### Behavior Different Across Implementations -C++ implementation tolerates anything to be put in the `grpc-status-details-bin` entry; Java will check if the code of the proto status is matching the status code of the RPC call; Golang will enforce the both code and message of the proto status and the RPC call be the same. +C++ implementation tolerates anything to be put in the `grpc-status-details-bin` entry; Java checks if the code of the proto status is matching the status code of the RPC call; Golang enforces both code and message of the proto status, and the RPC call be the same. So, which paradigm should Python follow? ### The Proto of Status -The proto of status is well-defined and used in numerous framework. Here is the definition of `Status`, for full version see [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). +The proto of status is well-defined and used in many frameworks. Here is the definition of `Status`, for full version see [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). ```proto message Status { int32 code = 1; @@ -64,58 +64,64 @@ def ...Servicer(...): ## Proposal -### Concept Change +### A New Public Interface: Status -**Status message** should be interpreted as a text (unicode allowed) field that indicates the status of the RPC call and it is intended to be read by the developers. +This class is used to describe the status of an RPC. -**Status details** should be understood as a encoded `google.rpc.status.Status` proto message that serves as rich status error details about the RPC Call. +The name of the class is `Status`, because it is shorter and cleaner than `RpcStatus` or `ServerRpcStatus`. In the future, we might want to utilize it at client side as well. -### Client side API - -Currently, the documentation about getting status resides in [grpc.Call](https://grpc.io/grpc/python/grpc.html#client-side-context) section. We propose to add a new member function named `status()`, it will return a `namedtuple('Status', ['code', 'message', 'details'])`. - -The usage snippet will look like: +The metadata field of `Status` is `trailing_metadata` instead of `metadata`. The `Status` should be the final status of an RPC, so it should assist developers to get information that only accessible at the end of the RPC. Also, if we want to support it at client side, we need to separate `initial_metadata` and `trailing_metadata`. ```Python -stub = ...Stub(channel) +class Status(six.with_metaclass(abc.ABCMeta)): + """Describes the status of an RPC. -try: - resp = stub.AMethodHandler(...) -except grpc.RpcError as rpc_error: - code = rpc_error.status().code - message = rpc_error.status().message - details = rpc_error.status().details - # or - code, message, details = rpc_error.status() + This is an EXPERIMENTAL API. + + Attributes: + code: A StatusCode object to be sent to the client. + details: An ASCII-encodable string to be sent to the client upon + termination of the RPC. + trailing_metadata: The trailing :term:`metadata` in the RPC. + """ ``` ### Server side API -gRPC Python team encountered lots of disagreements about how this part should be implemented. Available options will be listed in the `Rationale` section. Here I will post the final consensus for the team, but feel free to comment about other options. +gRPC Python team encountered lots of disagreements about how this part should be implemented. Available options are listed in the `Rationale` section. Here is our final consensus as a team, but feel free to comment about other options. -The new API will be added to [ServicerContext](https://grpc.io/grpc/python/grpc.html#service-side-context), and named `set_status`. It accept two positional arguments and a keyword argument, so setting the status code and status message are mandatory but not details. At the same time, current API `set_code`/`set_details` will be labeled as "deprecated". +During the discussion we come up with several criteria for server-side API: +1. Shouldn't pass `ServicerContext` around and let extension package to mutate it. +2. Should minimize the changes for our main package. +3. Should allow the status code/message validation at some point. +4. Should reserve the extensibility for future updates. +5. Should not confuse developers about the priority between the new API and old APIs. +6. Should be Pythonic. -The function signature will be: +As a result, we finally decided to add a new API named `abort_with_status` that accepts the new interface `grpc.Status`. Unlike `set_code`/`set_details` can be called multiple times, the abortion mechanism ensured the code, details, and metadata of the would be set as the final status of `grpc.Call`. ```Python -def set_status(code, message, details=""): pass -``` +def abort_with_status(self, status): + """Raises an exception to terminate the RPC with a non-OK status. -The usage will look like: + The status passed as argument will supercede any existing status code, + status message and trailing metadata. -```Python -def ...Servicer(...): - def ARPCCall(request, context): - ... - context.set_status( - grpc.StatusCode.INVALID_ARGUMENT, - "Failed to recognize input variables", - ) + This is an EXPERIMENTAL API. + + Args: + status: A grpc.Status object. The status code in it must not be + StatusCode.OK. + + Raises: + Exception: An exception is always raised to signal the abortion the + RPC to the gRPC runtime. + """ ``` ### Rich Status Usage -Besides this proposal, there will be a new gRPC Python extension package that depends on ProtoBuf named `grpcio-status`. The new package will provide convenient functions to help developers transform from ProtoBuf instance to gRPC status. The usage will look like: +Besides the change to status API, there should be a new gRPC Python extension package that depends on ProtoBuf named `grpcio-status`. The new package should provide convenient functions to help developers transform from ProtoBuf instance to gRPC status. The usage should look like: ```Python # Client side @@ -149,22 +155,39 @@ def ...Servicer(...): message='API call quota depleted', details=[detail] ) + context.abort_with_status(rpc_status.to_status(rich_status)) + # The method handler will abort +``` - context.set_status(*rpc_status.convert(rich_status)) +### Optional: Client side API + +Currently, the documentation about getting status resides in [grpc.Call](https://grpc.io/grpc/python/grpc.html#client-side-context) section. We propose to add a new member function named `status()`, it will return a `grpc.Status`. + +The usage snippet should look like: + +```Python +stub = ...Stub(channel) + +try: + resp = stub.AMethodHandler(...) +except grpc.RpcError as rpc_error: + code = rpc_error.status().code + details = rpc_error.status().details + trailing_metadata = rpc_error.status().trailing_metadata ``` ## Rationale -There are five more alternative options of implementing this feature. +There are six more alternative options for implementing this feature. ### Option 1: A New Exception Handling Mechanism -We expose a new public exception interface that developer can raise within their servicer method handler. The exception itself will contain information like status code, status message, and most importantly the rich status details. As for the extension package, we shall expose a function to help developer assemble that exception. +We expose a new public exception interface that developer can raise within their servicer method handler. The exception itself contains information like status code, status message, and most importantly the rich status details. As for the extension package, we shall expose a function to help developer assemble that exception. -Current server side abortion mechanism is rely on exception as well. When user called `context.abort(...)`, an designated exception will be raised. Then upstream function will catch that exception and perform abortion for the RPC call. +Current server side abortion mechanism relies on exception as well. When a developer called `context.abort(...)`, a designated exception raises. Then upstream function should catch that exception and perform an abortion for the RPC call. -PS: The custom error handler mechanism is needed for gRPC Python but unsupported yet. The correct way is to do it through fully-featured server-side interceptor in proposal [L13](https://github.com/grpc/proposal/pull/39). However, it is never got implemented. +PS: The custom error handler mechanism is needed for gRPC Python but unsupported yet. The correct way is to do it through fully-featured server-side interceptor in the proposal [L13](https://github.com/grpc/proposal/pull/39). However, it is never got implemented. ```Python # Usage Snippet @@ -192,20 +215,20 @@ def ...Servicer(...): #### Pros * Follows the existing implementation mechanism (abort) -* Allows to raise exception from any layer of stack, also prevents servicer context be passed around +* Allows raising the exception from any layer of the stack, also prevents servicer context be passed around #### Cons * Adds a new public interface * Adds a new exception handling mechanism (@gnossen: it's magic!) -* No rich status for successful call +* No rich status for the successful call ### Option 2: Server Context Status Class -In C++/Java/Golang, the status is implemented in a cohesive class that handles status-related information. Unfortunately, in Python, current design doesn't support this. And as @ericgribkoff mentioned, Python API name mixed the concept of status message and status details. There is a `set_details` method for servicer context, but its used to set status message. So, we presumably can reorganize those information as a server-side status class as alternative method of existing methods, and deprecate those single-value setter in future. +In C++/Java/Golang, the status is implemented in a cohesive class that handles status-related information. Unfortunately, in Python, current design doesn't support this. And as @ericgribkoff mentioned, Python API name mixed the concept of the status message and status details. There is a `set_details` method for servicer context, but its used to set a status message. So, we presumably can reorganize that information as a server-side status class an alternative method of existing methods, and deprecate those single-value setters in future. -Also, the new interface will allow gRPC Python to validate the code, message and details provided by developer are matched. Even with the validation, developers still have the ability to abuse this API by providing arbitrary status details. +Also, the new interface allows gRPC Python to validate the code, message, and details provided by the developer are matched. Even with the validation, developers still able to abuse this API by providing arbitrary status details. ```Python # Add a new interface for ExtraDetails @@ -257,7 +280,7 @@ def ...Servicer(...): ### Option 3: Call Server Context Method in New Extension Package -In this design, the new extension package provides a single function `abort_with_status` that accepts the server context and status proto instance. It will set code, message and trailing metadata inside the function. The downside is developers have to pass a mutable instance to another package to change it. Also, the semantic left ambiguity about the abortion behavior during exception whether to continue abortion or not. +In this design, the new extension package provides a single function `abort_with_status` that accepts the server context and status proto instance. It will set code, message and trailing metadata inside the function. The downside is developers have to pass a mutable instance to another package to change it. Also, the left semantic ambiguity about the abortion behavior during exception whether to continue abortion or not. ```Python # Usage Snippet @@ -293,7 +316,7 @@ def ...Servicer(...): ### Option 4: Convert to Metadata -The new package handles the conversion from ProtoBuf message instance to gRPC Python metadata, which is a double-value `tuple`. It is the most hands-off option, our main framework promised nothing to developers about the usage of the `grpc-status-details-bin`. +The new package handles the conversion from ProtoBuf message instance to gRPC Python metadata, which is a double-value `tuple`. It is the most hands-off option. Our main framework promised nothing to developers about the usage of the `grpc-status-details-bin`. ```Python # Usage Snippet @@ -379,11 +402,67 @@ def ...Servicer(...): * Confusing definition of message/details +### Option 6: Pack as Tuple & Splat if Needed + +The new API will be added to [ServicerContext](https://grpc.io/grpc/python/grpc.html#service-side-context), and named `set_status`. It accepts two positional arguments and a keyword argument, so setting the status code and status message is mandatory but not details. At the same time, current API `set_code`/`set_details` will be labeled as "deprecated". + +The function signature should be: + +```Python +def set_status(code, message, details=""): pass +``` + +The usage should look like: + +```Python +# Server side +from grpc_status import rpc_status +from google.protobuf import any_pb2 + +def ...Servicer(...): + def ARPCCall(request, context): + ... + detail = any_pb2.Any() + detail.Pack( + rpc_status.error_details_pb2.DebugInfo( + stack_entries=traceback.format_stack(), + detail="Can't recognize this argument", + ) + ) + rich_status = grpc_status.status_pb2.Status( + code=grpc_status.code_pb2.INVALID_ARGUMENT, + message='API call quota depleted', + details=[detail] + ) + + context.set_status(*rpc_status.convert(rich_status)) +``` + +#### Pros +* Concise API call +* Avoid introducing new classes + +#### Cons +* Recommend using splat is not Pythonic +* Need to educate developers about the priority of those three APIs + + ### Why we need a new package `grpcio-status`? -Although we can't enforce the consistency of status code, status message and status details, there is still value for the new package to help developers validate that. It demonstrates our recommendation through the design. +Although we can't enforce the consistency of status code, status message, and status details, there is still value for the new package to help developers validate that. It demonstrates our recommendation through the design. + +One more reason is that in the future, the `google.rpc.status.Status` may not be the only status proto we accept. This package provides a straightforward mapping between different status proto message. + + +### Not Swapping the Two Concepts + +Ideally speaking, the **status message** and **status details** are misused in Python implementation, and this proposal is our best chance to get them right. According to spec, the correct definition should be as followed: + +* **Status message** should be interpreted as a text (Unicode allowed) field that indicates the status of the RPC call, and it is intended to be read by the developers. +* **Status details** should be understood as a encoded `google.rpc.status.Status` proto message that serves as rich status error details about the RPC Call. + +However, the **status details** is deeply bound with gRPC Python since its very first version. The concept swapping may be correct, but it presumably increases developers' recognition burden. Moreover, even if we can cleanly update our document and API interface to swap those two concepts, there are tons of tutorial/articles/examples about gRPC Python on the internet we can't ever correct them. So, we think maintaining the status quo is our current optimal solution. -One more point is that in the future, the `google.rpc.status.Status` may not be the only status proto we accept. This package will provide easy mapping between different status proto message. ## Implementation From b7ef8c4363ee053a5f31e020629b7c59431ff480 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 14 Dec 2018 11:22:02 -0800 Subject: [PATCH 05/10] Update the pull requests --- L44-python-rich-status.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index b873d4e92..4816c8b75 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -466,7 +466,9 @@ However, the **status details** is deeply bound with gRPC Python since its very ## Implementation -PR: https://github.com/grpc/grpc/pull/17384 +Pull Requests: +* Part of new status API is implemented in https://github.com/grpc/grpc/pull/17481 +* The new extension package is implemented in https://github.com/grpc/grpc/pull/17490 ## Open issues (if applicable) From 8f14ebfc43b715995ebfcd02d513d6069fdc8c3c Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Thu, 24 Jan 2019 16:28:08 -0800 Subject: [PATCH 06/10] Minor fix of the snippet --- L44-python-rich-status.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index 4816c8b75..b04ba68f3 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -37,7 +37,7 @@ message Status { } ``` -### Current Python API +### Current Python API About RPC Status ```Python # Client side @@ -46,19 +46,18 @@ try: resp = stub.ARpcCall(...) except grpc.RpcError as rpc_error: code = rpc_error.code() - message = rpc_error.details() + details = rpc_error.details() # Unable to get rich status ``` ```Python # Server side def ...Servicer(...): - ... + def ARPCCall(request, context): - ... context.set_code(...) context.set_details(...) - ... + # Unable to set rich status ``` From 468c1143427df65ac4f44ff8a8a9ae0a67b25c71 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Thu, 21 Feb 2019 10:49:19 -0800 Subject: [PATCH 07/10] Update the reviewer to ericgribkoff@ --- L44-python-rich-status.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index b04ba68f3..1d6dd67bd 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -1,7 +1,7 @@ New RPC Status API for Python ---- * Author(s): lidizheng -* Approver: a11r +* Approver: ericgribkoff * Status: Draft * Implemented in: Python * Last updated: 12-10-2018 From 2013adb857a8a2420a34a69993b6577685f41ddd Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 22 Feb 2019 15:07:25 -0800 Subject: [PATCH 08/10] Adopt reviewer's suggestions --- L44-python-rich-status.md | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index 1d6dd67bd..959d3993f 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -9,16 +9,15 @@ New RPC Status API for Python ## Abstract -The current design of status API doesn't support rich status. Also the concept of status "details" doesn't comply with the gRPC spec. So, after discussion with @gnossen and @ericgribkoff, we think we should add a new set of status APIs to correct them. - +The current design of the gRPC Python servicer API for RPCs terminated with a non-OK status does not easily allow pairing the failure with associated additional information not expressible by the `grpc-status` and `grpc-message` fields. This proposal adds a rich `Status` class to gRPC Python that allows coupling the failure's status code, error string, and arbitrary additional information about the failure (sent as trailing metadata) in a unified interface. ## Background -The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of an RPC call `Status` and `Status-Message`. However, status code and a text message themselves are insufficient to express more complicated situation like the RPC call failed because of quota check of specific policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). +The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of an RPC call `grpc-status` and `grpc-Message`. However, status code and a text message themselves are insufficient to express more complicated situation like the RPC call failed because of quota check of specific policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). -The one primary use case of this feature is Google Cloud API. The Google Cloud API needs it to transport rich status from the server to the client. Also, they are the owner of those proto files. +The one primary use case of this feature is Google Cloud API. The Google Cloud API needs it to transport rich status from the server to the client. -So, the gRPC team introduced a new internal trailing metadata entry `grpc-status-details-bin` to serve this purpose. It takes in serialized proto `google.rpc.status.Status` message binary and transmits as binary metadata. However, since gRPC is a ProtoBuf-agnostic framework, it can't enforce either the content of the binary data be set in the trailing metadata entry, nor the consistency of status code/status message with the additional status detail proto message. The result of this unsolvable problem is that for three major implementations of gRPC we have, each of them is slightly different about this behavior. +So, the gRPC team introduced a new internal trailing metadata entry `grpc-status-details-bin` to serve this purpose. It takes in serialized proto `google.rpc.status.Status` message binary and transmits as binary metadata. Notably, the `google.rpc.status.Status` message contains its own `code` and `message` fields. However, since gRPC is a ProtoBuf-agnostic framework, it can't enforce either the content of the binary data set in the trailing metadata entry, nor the consistency of status code/status message with the additional status detail proto message. The result of this unsolvable problem is that for three major implementations of gRPC we have, each of them is slightly different about this behavior. ### Behavior Different Across Implementations @@ -26,7 +25,7 @@ C++ implementation tolerates anything to be put in the `grpc-status-details-bin` So, which paradigm should Python follow? -### The Proto of Status +### Status Proto The proto of status is well-defined and used in many frameworks. Here is the definition of `Status`, for full version see [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). ```proto @@ -69,7 +68,7 @@ This class is used to describe the status of an RPC. The name of the class is `Status`, because it is shorter and cleaner than `RpcStatus` or `ServerRpcStatus`. In the future, we might want to utilize it at client side as well. -The metadata field of `Status` is `trailing_metadata` instead of `metadata`. The `Status` should be the final status of an RPC, so it should assist developers to get information that only accessible at the end of the RPC. Also, if we want to support it at client side, we need to separate `initial_metadata` and `trailing_metadata`. +The metadata field of `Status` is `trailing_metadata` instead of `metadata`. The `Status` should be the final status of an RPC, so it should assist developers to get information that only accessible at the end of the RPC. ```Python class Status(six.with_metaclass(abc.ABCMeta)): @@ -87,7 +86,7 @@ class Status(six.with_metaclass(abc.ABCMeta)): ### Server side API -gRPC Python team encountered lots of disagreements about how this part should be implemented. Available options are listed in the `Rationale` section. Here is our final consensus as a team, but feel free to comment about other options. +Available options are listed in the `Rationale` section. Here is our final consensus as a team, but feel free to comment about other options. During the discussion we come up with several criteria for server-side API: 1. Shouldn't pass `ServicerContext` around and let extension package to mutate it. @@ -158,23 +157,6 @@ def ...Servicer(...): # The method handler will abort ``` -### Optional: Client side API - -Currently, the documentation about getting status resides in [grpc.Call](https://grpc.io/grpc/python/grpc.html#client-side-context) section. We propose to add a new member function named `status()`, it will return a `grpc.Status`. - -The usage snippet should look like: - -```Python -stub = ...Stub(channel) - -try: - resp = stub.AMethodHandler(...) -except grpc.RpcError as rpc_error: - code = rpc_error.status().code - details = rpc_error.status().details - trailing_metadata = rpc_error.status().trailing_metadata -``` - ## Rationale @@ -448,7 +430,7 @@ def ...Servicer(...): ### Why we need a new package `grpcio-status`? -Although we can't enforce the consistency of status code, status message, and status details, there is still value for the new package to help developers validate that. It demonstrates our recommendation through the design. +Although the grpcio package can't enforce the consistency of status code and status message with the information embedded in the rich `Status` proto, there is still value to provide an additional package - one which can depend on protobuf and is able to verify that these elements are consistent - to help developers use the API in its intended fashion.. It demonstrates our recommendation through the design. One more reason is that in the future, the `google.rpc.status.Status` may not be the only status proto we accept. This package provides a straightforward mapping between different status proto message. From 1fcd3e9e653b7b49d32308b76278a3685af79a53 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 22 Feb 2019 15:24:23 -0800 Subject: [PATCH 09/10] Rewrite the last section --- L44-python-rich-status.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index 959d3993f..15ceb0adc 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -435,14 +435,9 @@ Although the grpcio package can't enforce the consistency of status code and sta One more reason is that in the future, the `google.rpc.status.Status` may not be the only status proto we accept. This package provides a straightforward mapping between different status proto message. -### Not Swapping the Two Concepts +### Semantic difference of `details` between gRPC Python and `Status` proto -Ideally speaking, the **status message** and **status details** are misused in Python implementation, and this proposal is our best chance to get them right. According to spec, the correct definition should be as followed: - -* **Status message** should be interpreted as a text (Unicode allowed) field that indicates the status of the RPC call, and it is intended to be read by the developers. -* **Status details** should be understood as a encoded `google.rpc.status.Status` proto message that serves as rich status error details about the RPC Call. - -However, the **status details** is deeply bound with gRPC Python since its very first version. The concept swapping may be correct, but it presumably increases developers' recognition burden. Moreover, even if we can cleanly update our document and API interface to swap those two concepts, there are tons of tutorial/articles/examples about gRPC Python on the internet we can't ever correct them. So, we think maintaining the status quo is our current optimal solution. +gRPC Python uses the field name `details` to store data that will later be transmitted in `grpc-message` header. But the `details` field in proto `Status` is a list for error details proto messages. Unfortunately, this discrepancy is deeply embedded in the gRPC Python API and unlikely to be changed. ## Implementation From 3787bda63dd62ccb60dd0dd93e895d166def12a0 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Fri, 22 Feb 2019 15:50:18 -0800 Subject: [PATCH 10/10] Fix typo --- L44-python-rich-status.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L44-python-rich-status.md b/L44-python-rich-status.md index 15ceb0adc..32b3eda2e 100644 --- a/L44-python-rich-status.md +++ b/L44-python-rich-status.md @@ -13,7 +13,7 @@ The current design of the gRPC Python servicer API for RPCs terminated with a no ## Background -The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of an RPC call `grpc-status` and `grpc-Message`. However, status code and a text message themselves are insufficient to express more complicated situation like the RPC call failed because of quota check of specific policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). +The gRPC [Spec](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) defined two trailing data to indicate the status of an RPC call `grpc-status` and `grpc-message`. However, status code and a text message themselves are insufficient to express more complicated situation like the RPC call failed because of quota check of specific policy, or the stack trace from the crashed server, or obtain retry delay from the server (see [error details](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto)). The one primary use case of this feature is Google Cloud API. The Google Cloud API needs it to transport rich status from the server to the client.