fix(python-runtime): stop double-decoding compressed responses; parse from the structural body - #178
Open
hardbyte wants to merge 4 commits into
Open
fix(python-runtime): stop double-decoding compressed responses; parse from the structural body#178hardbyte wants to merge 4 commits into
hardbyte wants to merge 4 commits into
Conversation
The structural Response carries an already-decompressed body (httpx decodes on .content), but the headers may still advertise Content-Encoding: gzip. Rebuilding an httpx.Response from that pair made httpx decompress the decoded bytes a second time, raising DecodingError -> NetworkError before deserialization. Strip Content-Encoding (and the now-stale Content-Length) when lifting the structural body into the synthetic httpx.Response used for parsing. Metadata headers still reflect the original wire headers. Fixes #177
…rop synthetic httpx.Response round-trip Response parsing previously flattened the wire httpx.Response into the structural DTO, then rebuilt a second httpx.Response purely so error handling and JSON parsing could call .json() on it. That reconstruction re-ran httpx's content pipeline on already-decoded bytes — the root cause of #177's double-decompression — and left parsing exposed to any header/body inconsistency (e.g. middleware-rewritten bodies with stale wire headers). Parsing now operates directly on (status, body): module-level _raise_for_error_status / _parse_json_body / _validate_body replace the duplicated per-class helper methods in both sync and async clients, and the SSE error path feeds the read body through the same function. A synthetic httpx.Response is now built only as the raw_response metadata fallback for custom transports that don't surface a wire object, with compression headers stripped so it stays readable. Tests cover compressed success and error bodies (sync/async), typed errors from compressed responses, middleware-rewritten bodies under stale wire headers, non-JSON bodies on both paths, and the sanitized raw_response fallback.
…tent-Length in synthetic raw_response - transport.Response docstring now states body must be the decoded entity bytes (the runtime never re-applies Content-Encoding to it) — an observable contract for custom Client implementers. - _synthesize_raw_response drops Content-Length unconditionally, not only alongside Content-Encoding: middleware-rewritten bodies make it stale too. httpx re-derives an accurate value from the actual body. - Widen _synthesize_raw_response body hint to match its siblings; use contextlib.suppress for the intentional error-body swallows.
PR #151 replaced the ReflectapiOption ADT with ReflectapiPartialModel (three-state fields tracked via Pydantic's model_fields_set), but three demo test modules still imported the removed class, failing collection ever since: - tests/runtime_integration/test_reflectapi_option.py tested only the removed ADT's own methods (unwrap/map/eq/repr); the replacement semantics are covered by the runtime's test_partial.py and the demo's test_models.py — deleted. - test_advanced_edge_cases.py: ported the advanced cases to the partial model API, updating serialization expectations (unset fields are now omitted from the wire, not emitted as null) and adding explicit null-vs-absent round-trip coverage. - test_client_server_e2e.py: plain value instead of the ADT wrapper. Also updates the runtime README bullet that still described ReflectapiOption. Demo suite now collects fully: 173 passed, 16 skipped (live-server integration).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
📖 Documentation Preview: https://reflectapi-docs-preview-pr-178.partly.workers.dev Updated automatically from commit b7ef26c |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #177.
Problem
_make_requestrebuilt a synthetichttpx.Responsefrom the structural transport response. The structural body is already decompressed (it comes fromhttpx.Response.content), but the headers still advertisedContent-Encoding: gzip, so httpx tried to decompress the decoded JSON a second time:Fix
Commit-by-commit:
Content-Encoding/Content-Lengthwhen rebuilding the synthetic response, with sync/async/custom-transport regression tests.httpx.Responseat all. Error handling, JSON parsing, and Pydantic validation now operate directly on the structural(status, body)via module-level_raise_for_error_status/_parse_json_body/_validate_body, shared by the sync, async, and SSE paths (this also deletes the previously copy-pasted helper methods betweenClientBaseandAsyncClientBase). A synthetichttpx.Responsesurvives in exactly one place: theTransportMetadata.raw_responsefallback for custom transports that don't surface a wire object, with stale headers sanitized.metadata.headersstill exposes the original wire headers.transport.Responsefor customClientimplementers; always drop staleContent-Lengthin the synthetic (middleware-rewritten bodies make it stale even without compression); idiom cleanups (contextlib.suppress).ReflectapiOption; deleted the dead ADT tests and ported the rest toReflectapiPartialModelsemantics.Behavior preserved (verified during review): error message format (
httpx.codes.get_reason_phrasematchesreason_phrasefor all codes), charset handling (httpx's.json()isjson.loads(content)— no charset detection), exception-wrapping semantics, empty-body behavior, and chained/brotli/zstd encodings (the header strip is value-agnostic).Testing
error_modelfrom compressed responses, middleware-rewritten bodies under stale wire headers, non-JSON bodies on both paths, sanitizedraw_responsefallback (encoding stripped,Content-Lengthrecomputed), wire response preferred over synthetic. 9 of the 12 fail onmain, confirming they exercise the fix.client.py62 → 56 errors (all remaining pre-existing).