Skip to content

[Fix] generate: propagate the disconnect 400 instead of returning null#4782

Open
AmirF194 wants to merge 1 commit into
InternLM:mainfrom
AmirF194:fix/generate-disconnect-response-discarded
Open

[Fix] generate: propagate the disconnect 400 instead of returning null#4782
AmirF194 wants to merge 1 commit into
InternLM:mainfrom
AmirF194:fix/generate-disconnect-response-discarded

Conversation

@AmirF194

Copy link
Copy Markdown

Motivation

Fixes #4781. The non-streaming /generate endpoint aborts the session on client disconnect but discards the 400 response it computed, so the caller gets HTTP 200 with a null body instead. Same discard-the-inner-return shape as #4776/#4777, in the sibling /generate endpoint.

Modification

generate's _inner_call() returns the disconnect error response from inside its async with block, but the call site (await _inner_call()) never captured it, and the nonlocal response it otherwise sets is only reachable on the non-disconnect path. Capture _inner_call()'s return value and return it directly when not None, falling back to response only on the normal completion path:

inner_result = await _inner_call()
if inner_result is not None:
    return inner_result
return response

Verification

Docker (python:3.11-slim, CPU-only deps, no GPU), current main (9140d372). Added test_generate_disconnect_returns_client_disconnected_response to tests/test_lmdeploy/serve/test_session_cleanup.py, mirroring the existing test_completions_v1_disconnect_returns_client_disconnected_response in the same file: a fake AsyncEngine.generate() yields one item then blocks, and a fake raw_request.is_disconnected() returns True. Confirmed the test fails on unmodified main (result is None) and passes with this change. The full tests/test_lmdeploy/serve/test_session_cleanup.py file (10 tests) stays green, and pre-commit run --files on both changed files passes.

Not verified: the CUDA/GPU CI legs (no GPU on this machine) and the pre-existing failures in tests/test_lmdeploy/serve/openai/responses/ (unrelated pydantic validation errors in the /responses API, reproduced as already red on unmodified main, untouched by this change).

BC-breaking

None.

Checklist

  1. Pre-commit run on the changed files.
  2. Covered by a new unit test (fails on main, passes here).

Fixes InternLM#4781

_inner_call()'s disconnect branch returns create_error_response(400) from
inside its async with block, but the call site never captures it and the
nonlocal response it otherwise sets is only reachable on the normal
completion path. Capture _inner_call()'s return value and return it
directly when not None.
Copilot AI review requested due to automatic review settings July 24, 2026 13:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@AmirF194

Copy link
Copy Markdown
Author

Looked at the two failing jobs (pr_functions_test, unit_test). Both fail identically before any test runs: the turbomind CUDA extension build via cmake/ninja fails during pip install -e (exit 4 and 255 respectively, same cmake --build command). This diff only touches lmdeploy/serve/openai/api_server.py and its test file, no C++/CUDA sources, so the build failure isn't coming from this change. I haven't identified the actual cause of the build failure (looks like it could be a runner-level issue, not something I can confirm from the logs alone), so I won't guess at a fix, just flagging that it predates and is independent of this diff.

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.

[Bug] legacy /generate's non-streaming disconnect branch discards its 400 response, returns 200 null instead

2 participants