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

Refactor error parsing to better support JSON:API #725

Merged
merged 10 commits into from
May 31, 2023
Prev Previous commit
Next Next commit
Implement and document ErrorSubdocument for errors
GlobusAPIError.errors is now a list of ErrorSubdocument objects. These
are documented with an ultra-simple usage example as part of the core
exception documentation.

ErrorSubdocuments are defined to match well to the JSON:API notion of
an error object, but reduced to the fields which are appropriate to
our usage: code and message.

Because the `errors` objects now have some capabilities, some of the
parsing around (e.g., `messages` extraction) can be simplified.
`ErrorSubdocument` objects contain the list of fields they use for
`message` lookup, and therefore need to be customized to use
`GlobusAPIGlobusAPIError.MESSAGE_FIELDS` in a few locations.

Test changes are minimal in order to align types -- no test
behaviors are adjusted here other than those to fix the type of
`errors`.
sirosen committed May 24, 2023
commit b2ac513bef23a1008b424d69eb0a1c3ab071b830
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@

* A new attribute is added to API error objects, ``errors``. This is a list
of subdocuments parsed from the error data, especially relevant for
JSON:API errors and similar formats.
JSON:API errors and similar formats. See the
:ref:`ErrorSubdocument documentation <error_subdocuments>` for details.

* A new attribute is now present on API error objects, ``messages``. This is
a list of messages parsed from the error data, for errors with multiple
29 changes: 28 additions & 1 deletion docs/core/exceptions.rst
Original file line number Diff line number Diff line change
@@ -65,7 +65,6 @@ Malformed calls to Globus SDK methods typically raise ``GlobusSDKUsageError``,
but, in rare cases, may raise standard python exceptions (``ValueError``,
``OSError``, etc.)


Error Classes
-------------

@@ -97,6 +96,34 @@ Error Classes
:members:
:show-inheritance:

.. _error_subdocuments:

ErrorSubdocuments
-----------------

Errors returned from APIs may define a series of subdocuments, each containing
an error object. This is used if there were multiple errors encountered and the
API wants to send them back all at once.

All instances of ``GlobusAPIError`` define an attribute, ``errors``, which is
an array of ``ErrorSubdocument``\s. It may be empty if there were no subdocuments.
sirosen marked this conversation as resolved.
Show resolved Hide resolved

Error handling code can inspect these sub-errors like so:

.. code-block:: python

try:
some_complex_globus_operation()
except GlobusAPIError as e:
if e.errors:
print("sub-errors encountered")
print("(code, message)")
for suberror in e.errors:
print(f"({suberror.code}, {suberror.message}")

.. autoclass:: globus_sdk.exc.ErrorSubdocument
:members:

.. _error_info:

