Skip to content

fix(controller): stop sending a second response after a failed stream - #615

Merged
shipiyouniao merged 1 commit into
ovg-project:mainfrom
Lumea-art:fix/controller-stream-double-response
Nov 7, 2026
Merged

shipiyouniao merged 1 commit into
ovg-project:mainfrom
Lumea-art:fix/controller-stream-double-response

Conversation

@Lumea-art

Copy link
Copy Markdown
Contributor

What

Fixes #610.

The controller's completion proxy answered with a fresh web.Response(status=500)
from its catch-all except even after StreamResponse.prepare() had already put the
status line and headers on the wire. aiohttp then appended a second status line and
headers to the unfinished chunked body. With aiohttp 3.13/3.14, C-parser clients hang
until their own timeout and Python-parser clients report invalid chunk framing.

Changes

controller/frontend.py

  • handle_completion() / handle_chat_completion() now share
    _proxy_completion_request(); the duplicated ~55-line bodies are gone and each
    handler keeps its own upstream path and log label.
  • _relay_stream_response() commits the response only after prepare() succeeds.
    A failure past that point logs, truncates the connection
    (_abort_downstream_stream()), and returns the already-started response, so no
    second HTTP response is ever written. A failure before prepare() still
    propagates and becomes the ordinary JSON 500, as before.
  • result.release() still runs in finally on every path, so upstream connection
    cleanup and router/backend accounting are unchanged.
  • Handler return annotations are widened to web.StreamResponse, which is what the
    streaming path actually returns.

Tests

New tests/test_frontend_streaming.py, registered in tests/manifests/cpu.txt
(only needs aiohttp, which the CPU workflow already installs). It runs the real
handlers over a real aiohttp socket and reads the raw response bytes with a
hand-written HTTP request:

  • upstream stream fails after headers -> exactly one HTTP/1.1 status line, no
    500, first SSE event still delivered, body left truncated (parametrised over
    both endpoints);
  • healthy stream -> chunks relayed and the body terminated with 0\r\n\r\n;
  • router raises before headers -> ordinary JSON 500 with the error message;
  • missing model -> 400 and the router is never called.

Against the unfixed frontend.py the two truncation tests fail with
assert 2 == data.count(b"HTTP/1.1 "); with the fix all six pass.

Verification

pytest tests/test_frontend_streaming.py tests/test_sleep_manager.py tests/test_traffic_monitor.py
# 31 passed

ruff check controller/frontend.py tests/test_frontend_streaming.py   # All checks passed
isort --check-only controller/frontend.py tests/test_frontend_streaming.py  # clean
mypy --follow-imports skip --namespace-packages tests/test_frontend_streaming.py  # clean
python tools/check_test_classification.py  # test classification valid: total=110

Once StreamResponse.prepare() has written the status line and headers, the
catch-all handler in frontend.py still returned web.Response(status=500).
aiohttp appended that response to the unfinished chunked body, so clients
received two status lines in one response: C-parser clients hang until their
own timeout and Python-parser clients report invalid chunk framing.

Treat prepare() as the commit point. After it, _relay_stream_response() logs
and aborts the connection instead of starting a new response; before it, the
existing JSON 500 path is unchanged. Both completion handlers now share one
proxy helper, and result.release() still runs in finally so upstream cleanup
and backend accounting are untouched.

Add tests/test_frontend_streaming.py, which reads the raw downstream bytes
and asserts a single status line, the healthy chunked terminator, and the
pre-header error path.

Fixes ovg-project#610

@shipiyouniao shipiyouniao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified both completion endpoints through the normal frontend CLI and real router against a controlled HTTP backend (Python 3.11.15, aiohttp 3.14.3).

Base e0e4059 emits a second status line on upstream truncation. This head emits one, preserves the first SSE event, and closes without a final chunk, including keep-alive requests. Healthy streaming, non-streaming JSON, missing-model and routing-error paths also passed.

The new tests fail 2/6 on base and pass 6/6 on head. All nine CI checks are green. No blocking findings.

@shipiyouniao
shipiyouniao merged commit ef581b0 into ovg-project:main Nov 7, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Controller sends a second HTTP response after an upstream stream fails

2 participants