Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 96
feat: Add error_details property to GoogleAPICallError based on google.rpc.status.details.#286
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
288019c04b57755cebe8e5b3f7c17b7504dfe708ae813fc33d10157f88e6ffab6c9e5602b082a12edc5a626f7dea844feda830887File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -25,10 +25,14 @@ | ||
| from typing import Dict | ||
| from typing import Union | ||
| from google.rpc import error_details_pb2 | ||
| try: | ||
| import grpc | ||
| from grpc_status import rpc_status | ||
tseaver marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| except ImportError: # pragma: NO COVER | ||
| grpc = None | ||
| rpc_status = None | ||
| # Lookup tables for mapping exceptions from HTTP and gRPC transports. | ||
| # Populated by _GoogleAPICallErrorMeta | ||
| @@ -97,6 +101,7 @@ class GoogleAPICallError(GoogleAPIError, metaclass=_GoogleAPICallErrorMeta): | ||
| Args: | ||
| message (str): The exception message. | ||
| errors (Sequence[Any]): An optional list of error details. | ||
| details (Sequence[Any]): An optional list of objects defined in google.rpc.error_details. | ||
| response (Union[requests.Request, grpc.Call]): The response or | ||
| gRPC call metadata. | ||
| """ | ||
| @@ -117,15 +122,19 @@ class GoogleAPICallError(GoogleAPIError, metaclass=_GoogleAPICallErrorMeta): | ||
| This may be ``None`` if the exception does not match up to a gRPC error. | ||
| """ | ||
| def __init__(self, message, errors=(), response=None): | ||
| def __init__(self, message, errors=(), details=(), response=None): | ||
tseaver marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| super(GoogleAPICallError, self).__init__(message) | ||
| self.message = message | ||
| """str: The exception message.""" | ||
| self._errors = errors | ||
| self._details = details | ||
| self._response = response | ||
| def __str__(self): | ||
| return "{} {}".format(self.code, self.message) | ||
| if self.details: | ||
| return "{} {} {}".format(self.code, self.message, self.details) | ||
| else: | ||
| return "{} {}".format(self.code, self.message) | ||
| @property | ||
| def errors(self): | ||
| @@ -136,6 +145,19 @@ def errors(self): | ||
| """ | ||
| return list(self._errors) | ||
| @property | ||
| def details(self): | ||
| """Information contained in google.rpc.status.details. | ||
| Reference: | ||
| https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto | ||
| https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto | ||
| Returns: | ||
| Sequence[Any]: A list of structured objects from error_details.proto | ||
| """ | ||
| return list(self._details) | ||
| @property | ||
| def response(self): | ||
| """Optional[Union[requests.Request, grpc.Call]]: The response or | ||
| @@ -409,13 +431,15 @@ def from_http_response(response): | ||
| error_message = payload.get("error", {}).get("message", "unknown error") | ||
| errors = payload.get("error", {}).get("errors", ()) | ||
| # In JSON, details are already formatted in developer-friendly way. | ||
| details = payload.get("error", {}).get("details", ()) | ||
| message = "{method} {url}: {error}".format( | ||
| method=response.request.method, url=response.request.url, error=error_message | ||
| ) | ||
| exception = from_http_status( | ||
| response.status_code, message, errors=errors, response=response | ||
| response.status_code, message, errors=errors, details=details, response=response | ||
| ) | ||
| return exception | ||
| @@ -462,6 +486,37 @@ def _is_informative_grpc_error(rpc_exc): | ||
| return hasattr(rpc_exc, "code") and hasattr(rpc_exc, "details") | ||
| def _parse_grpc_error_details(rpc_exc): | ||
| status = rpc_status.from_call(rpc_exc) | ||
| if not status: | ||
| return [] | ||
| possible_errors = [ | ||
| error_details_pb2.BadRequest, | ||
| error_details_pb2.PreconditionFailure, | ||
| error_details_pb2.QuotaFailure, | ||
| error_details_pb2.ErrorInfo, | ||
| error_details_pb2.RetryInfo, | ||
| error_details_pb2.ResourceInfo, | ||
| error_details_pb2.RequestInfo, | ||
| error_details_pb2.DebugInfo, | ||
| error_details_pb2.Help, | ||
| error_details_pb2.LocalizedMessage, | ||
| ] | ||
tseaver marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| error_details = [] | ||
| for detail in status.details: | ||
| matched_detail_cls = list( | ||
| filter(lambda x: detail.Is(x.DESCRIPTOR), possible_errors) | ||
| ) | ||
| # If nothing matched, use detail directly. | ||
| if len(matched_detail_cls) == 0: | ||
| info = detail | ||
tseaver marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| else: | ||
| info = matched_detail_cls[0]() | ||
| detail.Unpack(info) | ||
| error_details.append(info) | ||
| return error_details | ||
| def from_grpc_error(rpc_exc): | ||
| """Create a :class:`GoogleAPICallError` from a :class:`grpc.RpcError`. | ||
| @@ -476,7 +531,11 @@ def from_grpc_error(rpc_exc): | ||
| # However, check for grpc.RpcError breaks backward compatibility. | ||
| if isinstance(rpc_exc, grpc.Call) or _is_informative_grpc_error(rpc_exc): | ||
| return from_grpc_status( | ||
| rpc_exc.code(), rpc_exc.details(), errors=(rpc_exc,), response=rpc_exc | ||
| rpc_exc.code(), | ||
| rpc_exc.details(), | ||
| errors=(rpc_exc,), | ||
| details=_parse_grpc_error_details(rpc_exc), | ||
| response=rpc_exc, | ||
| ) | ||
| else: | ||
| return GoogleAPICallError(str(rpc_exc), errors=(rpc_exc,), response=rpc_exc) | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -5,7 +5,7 @@ | ||||||
| # | ||||||
| # e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev", | ||||||
| # Then this file should have foo==1.14.0 | ||||||
| googleapis-common-protos==1.6.0 | ||||||
| googleapis-common-protos==1.52.0 | ||||||
| protobuf==3.12.0 | ||||||
| google-auth==1.25.0 | ||||||
| requests==2.18.0 | ||||||
| @@ -14,3 +14,4 @@ packaging==14.3 | ||||||
| grpcio==1.33.2 | ||||||
| grpcio-gcp==0.2.2 | ||||||
| grpcio-gcp==0.2.2 | ||||||
| grpcio-status==1.33.2 | ||||||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(to match the lower bound in setup.py) Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit confusing. I initially put 1.0.0 lower bound in setup.py without giving it too much thought. As we found out later, grpcio-status requires the same version as grpc (see]). To avoid confusion, I say we should change lower bound in ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed it to 1.33.2 | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -21,10 +21,13 @@ | ||
| try: | ||
| import grpc | ||
| from grpc_status import rpc_status | ||
| except ImportError: | ||
| grpc = None | ||
| grpc = rpc_status = None | ||
| from google.api_core import exceptions | ||
| from google.protobuf import any_pb2, json_format | ||
| from google.rpc import error_details_pb2, status_pb2 | ||
| def test_create_google_cloud_error(): | ||
| @@ -38,11 +41,8 @@ def test_create_google_cloud_error(): | ||
| def test_create_google_cloud_error_with_args(): | ||
| error = { | ||
| "domain": "global", | ||
| "location": "test", | ||
| "locationType": "testing", | ||
| "code": 600, | ||
| "message": "Testing", | ||
| "reason": "test", | ||
| } | ||
| response = mock.sentinel.response | ||
| exception = exceptions.GoogleAPICallError("Testing", [error], response=response) | ||
| @@ -235,3 +235,91 @@ def test_from_grpc_error_non_call(): | ||
| assert exception.message == message | ||
| assert exception.errors == [error] | ||
| assert exception.response == error | ||
| def create_bad_request_details(): | ||
| bad_request_details = error_details_pb2.BadRequest() | ||
| field_violation = bad_request_details.field_violations.add() | ||
| field_violation.field = "document.content" | ||
| field_violation.description = "Must have some text content to annotate." | ||
| status_detail = any_pb2.Any() | ||
| status_detail.Pack(bad_request_details) | ||
| return status_detail | ||
| def test_error_details_from_rest_response(): | ||
| bad_request_detail = create_bad_request_details() | ||
| status = status_pb2.Status() | ||
| status.code = 3 | ||
| status.message = ( | ||
| "3 INVALID_ARGUMENT: One of content, or gcs_content_uri must be set." | ||
| ) | ||
| status.details.append(bad_request_detail) | ||
| # See JSON schema in https://cloud.google.com/apis/design/errors#http_mapping | ||
| http_response = make_response( | ||
| json.dumps({"error": json.loads(json_format.MessageToJson(status))}).encode( | ||
| "utf-8" | ||
| ) | ||
| ) | ||
| exception = exceptions.from_http_response(http_response) | ||
| want_error_details = [json.loads(json_format.MessageToJson(bad_request_detail))] | ||
| assert want_error_details == exception.details | ||
| # 404 POST comes from make_response. | ||
| assert str(exception) == ( | ||
| "404 POST https://example.com/: 3 INVALID_ARGUMENT:" | ||
| " One of content, or gcs_content_uri must be set." | ||
| " [{'@type': 'type.googleapis.com/google.rpc.BadRequest'," | ||
| " 'fieldViolations': [{'field': 'document.content'," | ||
| " 'description': 'Must have some text content to annotate.'}]}]" | ||
| ) | ||
| def test_error_details_from_v1_rest_response(): | ||
| response = make_response( | ||
| json.dumps( | ||
| {"error": {"message": "\u2019 message", "errors": ["1", "2"]}} | ||
| ).encode("utf-8") | ||
| ) | ||
| exception = exceptions.from_http_response(response) | ||
| assert exception.details == [] | ||
| @pytest.mark.skipif(grpc is None, reason="gRPC not importable") | ||
| def test_error_details_from_grpc_response(): | ||
atulep marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| status = rpc_status.status_pb2.Status() | ||
| status.code = 3 | ||
| status.message = ( | ||
| "3 INVALID_ARGUMENT: One of content, or gcs_content_uri must be set." | ||
| ) | ||
| status_detail = create_bad_request_details() | ||
| status.details.append(status_detail) | ||
| # Actualy error doesn't matter as long as its grpc.Call, | ||
| # because from_call is mocked. | ||
| error = mock.create_autospec(grpc.Call, instance=True) | ||
| with mock.patch("grpc_status.rpc_status.from_call") as m: | ||
| m.return_value = status | ||
| exception = exceptions.from_grpc_error(error) | ||
| bad_request_detail = error_details_pb2.BadRequest() | ||
| status_detail.Unpack(bad_request_detail) | ||
| assert exception.details == [bad_request_detail] | ||
| @pytest.mark.skipif(grpc is None, reason="gRPC not importable") | ||
| def test_error_details_from_grpc_response_unknown_error(): | ||
atulep marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| status_detail = any_pb2.Any() | ||
| status = rpc_status.status_pb2.Status() | ||
| status.code = 3 | ||
| status.message = ( | ||
| "3 INVALID_ARGUMENT: One of content, or gcs_content_uri must be set." | ||
| ) | ||
| status.details.append(status_detail) | ||
| error = mock.create_autospec(grpc.Call, instance=True) | ||
| with mock.patch("grpc_status.rpc_status.from_call") as m: | ||
| m.return_value = status | ||
| exception = exceptions.from_grpc_error(error) | ||
| assert exception.details == [status_detail] | ||
Uh oh!
There was an error while loading. Please reload this page.