Repository navigation
fix(server): record invalid upstream responses as gateway errors - #658
Open
JianhaoYu (JianhaoYu-AI) wants to merge 1 commit into
Open
JianhaoYu (JianhaoYu-AI) wants to merge 1 commit into
JianhaoYu (JianhaoYu-AI) wants to merge 1 commit into
Conversation
Assisted-by: OpenAI Codex
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused change preserves valid-response behavior and cleanup, with regression coverage for the reported failures and no identified blocking issues.
No open issues 🎉
What changed in this PR
Fixes #655 by turning invalid upstream responses into structured gateway errors so failed model requests remain observable.
Changes:
- Handles malformed JSON, non-object JSON, and non-JSON responses, mapping statuses below 400 to 502 while preserving upstream error statuses.
- Uses the resulting status for both the recorded event and gateway response, leaving valid-object handling unchanged.
- Adds eight endpoint regression cases covering errors, event recording, retry counts, and in-flight cleanup.
| File | Description |
|---|---|
| tests/server/test_proxy_response.py | Adds endpoint coverage for invalid upstream responses and failure observability. |
| agentlightning/server/proxy.py | Normalizes invalid responses into structured errors before event capture. |
Review effort: Balanced
Author
|
The Test workflow is waiting for approval for this fork, while |
This branch has not been deployed
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
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.
Summary
Malformed upstream JSON and JSON values such as
nullor arrays currently raise before the gateway records a model-request event. Non-JSON HTTP 200 responses instead become empty successful responses.This change returns a structured upstream-response error, maps invalid responses below HTTP 400 to 502, and preserves existing upstream HTTP error statuses. The existing event path records the error and retry count; the in-flight counter is released as before. Valid object responses retain their current behavior.
Fixes #655.
Validation
Windows, Python 3.12.14, pytest 9.1.1; local dependencies, not the frozen lockfile. No model calls or GPU use.
python -m pytest -q tests/server/test_proxy_response.py tests/server/test_endpoints.py tests/test_client.py --tb=short: 51 passed.python -m pytest -q tests --tb=short: 117 passed, 9 failed, 3 skipped. The unchanged upstream baseline, excluding the new test file, gives 109 passed, the same 9 failed, 3 skipped. The failures reference POSIXos.killpg, unavailable on Windows; skips concern optional VERL dependencies.The new tests mock the upstream transport, call the public gateway endpoint, and inspect the returned error, stored event, retry count and in-flight counter. Existing tests cover valid completions and provider-error objects.
Scope and review status
This checks JSON decoding and the top-level object shape, not the entire provider schema. Linux CI, GPU training, frozen-lock validation and whole-project type checking were not run.
Duplicate-work checks found #587 (request-body validation) and #574 (triplet export), which address different paths. A refreshed open-PR search did not find this response-ingestion fix.
AI assistance was used for investigation, implementation and test execution. This is a draft; submitting-author review is still pending. No claim of completed human review is made.