Merge pull request #423

MPT-25030 Raise a typed MPT error for any error response body

Author: jentyk

docs/architecture.md

@@ -274,3 +274,12 @@ MPTError
     ├── MPTStreamingNotAcceptableError      # 406, format unsupported (also MPTStreamingError)
     └── MPTStreamingOverCapError            # 413, export over cap (also MPTStreamingError)
 ```
+
+An error response becomes an `MPTAPIError` only when its body parses as a JSON object, whose
+members populate `title`, `detail`, `trace_id` and `errors`. httpx detects the JSON encoding
+before parsing, so an object arriving as UTF-16 or UTF-32 keeps its members too — tolerance of
+a non-conforming body, not an endorsement of it: RFC 8259 requires UTF-8 for interchange.
+Any other body — valid JSON that is not an object (a bare string, `null`, an array), a body
+that is not JSON at all, or bytes that do not decode — carries no members to read and becomes
+a plain `MPTHttpError` whose `body` holds the raw diagnostic, with undecodable bytes replaced.
+Either way `status_code` is preserved, so a caller can always branch on the status.

docs/streaming.md

@@ -556,7 +556,10 @@ Split them by what a caller can do about them:
 `MPTStreamingNotEnabledError` and `MPTStreamingItemCountMissingError` are raised before the
 body is read, so no partial data is consumed. The three HTTP-backed types also subclass
 `MPTHttpError`, so existing `except MPTHttpError` handlers keep working and `status_code`
-remains available. Any other HTTP status passes through unchanged.
+remains available. Any other HTTP status passes through unchanged. The status decides the
+type, so the mapping holds whatever the error body turns out to be: an endpoint that answers
+`406` or `413` with a bare JSON string instead of `problem+json` still raises the typed error,
+only without the members `payload` would otherwise carry.
 
 A body the client cannot parse is not a streaming error at all but a `json.JSONDecodeError` —
 a malformed or non-object record line in the line-delimited format, a malformed or

docs/unit_tests.md

@@ -8,7 +8,7 @@ This guide covers the structure, tooling, and conventions for the unit test suit
 ```text
 tests/
 └── unit/
-    ├── conftest.py      # Shared fixtures (http_client, async_http_client, DummyModel)
+    ├── conftest.py      # Shared fixtures and parametrize cases (http_client, DummyModel)
     ├── http/            # Transport, services, and mixins
     ├── models/          # Model, Collection, and Meta behavior
     ├── resources/       # Resource-domain services (accounts, catalog, …)

mpt_api_client/exceptions.py

@@ -274,25 +274,36 @@ def __repr__(self) -> str:
 def transform_http_status_exception(http_status_exception: HTTPStatusError) -> MPTError:
     """Transforms httpx exceptions into MPT exceptions.
 
-    Attempts to extract API related information from HTTPStatusError and
-    raises MPTAPIError or MPTHttpError.
+    An error body that parses as a JSON object carries the API error members, so it
+    becomes an ``MPTAPIError``; httpx detects the JSON encoding, so a UTF-16 or UTF-32
+    object is read as readily as a UTF-8 one and keeps its members. Every other body — a
+    bare JSON string, ``null``, a JSON array, a body that is not JSON, or bytes that do
+    not decode — carries no members to read, so it becomes an ``MPTHttpError`` that still
+    preserves the status code.
 
     Args:
         http_status_exception: Native httpx exception
 
     Returns:
         MPTError
     """
+    response = http_status_exception.response
     try:
+        payload = response.json()
+    except ValueError:
+        # JSONDecodeError for a body that is not JSON, UnicodeDecodeError for one whose
+        # bytes do not decode; both are ValueError and both mean there are no members.
+        payload = None
+    if isinstance(payload, dict):
         return MPTAPIError(
-            status_code=http_status_exception.response.status_code,
-            message=http_status_exception.args[0],
-            payload=http_status_exception.response.json(),
-        )
-    except json.JSONDecodeError:
-        body = http_status_exception.response.content.decode()
-        return MPTHttpError(
-            status_code=http_status_exception.response.status_code,
+            status_code=response.status_code,
             message=http_status_exception.args[0],
-            body=body,
+            payload=payload,
         )
+    return MPTHttpError(
+        status_code=response.status_code,
+        message=http_status_exception.args[0],
+        # The body is a human-readable diagnostic rather than record data, so replacing
+        # undecodable bytes keeps the status reachable instead of raising over them.
+        body=response.content.decode(errors="replace"),
+    )

tests/unit/conftest.py

@@ -86,3 +86,22 @@ def term_variant_inherited_data():
         "contentType": "application/pdf",
         "fileId": "FILE-001",
     }
+
+
+# Error-response body shapes. Only a JSON object carries the API error members, so every
+# other shape must still reach the caller as a typed error that preserves the status code:
+# valid JSON that is not an object gave a raw AttributeError before MPT-25030, and bytes
+# that are not valid UTF-8 escaped the decode guard as a UnicodeDecodeError.
+JSON_OBJECT_ERROR_BODY = b'{"detail": "Invalid filter expression"}'
+NON_OBJECT_ERROR_BODIES = (
+    pytest.param(b'"Invalid filter expression"', id="bare json string"),
+    pytest.param(b"null", id="json null"),
+    pytest.param(b'[{"code": "invalid-rql"}]', id="json array"),
+    pytest.param(b"<html>Bad Gateway</html>", id="body that is not json"),
+    pytest.param(b"", id="empty body"),
+    pytest.param("Grenzwert\u00a0erreicht".encode("iso-8859-1"), id="body that is not utf-8"),
+)
+ERROR_BODY_SHAPES = (
+    pytest.param(JSON_OBJECT_ERROR_BODY, id="json object"),
+    *NON_OBJECT_ERROR_BODIES,
+)