ErrorInfo
3 changes: 3 additions & 0 deletions src/globus_sdk/__init__.py
Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ def _force_eager_imports() -> None:
},
"exc": {
"GlobusAPIError",
"ErrorSubdocument",
"GlobusConnectionError",
"GlobusConnectionTimeoutError",
"GlobusError",
@@ -134,6 +135,7 @@ def _force_eager_imports() -> None:
from .authorizers import RefreshTokenAuthorizer
from .client import BaseClient
from .exc import GlobusAPIError
from .exc import ErrorSubdocument
from .exc import GlobusConnectionError
from .exc import GlobusConnectionTimeoutError
from .exc import GlobusError
@@ -257,6 +259,7 @@ def __getattr__(name: str) -> t.Any:
"CollectionPolicies",
"ConfidentialAppAuthClient",
"DeleteData",
"ErrorSubdocument",
"FlowsAPIError",
"FlowsClient",
"GCSAPIError",
1 change: 1 addition & 0 deletions src/globus_sdk/_generate_init.py
Original file line number Diff line number Diff line change
@@ -81,6 +81,7 @@ def __getattr__(name: str) -> t.Any:
"exc",
(
"GlobusAPIError",
"ErrorSubdocument",
"GlobusConnectionError",
"GlobusConnectionTimeoutError",
"GlobusError",
3 changes: 2 additions & 1 deletion src/globus_sdk/exc/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .api import GlobusAPIError
from .api import ErrorSubdocument, GlobusAPIError
from .base import GlobusError, GlobusSDKUsageError
from .convert import (
GlobusConnectionError,
@@ -19,6 +19,7 @@
"GlobusError",
"GlobusSDKUsageError",
"GlobusAPIError",
"ErrorSubdocument",
"NetworkError",
"GlobusTimeoutError",
"GlobusConnectionTimeoutError",
168 changes: 116 additions & 52 deletions src/globus_sdk/exc/api.py
Original file line number Diff line number Diff line change
@@ -31,9 +31,8 @@ class GlobusAPIError(GlobusError):
:ivar list[str] messages: A list of error messages, extracted from the response
data. If the data cannot be parsed or does not contain any clear message fields,
this list may be empty.
:ivar list[dict] errors: A list of sub-error documents, as would be presented by
JSON:API APIs and similar interfaces. If no such parse succeeds, the list will
be empty.
:ivar list[GlobusSubError] errors: A list of sub-error documents, as would be
presented by JSON:API APIs and similar interfaces.
"""

MESSAGE_FIELDS = ["message", "detail", "title"]
@@ -47,7 +46,7 @@ def __init__(self, r: requests.Response, *args: t.Any, **kwargs: t.Any):
self.code: str | None = "Error"
sirosen marked this conversation as resolved.
Show resolved Hide resolved
self.request_id: str | None = None
self.messages: list[str] = []
self.errors: list[dict[str, t.Any]] = []
self.errors: list[ErrorSubdocument] = []

self._info: ErrorInfoContainer | None = None
self._underlying_response = r
@@ -261,6 +260,15 @@ def _parse_response(self) -> bool:
return False
return self._final_parse_callback()

def _final_parse_callback(self) -> bool:
"""
A final internal callback for extra customizations after
fully successful parsing.

By default, does nothing.
"""
return True

def _detect_error_format(self) -> _ErrorFormat:
# if the JSON:API mimetype was used, inspect the data to make sure it is
# well-formed
@@ -292,95 +300,97 @@ def _detect_error_format(self) -> _ErrorFormat:
return _ErrorFormat.type_zero

def _parse_jsonapi_error_format(self) -> bool:
# parsing a JSON:API error is only called after the field type for 'errors'
# has been checked
# however, the nested/underlying fields will not have been checked yet
self.errors = self._dict_data["errors"]
"""
Parsing a JSON:API Error

This is only called after the field type for 'errors' has been checked.
However, the nested/underlying fields will not have been checked yet.
"""
self.errors = [ErrorSubdocument(e) for e in self._dict_data["errors"]]
self.code = self._extract_code_from_error_array(self.errors)
self.messages = self._extract_messages_from_error_array(
self.errors, ("detail", "title")
)
self.messages = self._extract_messages_from_error_array(self.errors)
return True

def _parse_type_zero_error_format(self) -> bool:
# parsing a Type Zero error is only called after Type Zero has been detected
# therefore, we already have assurances about the values in 'code' and 'message'
# note that 'request_id' could be absent but *must* be a string if present
"""
Parsing a Type Zero Error

This is only called after Type Zero has been detected. Therefore, we already
have assurances about the values in 'code' and 'message'.
Note that 'request_id' could be absent but *must* be a string if present.
"""
self.code = self._dict_data["code"]
self.messages = [self._dict_data["message"]]
self.request_id = self._dict_data.get("request_id")
if isinstance(self._dict_data.get("errors"), list) and all(
isinstance(subdoc, dict) for subdoc in self._dict_data["errors"]
):
self.errors = self._dict_data["errors"]
raw_errors = self._dict_data["errors"]
else:
self.errors = [self._dict_data]
raw_errors = [self._dict_data]
self.errors = [
ErrorSubdocument(e, message_fields=self.MESSAGE_FIELDS) for e in raw_errors
]
return True

def _parse_undefined_error_format(self) -> bool:
# Undefined parsing: best effort support for unknown data shapes
# this is also a great place for custom parsing to hook in for different APIs
# if we know that there's an unusual format in use
"""
Undefined Parsing: best effort support for unknown data shapes

This is also a great place for custom parsing to hook in for different APIs
if we know that there's an unusual format in use
"""

# attempt to pull out errors if possible and valid
if isinstance(self._dict_data.get("errors"), list) and all(
isinstance(subdoc, dict) for subdoc in self._dict_data["errors"]
):
self.errors = self._dict_data["errors"]
raw_errors = self._dict_data["errors"]
# if no 'errors' were found, or 'errors' is invalid, then
# 'errors' should be set to contain the root document
else:
self.errors = [self._dict_data]
raw_errors = [self._dict_data]
self.errors = [
ErrorSubdocument(e, message_fields=self.MESSAGE_FIELDS) for e in raw_errors
]

# use 'code' if present and correct type
if isinstance(self._dict_data.get("code"), str):
self.code = t.cast(str, self._dict_data["code"])
# otherwise, check if all 'errors' which contain a 'code' have the same one
# if so, that will be safe to use instead
# otherwise, pull 'code' from the sub-errors array
elif self.errors:
code = self._extract_code_from_error_array(self.errors)
if code is not None:
self.code = code
# in undefined parse cases, the code will be left as `"Error"` for
# a higher degree of backwards compatibility
maybe_code = self._extract_code_from_error_array(self.errors)
if maybe_code is not None:
self.code = maybe_code

# either there is an array of subdocument errors or there is not,
# in which case we load only from the root doc
self.messages = self._extract_messages_from_error_array(
self.errors or [self._dict_data]
self.errors
or [ErrorSubdocument(self._dict_data, message_fields=self.MESSAGE_FIELDS)]
)

return True

def _extract_messages_from_error_array(
self,
errors: list[dict[str, t.Any]],
message_fields: t.Sequence[str] | None = None,
self, errors: list[ErrorSubdocument]
) -> list[str]:
"""
Extract 'messages' from an array of errors (JSON:API or otherwise)

This is done by checking each error document for the desired message fields and
popping out the first such message for each document.
"""
if message_fields is None:
message_fields = self.MESSAGE_FIELDS

ret: list[str] = []
for doc in errors:
for f in message_fields:
if isinstance(doc.get(f), str):
log.debug("Loaded message from '%s' field", f)
ret.append(doc[f])
# NOTE! this break ensures we take only one field per error
# document (or subdocument) as the 'message'
# so that a doc like
# {"message": "foo", "detail": "bar"}
# has a single message, "foo"
break

if doc.message is not None:
ret.append(doc.message)
return ret

def _extract_code_from_error_array(
self, errors: list[dict[str, t.Any]]
self, errors: list[ErrorSubdocument]
) -> str | None:
"""
Extract a 'code' field from an array of errors (JSON:API or otherwise)
@@ -389,17 +399,71 @@ def _extract_code_from_error_array(
"""
codes: set[str] = set()
for error in errors:
if isinstance(error.get("code"), str):
codes.add(error["code"])
if error.code is not None:
codes.add(error.code)
if len(codes) == 1:
return codes.pop()
return None

def _final_parse_callback(self) -> bool:

class ErrorSubdocument:
"""
Error subdocuments as returned by Globus APIs.

:ivar dict raw: The unparsed error subdocument
"""

# the default set of fields to use for message extraction, in order
# selected to match the fields defined by JSON:API by default
DEFAULT_MESSAGE_FIELDS: tuple[str, ...] = ("detail", "title")

def __init__(
self, data: dict[str, t.Any], *, message_fields: t.Sequence[str] | None = None
) -> None:
self.raw = data
self._message_fields: tuple[str, ...]

if message_fields is not None:
self._message_fields = tuple(message_fields)
else:
self._message_fields = self.DEFAULT_MESSAGE_FIELDS

@property
def message(self) -> str | None:
"""
A final internal callback for extra customizations after
fully successful parsing.
The 'message' string of this subdocument, derived from its data based on the
parsing context.

By default, does nothing.
May be `None` if no message is defined.
"""
return True
return _extract_message_from_dict(self.raw, self._message_fields)

@property
def code(self) -> str | None:
"""
The 'code' string of this subdocument, derived from its data based on the
parsing context.

May be `None` if no code is defined.
"""
if isinstance(self.raw.get("code"), str):
return t.cast(str, self.raw["code"])
return None

def get(self, key: str, default: t.Any = None) -> t.Any | None:
"""
A dict-like getter for the raw data.
"""
return self.raw.get(key, default)


def _extract_message_from_dict(
data: dict[str, t.Any], message_fields: tuple[str, ...]
) -> str | None:
"""
Extract a single message string from a dict if one is present.
"""
for f in message_fields:
if isinstance(data.get(f), str):
return t.cast(str, data[f])
return None
4 changes: 2 additions & 2 deletions src/globus_sdk/services/flows/errors.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from globus_sdk.exc import GlobusAPIError
from globus_sdk.exc import ErrorSubdocument, GlobusAPIError


class FlowsAPIError(GlobusAPIError):
@@ -23,7 +23,7 @@ def _parse_undefined_error_format(self) -> bool:
if not isinstance(self._dict_data.get("error"), dict):
return super()._parse_undefined_error_format()

self.errors = [self._dict_data["error"]]
self.errors = [ErrorSubdocument(self._dict_data["error"])]
self.code = self._extract_code_from_error_array(self.errors)

details = self._dict_data["error"].get("detail")
Loading