From 288019cb82c16e08703c0a364405c01815c236b0 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Tue, 5 Oct 2021 11:53:17 -0700 Subject: [PATCH 01/11] feat: adds support for error details. --- google/api_core/exceptions.py | 69 ++++++++++++++++++++++++++++++++++- setup.py | 2 +- tests/unit/test_exceptions.py | 44 +++++++++++++++++++++- 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index b0909f1e..7c8fecd3 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -22,12 +22,20 @@ from __future__ import unicode_literals import http.client +from google import rpc +from google.rpc import status_pb2 +from google.rpc import error_details_pb2 +from google.protobuf import json_format +import pprint +import json try: import grpc + from grpc_status import rpc_status except ImportError: # pragma: NO COVER grpc = None + rpc_status = None # Lookup tables for mapping exceptions from HTTP and gRPC transports. # Populated by _GoogleAPICallErrorMeta @@ -124,7 +132,7 @@ def __init__(self, message, errors=(), response=None): self._response = response def __str__(self): - return "{} {}".format(self.code, self.message) + return "{} {} {}".format(self.code, self.message, self.error_details) @property def errors(self): @@ -135,6 +143,65 @@ def errors(self): """ return list(self._errors) + def _parse_status(self, rpc_call) -> status_pb2.Status: + if grpc and isinstance(rpc_call, grpc.Call): + return rpc_status.from_call(rpc_call) + if not isinstance(rpc_call, dict): + return None + # Per HTTP mapping guide, rpc Status should be in + # error field of the response object, unless it's a + # v1 format. + # Ref: + # https://cloud.google.com/apis/design/errors#http_mapping + if rpc_call.get("error", None) is None: + # v1 error format, no status. + return None + status = rpc_call["error"] + status_pb = status_pb2.Status() + json_string = json.dumps(status) + json_format.Parse(json_string, status_pb) + return status_pb + + @property + def error_details(self): + """Detailed error information. + + Returns: + A list of additional error details. + """ + if getattr(self, "_error_details", None): + return self._error_details + error_details = [] + 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, + ] + for rpc_error in self._errors: + status = self._parse_status(rpc_error) + if status is None: + continue + 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 + else: + info = matched_detail_cls[0]() + detail.Unpack(info) + error_details.append(info) + self._error_details = error_details + return error_details + @property def response(self): """Optional[Union[requests.Request, grpc.Call]]: The response or diff --git a/setup.py b/setup.py index 48c8979f..c6cd70ac 100644 --- a/setup.py +++ b/setup.py @@ -36,7 +36,7 @@ "setuptools >= 40.3.0", ] extras = { - "grpc": "grpcio >= 1.33.2, < 2.0dev", + "grpc": ["grpcio >= 1.33.2, < 2.0dev", "grpcio-status >= 1.0.0, < 2.0dev"], "grpcgcp": "grpcio-gcp >= 0.2.2", "grpcio-gcp": "grpcio-gcp >= 0.2.2", } diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index e9709f26..f3eab8e5 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -14,10 +14,14 @@ import http.client import json +from google.rpc.status_pb2 import Status import grpc import mock import requests +from google.rpc import error_details_pb2 +from google.protobuf import any_pb2, json_format +from grpc_status import rpc_status from google.api_core import exceptions @@ -25,7 +29,7 @@ def test_create_google_cloud_error(): exception = exceptions.GoogleAPICallError("Testing") exception.code = 600 - assert str(exception) == "600 Testing" + assert str(exception) == "600 Testing []" assert exception.message == "Testing" assert exception.errors == [] assert exception.response is None @@ -42,7 +46,7 @@ def test_create_google_cloud_error_with_args(): response = mock.sentinel.response exception = exceptions.GoogleAPICallError("Testing", [error], response=response) exception.code = 600 - assert str(exception) == "600 Testing" + assert str(exception) == "600 Testing []" assert exception.message == "Testing" assert exception.errors == [error] assert exception.response == response @@ -224,3 +228,39 @@ def test_from_grpc_error_non_call(): assert exception.message == message assert exception.errors == [error] assert exception.response == error + + +def test_error_details_from_rest_response(): + 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) + + 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) + + # See 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 = [bad_request_details] + assert want_error_details == exception.error_details + + +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.error_details == [] From 04b57755208dfaed0217aee31eb72749da6b3b6b Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Fri, 8 Oct 2021 13:17:24 -0700 Subject: [PATCH 02/11] feat: changes how REST errors constructed. Also, adds more tests for gRPC and REST. --- google/api_core/exceptions.py | 111 ++++++++++------------- tests/asyncio/test_grpc_helpers_async.py | 2 + tests/unit/test_exceptions.py | 43 +++++++-- tests/unit/test_grpc_helpers.py | 3 + 4 files changed, 88 insertions(+), 71 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 7c8fecd3..81971ab9 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -22,12 +22,7 @@ from __future__ import unicode_literals import http.client -from google import rpc -from google.rpc import status_pb2 from google.rpc import error_details_pb2 -from google.protobuf import json_format -import pprint -import json try: import grpc @@ -124,11 +119,12 @@ 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): super(GoogleAPICallError, self).__init__(message) self.message = message """str: The exception message.""" self._errors = errors + self._details = details self._response = response def __str__(self): @@ -143,64 +139,18 @@ def errors(self): """ return list(self._errors) - def _parse_status(self, rpc_call) -> status_pb2.Status: - if grpc and isinstance(rpc_call, grpc.Call): - return rpc_status.from_call(rpc_call) - if not isinstance(rpc_call, dict): - return None - # Per HTTP mapping guide, rpc Status should be in - # error field of the response object, unless it's a - # v1 format. - # Ref: - # https://cloud.google.com/apis/design/errors#http_mapping - if rpc_call.get("error", None) is None: - # v1 error format, no status. - return None - status = rpc_call["error"] - status_pb = status_pb2.Status() - json_string = json.dumps(status) - json_format.Parse(json_string, status_pb) - return status_pb - @property def error_details(self): - """Detailed error information. + """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: - A list of additional error details. + Sequence[Any]: A list of structured objects from error_details.proto """ - if getattr(self, "_error_details", None): - return self._error_details - error_details = [] - 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, - ] - for rpc_error in self._errors: - status = self._parse_status(rpc_error) - if status is None: - continue - 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 - else: - info = matched_detail_cls[0]() - detail.Unpack(info) - error_details.append(info) - self._error_details = error_details - return error_details + return list(self._details) @property def response(self): @@ -475,13 +425,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 @@ -528,6 +480,41 @@ def _is_informative_grpc_error(rpc_exc): return hasattr(rpc_exc, "code") and hasattr(rpc_exc, "details") +def _parse_grpc_error_details(rpc_exc): + if not rpc_status: + return [] + if not isinstance(rpc_exc, grpc.Call): + return [] + 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, + ] + 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 + 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`. @@ -542,7 +529,9 @@ 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) diff --git a/tests/asyncio/test_grpc_helpers_async.py b/tests/asyncio/test_grpc_helpers_async.py index f0b6c5f1..c412dfba 100644 --- a/tests/asyncio/test_grpc_helpers_async.py +++ b/tests/asyncio/test_grpc_helpers_async.py @@ -33,6 +33,8 @@ def code(self): def details(self): return None + def trailing_metadata(self): + return None @pytest.mark.asyncio async def test_wrap_unary_errors(): diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index f3eab8e5..406683a7 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -14,17 +14,18 @@ import http.client import json +from google.rpc import status_pb2 from google.rpc.status_pb2 import Status import grpc import mock import requests from google.rpc import error_details_pb2 +from google.rpc import code_pb2 from google.protobuf import any_pb2, json_format from grpc_status import rpc_status - from google.api_core import exceptions - +from google.protobuf import text_format def test_create_google_cloud_error(): exception = exceptions.GoogleAPICallError("Testing") @@ -37,11 +38,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) @@ -230,29 +228,33 @@ def test_from_grpc_error_non_call(): assert exception.response == error -def test_error_details_from_rest_response(): +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 = 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) + status.details.append(bad_request_detail) - # See schema in https://cloud.google.com/apis/design/errors#http_mapping + # 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 = [bad_request_details] + want_error_details = [json.loads(json_format.MessageToJson(bad_request_detail))] assert want_error_details == exception.error_details @@ -264,3 +266,24 @@ def test_error_details_from_v1_rest_response(): ) exception = exceptions.from_http_response(response) assert exception.error_details == [] + + +def test_error_details_from_grpc_response(): + 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.error_details == [bad_request_detail] \ No newline at end of file diff --git a/tests/unit/test_grpc_helpers.py b/tests/unit/test_grpc_helpers.py index 12bf1849..ae4f08fa 100644 --- a/tests/unit/test_grpc_helpers.py +++ b/tests/unit/test_grpc_helpers.py @@ -52,6 +52,9 @@ def code(self): def details(self): return None + def trailing_metadata(self): + return None + def test_wrap_unary_errors(): grpc_error = RpcErrorImpl(grpc.StatusCode.INVALID_ARGUMENT) From 5cebe8e7b72685070bf66004ff1aaea0f23dd684 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Mon, 11 Oct 2021 10:17:21 -0700 Subject: [PATCH 03/11] chore: updates versions for deps. --- setup.py | 2 +- testing/constraints-3.6.txt | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index c6cd70ac..e99c7da2 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ # 'Development Status :: 5 - Production/Stable' release_status = "Development Status :: 5 - Production/Stable" dependencies = [ - "googleapis-common-protos >= 1.6.0, < 2.0dev", + "googleapis-common-protos >= 1.52.0, < 2.0dev", "protobuf >= 3.12.0", "google-auth >= 1.25.0, < 3.0dev", "requests >= 2.18.0, < 3.0.0dev", diff --git a/testing/constraints-3.6.txt b/testing/constraints-3.6.txt index 744e991b..0c2a07b6 100644 --- a/testing/constraints-3.6.txt +++ b/testing/constraints-3.6.txt @@ -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 From 7b7504d2f91ff3ddc2b1ac298dbcb37bcaf77890 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Tue, 12 Oct 2021 12:21:42 -0700 Subject: [PATCH 04/11] chore: addresses pr comments. --- google/api_core/exceptions.py | 16 +++++---- tests/asyncio/test_grpc_helpers_async.py | 1 + tests/unit/test_exceptions.py | 43 +++++++++++++++--------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 81971ab9..449ce2e0 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -99,6 +99,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. """ @@ -128,7 +129,10 @@ def __init__(self, message, errors=(), details=(), response=None): self._response = response def __str__(self): - return "{} {} {}".format(self.code, self.message, self.error_details) + if self.error_details: + return "{} {} {}".format(self.code, self.message, self.error_details) + else: + return "{} {}".format(self.code, self.message) @property def errors(self): @@ -481,9 +485,7 @@ def _is_informative_grpc_error(rpc_exc): def _parse_grpc_error_details(rpc_exc): - if not rpc_status: - return [] - if not isinstance(rpc_exc, grpc.Call): + if not rpc_status or not isinstance(rpc_exc, grpc.Call): return [] status = rpc_status.from_call(rpc_exc) if not status: @@ -529,9 +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,), + rpc_exc.code(), + rpc_exc.details(), + errors=(rpc_exc,), details=_parse_grpc_error_details(rpc_exc), - response=rpc_exc + response=rpc_exc, ) else: return GoogleAPICallError(str(rpc_exc), errors=(rpc_exc,), response=rpc_exc) diff --git a/tests/asyncio/test_grpc_helpers_async.py b/tests/asyncio/test_grpc_helpers_async.py index c412dfba..2aaf40f7 100644 --- a/tests/asyncio/test_grpc_helpers_async.py +++ b/tests/asyncio/test_grpc_helpers_async.py @@ -36,6 +36,7 @@ def details(self): def trailing_metadata(self): return None + @pytest.mark.asyncio async def test_wrap_unary_errors(): grpc_error = RpcErrorImpl(grpc.StatusCode.INVALID_ARGUMENT) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 406683a7..70dac45b 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -14,23 +14,26 @@ import http.client import json -from google.rpc import status_pb2 -from google.rpc.status_pb2 import Status - -import grpc import mock import requests -from google.rpc import error_details_pb2 -from google.rpc import code_pb2 + +from google.rpc import error_details_pb2, status_pb2 from google.protobuf import any_pb2, json_format -from grpc_status import rpc_status + +try: + import grpc + from grpc_status import rpc_status +except ImportError: + grpc = None + grpc_status = None + from google.api_core import exceptions -from google.protobuf import text_format + def test_create_google_cloud_error(): exception = exceptions.GoogleAPICallError("Testing") exception.code = 600 - assert str(exception) == "600 Testing []" + assert str(exception) == "600 Testing" assert exception.message == "Testing" assert exception.errors == [] assert exception.response is None @@ -44,7 +47,7 @@ def test_create_google_cloud_error_with_args(): response = mock.sentinel.response exception = exceptions.GoogleAPICallError("Testing", [error], response=response) exception.code = 600 - assert str(exception) == "600 Testing []" + assert str(exception) == "600 Testing" assert exception.message == "Testing" assert exception.errors == [error] assert exception.response == response @@ -236,11 +239,11 @@ def create_bad_request_details(): 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 = rpc_status.status_pb2.Status() + status = status_pb2.Status() status.code = 3 status.message = ( "3 INVALID_ARGUMENT: One of content, or gcs_content_uri must be set." @@ -256,6 +259,14 @@ def test_error_details_from_rest_response(): exception = exceptions.from_http_response(http_response) want_error_details = [json.loads(json_format.MessageToJson(bad_request_detail))] assert want_error_details == exception.error_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(): @@ -269,6 +280,8 @@ def test_error_details_from_v1_rest_response(): def test_error_details_from_grpc_response(): + if not rpc_status: + return status = rpc_status.status_pb2.Status() status.code = 3 status.message = ( @@ -280,10 +293,10 @@ def test_error_details_from_grpc_response(): # 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: + 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.error_details == [bad_request_detail] \ No newline at end of file + assert exception.error_details == [bad_request_detail] From 813fc333c420b5215c25f0f7a8b2be8cb3b5efee Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Tue, 12 Oct 2021 14:42:02 -0700 Subject: [PATCH 05/11] chore: address pr comments. --- google/api_core/exceptions.py | 2 -- tests/unit/test_exceptions.py | 19 +++++++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 449ce2e0..baa7f980 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -485,8 +485,6 @@ def _is_informative_grpc_error(rpc_exc): def _parse_grpc_error_details(rpc_exc): - if not rpc_status or not isinstance(rpc_exc, grpc.Call): - return [] status = rpc_status.from_call(rpc_exc) if not status: return [] diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 70dac45b..63950b4e 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -280,8 +280,6 @@ def test_error_details_from_v1_rest_response(): def test_error_details_from_grpc_response(): - if not rpc_status: - return status = rpc_status.status_pb2.Status() status.code = 3 status.message = ( @@ -300,3 +298,20 @@ def test_error_details_from_grpc_response(): bad_request_detail = error_details_pb2.BadRequest() status_detail.Unpack(bad_request_detail) assert exception.error_details == [bad_request_detail] + + +def test_error_details_from_grpc_response_unknown_error(): + 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.error_details == [status_detail] From d10157fd473a679679c36fda501bd41fae82f620 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Wed, 13 Oct 2021 14:41:38 -0700 Subject: [PATCH 06/11] chore: remove try/catch for imports in test. --- tests/unit/test_exceptions.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 63950b4e..6df01a10 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -19,13 +19,8 @@ from google.rpc import error_details_pb2, status_pb2 from google.protobuf import any_pb2, json_format - -try: - import grpc - from grpc_status import rpc_status -except ImportError: - grpc = None - grpc_status = None +import grpc +from grpc_status import rpc_status from google.api_core import exceptions From 88e6ffa02becd0e51d732518262cb4786921f0aa Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Thu, 14 Oct 2021 10:58:43 -0700 Subject: [PATCH 07/11] chore: fix lint. --- google/api_core/exceptions.py | 1 + tests/unit/test_exceptions.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index baa7f980..1cbe98b7 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -22,6 +22,7 @@ from __future__ import unicode_literals import http.client + from google.rpc import error_details_pb2 try: diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 6df01a10..ef9246b1 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -14,15 +14,15 @@ import http.client import json -import mock -import requests -from google.rpc import error_details_pb2, status_pb2 -from google.protobuf import any_pb2, json_format import grpc from grpc_status import rpc_status +import mock +import requests 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(): From 02b082a667fe0b73907b670ebbfac606a11d74e7 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Mon, 18 Oct 2021 10:20:16 -0700 Subject: [PATCH 08/11] chore: changes "error_details" to "details". --- google/api_core/exceptions.py | 6 +++--- tests/unit/test_exceptions.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index c699aa1d..fdb21090 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -131,8 +131,8 @@ def __init__(self, message, errors=(), details=(), response=None): self._response = response def __str__(self): - if self.error_details: - return "{} {} {}".format(self.code, self.message, self.error_details) + if self.details: + return "{} {} {}".format(self.code, self.message, self.details) else: return "{} {}".format(self.code, self.message) @@ -146,7 +146,7 @@ def errors(self): return list(self._errors) @property - def error_details(self): + def details(self): """Information contained in google.rpc.status.details. Reference: diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index ef9246b1..962e34b7 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -253,7 +253,7 @@ def test_error_details_from_rest_response(): ) exception = exceptions.from_http_response(http_response) want_error_details = [json.loads(json_format.MessageToJson(bad_request_detail))] - assert want_error_details == exception.error_details + assert want_error_details == exception.details # 404 POST comes from make_response. assert str(exception) == ( "404 POST https://example.com/: 3 INVALID_ARGUMENT:" @@ -271,7 +271,7 @@ def test_error_details_from_v1_rest_response(): ).encode("utf-8") ) exception = exceptions.from_http_response(response) - assert exception.error_details == [] + assert exception.details == [] def test_error_details_from_grpc_response(): @@ -292,7 +292,7 @@ def test_error_details_from_grpc_response(): bad_request_detail = error_details_pb2.BadRequest() status_detail.Unpack(bad_request_detail) - assert exception.error_details == [bad_request_detail] + assert exception.details == [bad_request_detail] def test_error_details_from_grpc_response_unknown_error(): @@ -309,4 +309,4 @@ def test_error_details_from_grpc_response_unknown_error(): with mock.patch("grpc_status.rpc_status.from_call") as m: m.return_value = status exception = exceptions.from_grpc_error(error) - assert exception.error_details == [status_detail] + assert exception.details == [status_detail] From 12edc5aed7af8f025f606324ba5c566ea46eb497 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Mon, 18 Oct 2021 10:23:22 -0700 Subject: [PATCH 09/11] chore: changes dependency req. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 8e59cf41..ddc56004 100644 --- a/setup.py +++ b/setup.py @@ -36,7 +36,7 @@ "setuptools >= 40.3.0", ] extras = { - "grpc": ["grpcio >= 1.33.2, < 2.0dev", "grpcio-status >= 1.0.0, < 2.0dev"], + "grpc": ["grpcio >= 1.33.2, < 2.0dev", "grpcio-status >= 1.33.2, < 2.0dev"], "grpcgcp": "grpcio-gcp >= 0.2.2", "grpcio-gcp": "grpcio-gcp >= 0.2.2", } From a844fed9422f78a5aec95453ae1e2c3a6c2118f5 Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Tue, 19 Oct 2021 10:47:42 -0700 Subject: [PATCH 10/11] Update tests/unit/test_exceptions.py Co-authored-by: Tres Seaver --- tests/unit/test_exceptions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 40c63585..825db244 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -285,6 +285,7 @@ def test_error_details_from_v1_rest_response(): assert exception.details == [] +@pytest.mark.skipif(grpc is None, reason="gRPC not importable") def test_error_details_from_grpc_response(): status = rpc_status.status_pb2.Status() status.code = 3 From a830887491ea98fc99bfe29faa17d9bfe583a1ad Mon Sep 17 00:00:00 2001 From: Aza Tulepbergenov Date: Tue, 19 Oct 2021 10:47:52 -0700 Subject: [PATCH 11/11] Update tests/unit/test_exceptions.py Co-authored-by: Tres Seaver --- tests/unit/test_exceptions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 825db244..f6345fe1 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -307,6 +307,7 @@ def test_error_details_from_grpc_response(): 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(): status_detail = any_pb2.Any()