Skip to content

Commit dcf1d1e

Browse files
jentykclaude
andcommitted
fix(http): raise a typed MPT error for any error response body
transform_http_status_exception handed response.json() straight to MPTAPIError, whose __init__ calls payload.get(...). An error body that is valid JSON but not an object - a bare string, null, or an array - made that call raise AttributeError, so the caller lost the HTTP status entirely and could not branch on it. The payload: dict[str, str] annotation confirms a non-dict was never intended. Returning BadRequest("Invalid filter expression") is idiomatic ASP.NET Core and serializes to a bare JSON string, so a hand-written collection action answering 400 for invalid RQL reaches this shape without any misconfiguration. The streaming error mapping made it worse: raise_streaming_error picks MPTStreamingNotSupportedError, MPTStreamingNotAcceptableError and MPTStreamingOverCapError off the status of an MPTHttpError, so a bare string body on 406 or 413 lost the typed streaming error and the over-cap detail with it. The same except clause also missed UnicodeDecodeError, which response.json() raises for a body whose bytes do not decode - a ValueError but not a JSONDecodeError - and the fallback response.content.decode() would then have failed the same way. Decode the body first, catch ValueError to cover both decode failures, and build an MPTAPIError only when the payload really is a dict. Everything else becomes an MPTHttpError carrying the raw body with undecodable bytes replaced, which is right for a human-readable diagnostic rather than record data. The status code now survives every body shape. Tests parametrize the error-body shapes - object, bare string, null, array, non-JSON, empty, and an ISO-8859-1 body - over the paged path and over 501, 406 and 413 on the sync and async streaming paths; the shared cases live in tests/unit/conftest.py. A further case pins the UTF-16 and UTF-32 objects that httpx's encoding detection parses, so the documented rule matches what the code does. Document the selection rule in docs/architecture.md and note in docs/streaming.md that the status, not the body, decides the streaming error type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 6ca78f7 commit dcf1d1e

7 files changed

Lines changed: 137 additions & 14 deletions

File tree

‎docs/architecture.md‎

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

‎docs/streaming.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,10 @@ Split them by what a caller can do about them:
554554
`MPTStreamingNotEnabledError` and `MPTStreamingItemCountMissingError` are raised before the
555555
body is read, so no partial data is consumed. The three HTTP-backed types also subclass
556556
`MPTHttpError`, so existing `except MPTHttpError` handlers keep working and `status_code`
557-
remains available. Any other HTTP status passes through unchanged.
557+
remains available. Any other HTTP status passes through unchanged. The status decides the
558+
type, so the mapping holds whatever the error body turns out to be: an endpoint that answers
559+
`406` or `413` with a bare JSON string instead of `problem+json` still raises the typed error,
560+
only without the members `payload` would otherwise carry.
558561

559562
A body the client cannot parse is not a streaming error at all but a `json.JSONDecodeError` —
560563
a malformed or non-object record line in the line-delimited format, a malformed or

