Skip to content

fix: convert conditional disagg sglang warning to httperror 400 - #12578

Merged
karen-sy merged 2 commits into
mainfrom
karenc/condp-runtimeerror-fix
Aug 4, 2026
Merged

fix: convert conditional disagg sglang warning to httperror 400#12578
karen-sy merged 2 commits into
mainfrom
karenc/condp-runtimeerror-fix

Conversation

@karen-sy

@karen-sy karen-sy commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Overview:

The SGLang backend correctly rejects the conditional-disaggregation annotation x-bypass-remote-prefill and raises a message written specifically for the user:

Detected request annotation 'x-bypass-remote-prefill', but SGLang backend does not support conditional disaggregation yet. Use vLLM or TensorRT-LLM for conditional disaggregation.

That message is present in full in both the worker log and the frontend log, but the client receives:

{"message":"Internal server error","type":"Internal Server Error","code":500} The backend expresses the rejection as a bare RuntimeError, which the frontend classifies as 5xx, and 5xx bodies are deliberately replaced with a generic message by SanitizedError. The result: the one sentence that tells the user which backend to switch to never reaches them.

From the caller's point of view a self-correctable configuration mistake presents as an unattributable internal server error.

Details:

Where should the reviewer start?

Related Issues

⚠️ This section is required. Choose one path below and delete the other.

🔗 This PR is linked to an issue:


Open in Devin Review

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling for unsupported conditional disaggregation bypass requests.
    • Requests now return a clear HTTP 400 response with guidance to use vLLM or TensorRT-LLM instead.

Signed-off-by: Karen Chung <karenc@nvidia.com>
@karen-sy
karen-sy requested a review from a team as a code owner August 3, 2026 17:44
@github-actions github-actions Bot added fix backend::sglang Relates to the sglang backend labels Aug 3, 2026

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@datadog-official

datadog-official Bot commented Aug 3, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 15.21% (-32.37%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 75b192d | Docs | Datadog PR Page | Give us feedback!

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Decode handler error handling

Layer / File(s) Summary
Return HTTP validation errors
components/src/dynamo/sglang/request_handlers/llm/decode_handler.py
The handler raises HttpError(400, ...) instead of RuntimeError and identifies vLLM and TensorRT-LLM as supported backends.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes converting the SGLang conditional disaggregation failure to an HTTP 400 error.
Description check ✅ Passed The description explains the problem and intended fix, and identifies the related issue, but leaves Details and reviewer-start guidance incomplete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
components/src/dynamo/sglang/request_handlers/llm/decode_handler.py (1)

50-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the new HttpError path.

generate now raises HttpError for this request condition, but its Raises section still lists only RuntimeError on Lines 354-356. Add the HTTP 400 behavior to the docstring.

🤖 Prompt for 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.

In `@components/src/dynamo/sglang/request_handlers/llm/decode_handler.py` around
lines 50 - 54, Update the generate method’s docstring Raises section to document
the new HttpError behavior: it raises HTTP 400 when
BYPASS_REMOTE_PREFILL_ANNOTATION is requested but unsupported by the SGLang
backend. Preserve the existing RuntimeError documentation.
🤖 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 `@components/src/dynamo/sglang/request_handlers/llm/decode_handler.py`:
- Around line 50-54: Update the streaming HTTP configuration so the
status-preserving pre-commit error peek is enabled by default, allowing the
HttpError raised in DecodeWorkerHandler.generate for
BYPASS_REMOTE_PREFILL_ANNOTATION to produce HTTP 400 instead of a committed 200.
Add an endpoint test covering x-bypass-remote-prefill that asserts the 400
response and includes the conditional-disaggregation guidance message.

---

Nitpick comments:
In `@components/src/dynamo/sglang/request_handlers/llm/decode_handler.py`:
- Around line 50-54: Update the generate method’s docstring Raises section to
document the new HttpError behavior: it raises HTTP 400 when
BYPASS_REMOTE_PREFILL_ANNOTATION is requested but unsupported by the SGLang
backend. Preserve the existing RuntimeError documentation.
🪄 Autofix (Beta)

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: e12f2c19-4edc-4794-94e1-1d2e6413c840

📥 Commits

Reviewing files that changed from the base of the PR and between 74bf051 and 0db2a74.

📒 Files selected for processing (1)
  • components/src/dynamo/sglang/request_handlers/llm/decode_handler.py

Comment thread components/src/dynamo/sglang/request_handlers/llm/decode_handler.py
@karen-sy
karen-sy enabled auto-merge (squash) August 4, 2026 00:57
@karen-sy
karen-sy merged commit c45d976 into main Aug 4, 2026
103 checks passed
@karen-sy
karen-sy deleted the karenc/condp-runtimeerror-fix branch August 4, 2026 01:00
hhzhang16 added a commit that referenced this pull request Aug 4, 2026
dyn-3691-extract-shared-target-pid-cuda-customstorage-operation-layer

* 'main' of https://github.com/ai-dynamo/dynamo: (50 commits)
  docs(cli): correct removed vLLM prefill-worker flag reference (#12581)
  docs(operator): reserve webhook Ignore for emergencies (#12563)
  ci(docs): make previews and checks match what actually publishes (#12339)
  refactor(vllm): organize custom encoder modules (#12416)
  feat(llm): Select reasoning output field via env var (#11464)
  feat(runtime): add TLS support to TCP request plane (#10921)
  fix: convert conditional disagg sglang warning to httperror 400 (#12578)
  feat(operator): add runtime feature gates (#12421)
  refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait (#12447)
  feat(replay): add deterministic canonical offline reports (#12363)
  build: bump ModelExpress to 0.5.0(OPS-7978) (#12455)
  fix(mocker): use logical KV tokens for decode timing (#12583)
  fix(examples): update Triton example for CUDA 13 + fix libdcgm copy (DYN-3697) (#12577)
  refactor(operator): implement composition-first DGD reconciliation (#12283)
  feat(frontend): add basetenkenizer backend (#12376)
  fix(profiler): configure rapid mocker without planner (#12573)
  docs(vllm): correct worker-role flags and document --kv-transfer-config (#12568)
  ci: add Kubernetes deploy test to nightly (#12090)
  fix(container): reuse pinned protoc in runtime image (#12535)
  feat(self-host): flip DYN_SELF_HOST_METADATA default to ON (gh-8749) (#11417)
  ...

Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::sglang Relates to the sglang backend fix size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants