Refactor error parsing to better support JSON:API

changelog.d/20230501_212027_sirosen_fix_error_shapes_with_messages.rst

@@ -0,0 +1,52 @@
+* Error parsing in the SDK has been enhanced to better support JSON:API and
+  related error formats with multiple sub-errors. Several attributes are
+  added or changed. For most SDK users, the changes will be completely
+  transparent or a minor improvement. (:pr:`NUMBER`)
+
+  * Error parsing now attempts to detect the format of the error data and will
+    parse JSON:API data differently from non-JSON:API data. Furthermore,
+    parsing is stricter about the expectations about fields and their types.
+    JSON:API parsing now has its own distinct parsing path, followed only when
+    the JSON:API mimetype is present.
+
+  * 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. 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
+    messages. When there is only one message, ``messages`` will only contain
+    one item.
+
+  * The ``message`` field is now an alias for a joined string of
+    ``messages``. Assigning a string to ``message`` is supported for error
+    subclasses, but is deprecated.
+
+  * ``message`` will now be ``None`` when no messages can be parsed from the error data.
+    Previously, the default for ``message`` would be an alias for ``text``.
+
+  * All error types now support ``request_id`` as an attribute, but it will
+    default to ``None`` for errors which do not include a ``request_id``.
+
+  * An additional field is checked by default for error message data,
+    ``title``. This is useful when errors contain ``title`` but no
+    ``detail`` field. The extraction of messages from errors has been made
+    stricter, especially in the JSON:API case.
+
+  * The ``code`` field of errors will no longer attempt to parse only the first
+    ``code`` from multiple sub-errors. Instead, ``code`` will first parse a
+    top-level ``code`` field, and then fallback to checking if *all* sub-errors
+    have the same ``code`` value. The result is that certain errors which would
+    populate a non-default ``code`` value no longer will, but the ``code`` will
+    also no longer be misleading when multiple errors with different codes are
+    present in an error object.
+
+  * The ``code`` field of an error may now be ``None``. This is specifically
+    possible when the error format is detected to be known as JSON:API and
+    there is no ``code`` present in any responses.
+
+* Behavior has changed slightly specifically for ``TimerAPIError``. When parsing
+  fails, the ``code`` will be ``Error`` and the ``messages`` will be empty. The
+  ``detail`` field will be treated as the ``errors`` array for these errors
+  when it is present and contains an array of objects.

docs/core/exceptions.rst

@@ -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.
+
+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

src/globus_sdk/__init__.py

@@ -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",

src/globus_sdk/_generate_init.py

@@ -81,6 +81,7 @@ def __getattr__(name: str) -> t.Any:
         "exc",
         (
             "GlobusAPIError",
+            "ErrorSubdocument",
             "GlobusConnectionError",
             "GlobusConnectionTimeoutError",
             "GlobusError",

src/globus_sdk/_testing/data/flows/create_flow.py

@@ -29,4 +29,38 @@
         json=TWO_HOP_TRANSFER_FLOW_DOC,
         match=[matchers.json_params_matcher(params=_two_hop_transfer_create_request)],
     ),
+    bad_admin_principal_error=RegisteredResponse(
+        service="flows",
+        path="/flows",
+        method="POST",
+        status=422,
+        json={
+            "error": {
+                "code": "UNPROCESSABLE_ENTITY",
+                "detail": [
+                    {
+                        "loc": ["flow_administrators", 0],
+                        "msg": (
+                            "Unrecognized principal string: "
+                            '"ae341a98-d274-11e5-b888-dbae3a8ba545". '
+                            'Allowed principal types in role "FlowAdministrator": '
+                            "[<AuthGroupUrn>, <AuthIdentityUrn>]"
+                        ),
+                        "type": "value_error",
+                    },
+                    {
+                        "loc": ["flow_administrators", 1],
+                        "msg": (
+                            "Unrecognized principal string: "
+                            '"4fab4345-6d20-43a0-9a25-16b2e3bbe765". '
+                            'Allowed principal types in role "FlowAdministrator": '
+                            "[<AuthGroupUrn>, <AuthIdentityUrn>]"
+                        ),
+                        "type": "value_error",
+                    },
+                ],
+            },
+            "debug_id": "cf71b1d1-ab7e-48b1-8c54-764201d28ded",
+        },
+    ),
 )

src/globus_sdk/_testing/data/flows/run_flow.py

@@ -20,4 +20,18 @@
         json=TWO_HOP_TRANSFER_RUN,
         match=[matchers.json_params_matcher(params=_request_params)],
     ),
+    missing_scope_error=RegisteredResponse(
+        service="flows",
+        method="POST",
+        path=f"/flows/{TWO_HOP_TRANSFER_FLOW_ID}/run",
+        status=403,
+        json={
+            "error": {
+                "code": "MISSING_SCOPE",
+                "detail": (
+                    "This action requires the following scope: frobulate[demuddle]"
+                ),
+            }
+        },
+    ),
 )

src/globus_sdk/_testing/data/timer/create_job.py

@@ -9,7 +9,33 @@
         path="/jobs/",
         method="POST",
         json=JOB_JSON,
-        metadata={"job_id": JOB_ID},
         status=201,
     ),
+    validation_error=RegisteredResponse(
+        service="timer",
+        path="/jobs/",
+        method="POST",
+        json={
+            "detail": [
+                {
+                    "loc": ["body", "start"],
+                    "msg": "field required",
+                    "type": "value_error.missing",
+                },
+                {
+                    "loc": ["body", "callback_url"],
+                    "msg": "field required",
+                    "type": "value_error.missing",
+                },
+            ]
+        },
+        metadata={
+            "job_id": JOB_ID,
+            "expect_messages": [
+                "field required: body.start",
+                "field required: body.callback_url",
+            ],
+        },
+        status=422,
+    ),
 )

src/globus_sdk/_testing/data/timer/get_job.py

@@ -118,4 +118,17 @@
         method="GET",
         json=JOB_JSON,
     ),
+    simple_500_error=RegisteredResponse(
+        service="timer",
+        path=f"/jobs/{JOB_ID}",
+        method="GET",
+        status=500,
+        json={
+            "error": {
+                "code": "ERROR",
+                "detail": "Request failed terribly",
+                "status": 500,
+            }
+        },
+    ),
 )

src/globus_sdk/_testing/models.py

@@ -57,9 +57,6 @@ def __init__(
         self.method = method.upper()
         self.json = json
         self.body = body
-
-        if headers is None:
-            headers = {"Content-Type": "application/json"}
         self.headers = headers
 
         self._metadata = metadata

src/globus_sdk/exc/__init__.py

@@ -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",