‎docs/unit_tests.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ This guide covers the structure, tooling, and conventions for the unit test suit
88
```text
99
tests/
1010
└── unit/
11-
├── conftest.py # Shared fixtures (http_client, async_http_client, DummyModel)
11+
├── conftest.py # Shared fixtures and parametrize cases (http_client, DummyModel)
1212
├── http/ # Transport, services, and mixins
1313
├── models/ # Model, Collection, and Meta behavior
1414
├── resources/ # Resource-domain services (accounts, catalog, …)

‎mpt_api_client/exceptions.py‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -274,25 +274,36 @@ def __repr__(self) -> str:
274274
def transform_http_status_exception(http_status_exception: HTTPStatusError) -> MPTError:
275275
"""Transforms httpx exceptions into MPT exceptions.
276276
277-
Attempts to extract API related information from HTTPStatusError and
278-
raises MPTAPIError or MPTHttpError.
277+
An error body that parses as a JSON object carries the API error members, so it
278+
becomes an ``MPTAPIError``; httpx detects the JSON encoding, so a UTF-16 or UTF-32
279+
object is read as readily as a UTF-8 one and keeps its members. Every other body — a
280+
bare JSON string, ``null``, a JSON array, a body that is not JSON, or bytes that do
281+
not decode — carries no members to read, so it becomes an ``MPTHttpError`` that still
282+
preserves the status code.
279283
280284
Args:
281285
http_status_exception: Native httpx exception
282286
283287
Returns:
284288
MPTError
285289
"""
290+
response = http_status_exception.response
286291
try:
292+
payload = response.json()
293+
except ValueError:
294+
# JSONDecodeError for a body that is not JSON, UnicodeDecodeError for one whose
295+
# bytes do not decode; both are ValueError and both mean there are no members.
296+
payload = None
297+
if isinstance(payload, dict):
287298
return MPTAPIError(
288-
status_code=http_status_exception.response.status_code,
289-
message=http_status_exception.args[0],
290-
payload=http_status_exception.response.json(),
291-
)
292-
except json.JSONDecodeError:
293-
body = http_status_exception.response.content.decode()
294-
return MPTHttpError(
295-
status_code=http_status_exception.response.status_code,
299+
status_code=response.status_code,
296300
message=http_status_exception.args[0],
297-
body=body,
301+
payload=payload,
298302
)
303+
return MPTHttpError(
304+
status_code=response.status_code,
305+
message=http_status_exception.args[0],
306+
# The body is a human-readable diagnostic rather than record data, so replacing
307+
# undecodable bytes keeps the status reachable instead of raising over them.
308+
body=response.content.decode(errors="replace"),
309+
)

‎tests/unit/conftest.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,22 @@ def term_variant_inherited_data():
8686
"contentType": "application/pdf",
8787
"fileId": "FILE-001",
8888
}
89+
90+
91+
# Error-response body shapes. Only a JSON object carries the API error members, so every
92+
# other shape must still reach the caller as a typed error that preserves the status code:
93+
# valid JSON that is not an object gave a raw AttributeError before MPT-25030, and bytes
94+
# that are not valid UTF-8 escaped the decode guard as a UnicodeDecodeError.
95+
JSON_OBJECT_ERROR_BODY = b'{"detail": "Invalid filter expression"}'
96+
NON_OBJECT_ERROR_BODIES = (
97+
pytest.param(b'"Invalid filter expression"', id="bare json string"),
98+
pytest.param(b"null", id="json null"),
99+
pytest.param(b'[{"code": "invalid-rql"}]', id="json array"),
100+
pytest.param(b"<html>Bad Gateway</html>", id="body that is not json"),
101+
pytest.param(b"", id="empty body"),
102+
pytest.param("Grenzwert\u00a0erreicht".encode("iso-8859-1"), id="body that is not utf-8"),
103+
)
104+
ERROR_BODY_SHAPES = (
105+
pytest.param(JSON_OBJECT_ERROR_BODY, id="json object"),
106+
*NON_OBJECT_ERROR_BODIES,
107+
)

‎tests/unit/http/mixins/test_streaming_mixin.py‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
deserialize_stream_record,
2727
)
2828
from mpt_api_client.models import DeletionStub, Model
29-
from tests.unit.conftest import API_URL, DummyModel
29+
from tests.unit.conftest import API_URL, ERROR_BODY_SHAPES, DummyModel
3030
from tests.unit.http.conftest import (
3131
JSON_LEGAL_IN_STRING,
3232
KEEPALIVE_LINE,
@@ -786,6 +786,42 @@ async def test_async_stream_raises_when_over_cap(async_streaming_service):
786786
assert raised.value.payload == over_cap_problem()
787787

788788

789+
# Every status the streaming reader maps to a typed error reads its diagnostic off the
790+
# error body, so the mapping has to survive an error body of any shape.
791+
STREAMING_ERROR_STATUSES = (
792+
pytest.param(httpx.codes.NOT_IMPLEMENTED, MPTStreamingNotSupportedError, id="501"),
793+
pytest.param(httpx.codes.NOT_ACCEPTABLE, MPTStreamingNotAcceptableError, id="406"),
794+
pytest.param(httpx.codes.REQUEST_ENTITY_TOO_LARGE, MPTStreamingOverCapError, id="413"),
795+
)
796+
797+
798+
@pytest.mark.parametrize("body", ERROR_BODY_SHAPES)
799+
@pytest.mark.parametrize(("status_code", "error_class"), STREAMING_ERROR_STATUSES)
800+
@respx.mock
801+
def test_stream_maps_status_per_body_shape(streaming_service, status_code, error_class, body):
802+
respx.get(STREAM_URL).mock(return_value=httpx.Response(status_code, content=body))
803+
iterator = streaming_service.stream()
804+
805+
with pytest.raises(error_class) as raised:
806+
next(iterator)
807+
808+
assert raised.value.status_code == status_code
809+
810+
811+
@pytest.mark.parametrize("body", ERROR_BODY_SHAPES)
812+
@respx.mock
813+
async def test_async_stream_over_cap_body_shape(async_streaming_service, body):
814+
respx.get(STREAM_URL).mock(
815+
return_value=httpx.Response(httpx.codes.REQUEST_ENTITY_TOO_LARGE, content=body)
816+
)
817+
iterator = async_streaming_service.stream()
818+
819+
with pytest.raises(MPTStreamingOverCapError) as raised:
820+
await anext(iterator)
821+
822+
assert raised.value.status_code == httpx.codes.REQUEST_ENTITY_TOO_LARGE
823+
824+
789825
@respx.mock
790826
def test_stream_yields_stub_for_deleted_row(
791827
nullable_fields_service, deletion_stub_record, data_record

‎tests/unit/test_exceptions.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from httpx import HTTPStatusError, Request, Response, codes
55

66
from mpt_api_client import exceptions
7+
from tests.unit.conftest import NON_OBJECT_ERROR_BODIES
78

89

910
@pytest.mark.parametrize(
@@ -145,6 +146,50 @@ def test_transform_http_status_exception_api():
145146
assert result.payload == payload
146147

147148

149+
def error_status_exception(body):
150+
response = Response(
151+
status_code=codes.BAD_REQUEST,
152+
request=Request("GET", "http://test"),
153+
content=body,
154+
)
155+
return HTTPStatusError("Bad Request", request=response.request, response=response)
156+
157+
158+
@pytest.mark.parametrize("body", NON_OBJECT_ERROR_BODIES)
159+
def test_transform_non_object_error_body(body):
160+
exc = error_status_exception(body)
161+
162+
result = exceptions.transform_http_status_exception(exc)
163+
164+
# An exact type check, because MPTAPIError would satisfy an isinstance check here.
165+
assert (type(result), result.status_code) == (exceptions.MPTHttpError, codes.BAD_REQUEST)
166+
167+
168+
# httpx detects the JSON encoding, so these keep their members rather than degrading to a
169+
# bare MPTHttpError. Encoding without an endianness suffix prepends the BOM it detects on.
170+
@pytest.mark.parametrize(
171+
"encoding",
172+
[
173+
pytest.param("utf-16", id="utf-16"),
174+
pytest.param("utf-32", id="utf-32"),
175+
],
176+
)
177+
def test_transform_detected_json_encoding(encoding):
178+
exc = error_status_exception('{"detail": "Invalid filter expression"}'.encode(encoding))
179+
180+
result = exceptions.transform_http_status_exception(exc)
181+
182+
assert (type(result), result.detail) == (exceptions.MPTAPIError, "Invalid filter expression")
183+
184+
185+
def test_transform_undecodable_error_body():
186+
exc = error_status_exception("Grenzwert\u00a0erreicht".encode("iso-8859-1"))
187+
188+
result = exceptions.transform_http_status_exception(exc)
189+
190+
assert result.body == "Grenzwert\ufffderreicht"
191+
192+
148193
def test_transform_http_status_exception():
149194
response = Response(
150195
status_code=500,

0 commit comments

Comments
 (0)