Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add error_details property to GoogleAPICallError based on google.rpc.status.details. #286

Merged
merged 15 commits into from Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 63 additions & 4 deletions google/api_core/exceptions.py
Expand Up @@ -25,10 +25,14 @@
from typing import Dict
from typing import Union

from google.rpc import error_details_pb2
tseaver marked this conversation as resolved.
Show resolved Hide resolved

try:
import grpc
from grpc_status import rpc_status
tseaver marked this conversation as resolved.
Show resolved Hide resolved
except ImportError: # pragma: NO COVER
grpc = None
rpc_status = None

# Lookup tables for mapping exceptions from HTTP and gRPC transports.
# Populated by _GoogleAPICallErrorMeta
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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.
Show resolved Hide resolved
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):
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Show resolved Hide resolved
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.
Show resolved Hide resolved
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`.

Expand All @@ -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)
4 changes: 2 additions & 2 deletions setup.py
Expand Up @@ -29,14 +29,14 @@
# '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",
"setuptools >= 40.3.0",
]
extras = {
"grpc": "grpcio >= 1.33.2, < 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",
}
Expand Down
3 changes: 2 additions & 1 deletion testing/constraints-3.6.txt
Expand Up @@ -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
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
grpcio-status==1.33.2
grpcio-status==1.0.0

(to match the lower bound in setup.py)

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the grpcio-status version supposed to be identical to the one for grpcio?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 setup.py to match the bound for grpcio (ie 1.33.2).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to 1.33.2

3 changes: 3 additions & 0 deletions tests/asyncio/test_grpc_helpers_async.py
Expand Up @@ -42,6 +42,9 @@ def code(self):
def details(self):
return None

def trailing_metadata(self):
return None


@pytest.mark.asyncio
async def test_wrap_unary_errors():
Expand Down
98 changes: 93 additions & 5 deletions tests/unit/test_exceptions.py
Expand Up @@ -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():
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Show resolved Hide resolved
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.
Show resolved Hide resolved
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]
3 changes: 3 additions & 0 deletions tests/unit/test_grpc_helpers.py
Expand Up @@ -56,6 +56,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)
Expand Down