tests/unit/http/mixins/test_streaming_mixin.py

@@ -26,7 +26,7 @@
     deserialize_stream_record,
 )
 from mpt_api_client.models import DeletionStub, Model
-from tests.unit.conftest import API_URL, DummyModel
+from tests.unit.conftest import API_URL, ERROR_BODY_SHAPES, DummyModel
 from tests.unit.http.conftest import (
     JSON_LEGAL_IN_STRING,
     KEEPALIVE_LINE,
@@ -786,6 +786,42 @@ async def test_async_stream_raises_when_over_cap(async_streaming_service):
     assert raised.value.payload == over_cap_problem()
 
 
+# Every status the streaming reader maps to a typed error reads its diagnostic off the
+# error body, so the mapping has to survive an error body of any shape.
+STREAMING_ERROR_STATUSES = (
+    pytest.param(httpx.codes.NOT_IMPLEMENTED, MPTStreamingNotSupportedError, id="501"),
+    pytest.param(httpx.codes.NOT_ACCEPTABLE, MPTStreamingNotAcceptableError, id="406"),
+    pytest.param(httpx.codes.REQUEST_ENTITY_TOO_LARGE, MPTStreamingOverCapError, id="413"),
+)
+
+
+@pytest.mark.parametrize("body", ERROR_BODY_SHAPES)
+@pytest.mark.parametrize(("status_code", "error_class"), STREAMING_ERROR_STATUSES)
+@respx.mock
+def test_stream_maps_status_per_body_shape(streaming_service, status_code, error_class, body):
+    respx.get(STREAM_URL).mock(return_value=httpx.Response(status_code, content=body))
+    iterator = streaming_service.stream()
+
+    with pytest.raises(error_class) as raised:
+        next(iterator)
+
+    assert raised.value.status_code == status_code
+
+
+@pytest.mark.parametrize("body", ERROR_BODY_SHAPES)
+@respx.mock
+async def test_async_stream_over_cap_body_shape(async_streaming_service, body):
+    respx.get(STREAM_URL).mock(
+        return_value=httpx.Response(httpx.codes.REQUEST_ENTITY_TOO_LARGE, content=body)
+    )
+    iterator = async_streaming_service.stream()
+
+    with pytest.raises(MPTStreamingOverCapError) as raised:
+        await anext(iterator)
+
+    assert raised.value.status_code == httpx.codes.REQUEST_ENTITY_TOO_LARGE
+
+
 @respx.mock
 def test_stream_yields_stub_for_deleted_row(
     nullable_fields_service, deletion_stub_record, data_record

tests/unit/test_exceptions.py

@@ -4,6 +4,7 @@
 from httpx import HTTPStatusError, Request, Response, codes
 
 from mpt_api_client import exceptions
+from tests.unit.conftest import NON_OBJECT_ERROR_BODIES
 
 
 @pytest.mark.parametrize(
@@ -145,6 +146,50 @@ def test_transform_http_status_exception_api():
     assert result.payload == payload
 
 
+def error_status_exception(body):
+    response = Response(
+        status_code=codes.BAD_REQUEST,
+        request=Request("GET", "http://test"),
+        content=body,
+    )
+    return HTTPStatusError("Bad Request", request=response.request, response=response)
+
+
+@pytest.mark.parametrize("body", NON_OBJECT_ERROR_BODIES)
+def test_transform_non_object_error_body(body):
+    exc = error_status_exception(body)
+
+    result = exceptions.transform_http_status_exception(exc)
+
+    # An exact type check, because MPTAPIError would satisfy an isinstance check here.
+    assert (type(result), result.status_code) == (exceptions.MPTHttpError, codes.BAD_REQUEST)
+
+
+# httpx detects the JSON encoding, so these keep their members rather than degrading to a
+# bare MPTHttpError. Encoding without an endianness suffix prepends the BOM it detects on.
+@pytest.mark.parametrize(
+    "encoding",
+    [
+        pytest.param("utf-16", id="utf-16"),
+        pytest.param("utf-32", id="utf-32"),
+    ],
+)
+def test_transform_detected_json_encoding(encoding):
+    exc = error_status_exception('{"detail": "Invalid filter expression"}'.encode(encoding))
+
+    result = exceptions.transform_http_status_exception(exc)
+
+    assert (type(result), result.detail) == (exceptions.MPTAPIError, "Invalid filter expression")
+
+
+def test_transform_undecodable_error_body():
+    exc = error_status_exception("Grenzwert\u00a0erreicht".encode("iso-8859-1"))
+
+    result = exceptions.transform_http_status_exception(exc)
+
+    assert result.body == "Grenzwert\ufffderreicht"
+
+
 def test_transform_http_status_exception():
     response = Response(
         status_code=500,