[Router][Bugfix] propagate client cancellation - #1040
Open
unsorted-otter wants to merge 6 commits into
Open
Conversation
The router waited for backend response headers before ever checking for a client disconnect, so a non-streaming request kept running on the engine after the client left. Fixes vllm-project#634 Signed-off-by: unsorted-otter <unsorted.otter@proton.me>
Shield the generator close, stop swallowing cancellation aimed at our own task, and await the disconnect listener instead of only cancelling it. Signed-off-by: unsorted-otter <unsorted.otter@proton.me>
unsorted-otter
requested review from
ApostaC,
Shaoting-Feng,
YuhanLiu11 and
ruizhang0101
as code owners
August 19, 2026 06:40
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements client disconnect handling in the router, aborting backend requests and returning a 499 status code if a client disconnects before a response is sent. It also ensures request statistics are properly cleaned up on all exit paths. The review feedback highlights a potential resource leak in the disconnect handling logic, Python compatibility issues with asyncio.Task.cancelling() on Python versions older than 3.11, and a recommendation to make the log sanitization helper safer against non-string inputs.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: unsorted-otter <unsorted.otter@proton.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: unsorted-otter <unsorted.otter@proton.me>
Signed-off-by: unsorted-otter <unsorted.otter@proton.me>
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.
When a client disconnects from a non-streaming request, the router just keeps waiting for the backend's response headers, it never checks whether the client is still around. So the engine has no way to know it should stop, runs the request to completion, and the router throws the answer away.
This races waiting for the backend headers against the client disconnecting, same idea as vLLM's own
entrypoints.utils.with_cancellation. Whichever happens first wins: if the client leaves, the pending call gets cancelled, which closes the backend connection and lets the engine abort. A normal response works exactly as before.#867 already tried to fix this and never got merged. Two differences here, both directly answering what that review pointed out:
request.is_disconnected(). vLLM's own code avoids that on purpose, because it breaks once there's middleware in the stack. This uses a background task racingrequest.receive()forhttp.disconnectinstead, same aswith_cancellation.X-Request-Idheader. The one new log line here strips that before logging it.Also moved
on_request_completeinto afinallyblock, since it used to only run after a successful response. A cancelled or failed request stayed marked as in-flight forever, and the routing logic kept treating that engine as busier than it really was.Test plan
src/tests/test_client_disconnect.py: covers a client disconnecting before the backend responds (checks the backend call actually gets torn down, not just that we return 499), a normal connected client still getting its response, and the cleanup helper correctly telling apart its own cancellation from an unrelated one hitting the caller at the same time (there's a test that fails against the naive version of that check, so it doesn't creep back in).pytest src/tests/test_client_disconnect.pylocally, all 6 pass.black,isort, andruffall pass on the changed files.Fixes #634
-swhen doinggit commit[Bugfix],[Feat], and[CI].Detailed Checklist (Click to Expand)
Thank you for your contribution to production-stack! Before submitting the pull request, please ensure the PR meets the following criteria. This helps us maintain the code quality and improve the efficiency of the review process.
PR Title and Classification
Please try to classify PRs for easy understanding of the type of changes. The PR title is prefixed appropriately to indicate the type of change. Please use one of the following:
[Bugfix]for bug fixes.[CI/Build]for build or continuous integration improvements.[Doc]for documentation fixes and improvements.[Feat]for new features in the cluster (e.g., autoscaling, disaggregated prefill, etc.).[Router]for changes to thevllm_router(e.g., routing algorithm, router observability, etc.).[Misc]for PRs that do not fit the above categories. Please use this sparingly.Note: If the PR spans more than one category, please include all relevant prefixes.
Code Quality
The PR need to meet the following code quality standards:
pre-committo format your code. SeeREADME.mdfor installation.DCO and Signed-off-by
When contributing changes to this project, you must agree to the DCO. Commits must include a
Signed-off-by:header which certifies agreement with the terms of the DCO.Using
-swithgit commitwill automatically add this header.What to Expect for the Reviews
We aim to address all PRs in a timely manner. If no one reviews your PR within 5 days, please @-mention one of YuhanLiu11
, Shaoting-Feng or ApostaC.