fix(inference-gateway): decode compressed middleware responses - #1218
Conversation
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
📝 WalkthroughWalkthroughThe inference gateway now enables aiohttp upstream content decoding. Decoded content feeds SSE and JSON processing. Payload-decoding and JSON-processing failures return distinct 502 errors. Tests cover headers, gzip JSON, gzip SSE, and failure paths. ChangesUpstream response decoding
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Upstream
participant fetch_proxy_response
participant SSEParser
participant JSONParser
Upstream->>fetch_proxy_response: Encoded response
fetch_proxy_response->>SSEParser: Decoded streaming chunks
fetch_proxy_response->>JSONParser: Decoded buffered body
JSONParser-->>fetch_proxy_response: Parsed object or 502 error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.py`:
- Around line 486-499: Update the decompression flow around the chunk loop and
final decoder.flush() to track whether the final compressed member completed
before aiohttp can reset decoder state. Require gzip/deflate
ZLibDecompressor.eof and equivalent end-of-stream completion indicators for
Brotli and Zstandard; raise UpstreamContentDecodingError(content_encoding) for
incomplete bodies, while preserving normal decoded and trailing output.
- Around line 519-524: Update _parse_sse_stream and the surrounding
stream_response_result flow so UpstreamContentDecodingError raised lazily by
_decode_content_chunks or _parse_sse_chunks is handled before SSE output begins,
returning the established 502 response instead of propagating after headers are
sent. Add a regression test covering corrupt gzip content in an SSE response and
verify no partial successful response is emitted.
- Around line 458-471: Update _build_content_decoder to parse comma-separated
Content-Encoding values, ignore surrounding whitespace, and construct a decoder
chain in reverse application order so values such as “gzip, br” decode
correctly. Preserve identity/empty handling and unsupported-coding errors, then
add buffered JSON and fragmented SSE coverage for chained encodings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a10fde9a-7255-4d13-8dad-a0618ccea8ae
📒 Files selected for processing (4)
services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/service.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/testing/_loopback.pyservices/core/inference-gateway/tests/unit/test_proxy.py
|
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/core/inference-gateway/tests/unit/test_proxy.py`:
- Around line 1514-1532: Move the header-sanitization assertions from
test_fetch_proxy_response_decodes_chained_json to a stream_response_result test
that exercises re-serialization. Assert that content-length, content-encoding,
and transfer-encoding are absent from the serialized response headers, while
keeping this fetch_proxy_response test focused on chained JSON decoding and
preserving the direct proxy path’s content-encoding behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bf38e0cc-8b97-430a-83c8-f09c0394241e
📒 Files selected for processing (2)
services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.pyservices/core/inference-gateway/tests/unit/test_proxy.py
🚧 Files skipped from review as they are similar to previous changes (1)
- services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.py
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
Signed-off-by: Brian Newsom <brnewsom@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.py`:
- Line 492: Restore chained Content-Encoding support in the proxy response
handling around auto_decompress=True by explicitly decoding ordered coding
chains or preserving a decoder chain for multi-coding responses, including
combinations such as gzip, deflate. In
services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.py
lines 492-492, update the implementation; in
services/core/inference-gateway/tests/unit/test_proxy.py lines 1498-1518, add
coverage for chained JSON responses and fragmented chained SSE responses. Use uv
for Python tooling.
- Around line 519-522: Update the compressed SSE handling in _parse_sse_stream’s
caller so compressed responses are fully buffered and validated before
constructing or returning ResponseResult, while uncompressed SSE remains
live-streamed. Add corrupt compressed-SSE coverage in
services/core/inference-gateway/tests/unit/test_proxy.py:1553-1581 asserting the
gateway returns 502 before any response iterator is exposed; the proxy
implementation change is required at
services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.py:519-522.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9ba3af43-8e0c-46e7-9afc-894c63a1481f
📒 Files selected for processing (4)
services/core/inference-gateway/src/nmp/core/inference_gateway/api/proxy.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/service.pyservices/core/inference-gateway/src/nmp/core/inference_gateway/testing/_loopback.pyservices/core/inference-gateway/tests/unit/test_proxy.py
🚧 Files skipped from review as they are similar to previous changes (2)
- services/core/inference-gateway/src/nmp/core/inference_gateway/service.py
- services/core/inference-gateway/src/nmp/core/inference_gateway/testing/_loopback.py
Summary
Inference middleware uses the gateway's non-decompressing HTTP session but then parses upstream bytes as JSON or SSE. When a sandbox egress proxy negotiates gzip, that path sees compressed bytes and returns a misleading JSON 502 or an empty event stream. Enable aiohttp decompression only for middleware-aware requests while leaving direct proxy responses byte-transparent.
Changes
auto_decompress=Trueon the middleware-aware upstream request; keep the shared session default disabled for direct proxying.Scope boundary
Content-Encodinghandling to aiohttp instead of maintaining gateway-specific codecs.Accept-Encodingheaders or sandbox proxy configuration; the gateway now handles compression negotiated anywhere on the upstream path.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
uv run --frozen pytest services/core/inference-gateway/tests/unit/test_proxy.py -k 'fetch_proxy_response_decodes_gzip' -vfailed both regression cases with the reported JSON 502 and an empty SSE event list.uv run --frozen pytest services/core/inference-gateway/tests/unit -v— 463 passed, 5 skipped.uv run --frozen pre-commit run -a— all hooks passed.make refresh-openapi— passed with no generated changes.external-onlysbx sandbox against this branch's local gateway.HTTP_PROXY/HTTPS_PROXYwere set,host.docker.internalwas absent from bothNO_PROXYandno_proxy, and the run command did not override either bypass list. Gateway chat-completion requests returned 200 rather than the reported non-JSON 502.run.json.status=completedwithwinner_agent=agent-1and validationreward=1.0,shape_ok=1.0. One unrelated upstream request stalled with the gateway's existing unbounded timeout; interrupting and rerunning the documented command with the same experiment directory resumed successfully from persisted state.uv run --frozen pytest plugins/nemo-experimentalist/tests/experimentalist/ -k smoke -q— 76 passed, 15 skipped, 321 deselected.