fix(frontend): return 4xx for Backend(InvalidArgument) on streaming c…

[ritazh]

…hat/responses

Overview:

A /v1/chat/completions or /v1/responses request with stream: true that hits a synchronous backend InvalidArgument at request-parse time (before any token is generated) currently emits:

• HTTP 200 (stream opens)
• SSE data frame: {"error":{"message":"Internal server error","type":"internal_server_error","code":500}}
• data: [DONE]

The identical request with stream: false correctly returns HTTP 400 with the typed message, because the non-streaming branch already calls check_for_backend_error() before folding the stream for chat_completions and responses. The streaming branches skip this check and rely on monitor_for_disconnects_with_timeout(), which hardcodes SanitizedError::Internal for any Err and loses the Backend(InvalidArgument) classification.

Reproducible with zai-org/GLM-5.2 (text-only) + image_url content + stream: true: the model correctly refuses (ValueError: Received multimodal data but multimodal processing is not enabled), then returns Backend(InvalidArgument), but the streaming SSE frame still surfaces code: 500.

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:

• Closes #XXXX

🚫 This PR is NOT linked to an issue:

• Confirmed — no related issue

Summary by CodeRabbit

• Bug Fixes
  • Streaming chat completion and response requests now return the correct HTTP error status when a backend error occurs before streaming begins.
  • Prevented initial backend errors from being incorrectly delivered as server-sent events after a successful response has already started.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

👋 Hi ritazh! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

[coderabbitai]

Walkthrough

Streaming chat-completions and responses handlers now inspect initial backend stream events before committing HTTP 200, returning typed HTTP errors for synchronous backend failures while preserving existing SSE behavior.

Changes

Streaming error handling

Layer / File(s)	Summary
Initial backend error checks lib/llm/src/http/service/openai.rs	Both streaming branches await check_for_backend_error after arming stream handling and return the detected typed ErrorResponse before starting a successful HTTP response.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The overview is detailed, but the Details and Where should reviewer start sections are left blank, so the template is only partially filled.	Fill in the Details and Where should reviewer start sections, or remove them if they are not required by this repository's PR template.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main fix: streaming chat/responses now return 4xx for Backend(InvalidArgument) instead of SSE 500 errors.
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]

Actionable comments posted: 1

🧹 Nitpick comments (4)

lib/llm/src/http/service/openai.rs (4)

1965-1982: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression test for the reported repro.

The PR fixes a concrete repro (text-only model + image_url content + stream: true → should be 4xx, not 200+500-SSE). Since this file review doesn't show a corresponding test update, consider adding one covering the streaming chat-completions path specifically (in addition to any responses-endpoint equivalent).

🤖 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 `@lib/llm/src/http/service/openai.rs` around lines 1965 - 1982, Add a
regression test for the streaming chat-completions path covering a text-only
model receiving image_url content with stream enabled. Assert that the request
returns the expected 4xx status and typed backend error response rather than
HTTP 200 followed by a generic 500 SSE frame, using the existing test helpers
and conventions.

---

1965-1982: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated arm+check+log+mark_error pattern.

This block (arm → check_for_backend_error → tracing::error! → mark_error) is now duplicated in the responses-streaming branch (lines 2457-2471) and mirrors the pre-existing non-streaming blocks (2050-2058, 2541-2549). Consider extracting a small shared helper to keep the 4xx/5xx-preserving contract consistent as this logic evolves.

🤖 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 `@lib/llm/src/http/service/openai.rs` around lines 1965 - 1982, The arm,
backend-error check, logging, and inflight error marking are duplicated across
streaming and non-streaming response paths. Extract this sequence into a small
shared helper, then use it from the affected branches around
check_for_backend_error while preserving the existing typed error response and
HTTP status behavior.

---

2457-2471: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Same test-coverage suggestion as the chat-completions streaming branch.

A regression test for the Responses streaming endpoint (image content vs text-only model, stream: true) would complement the one suggested at lines 1965-1982; see consolidated comment.

🤖 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 `@lib/llm/src/http/service/openai.rs` around lines 2457 - 2471, Add a
regression test for the Responses streaming endpoint covering an image-content
request sent to a text-only model with stream: true, and assert it returns the
expected typed 4xx backend error before committing HTTP 200. Place it alongside
the existing streaming error tests and mirror the chat-completions coverage
pattern referenced by check_for_backend_error.

---

2457-2471: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Same duplicated pattern as the chat-completions streaming branch.

Mirrors the arm+check+log+mark_error duplication flagged at lines 1965-1982; see consolidated comment.

🤖 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 `@lib/llm/src/http/service/openai.rs` around lines 2457 - 2471, The streaming
branch duplicates the arm, backend-error check, logging, and inflight
error-marking logic already used by the chat-completions path. Consolidate this
flow into the shared helper or abstraction introduced for that pattern, updating
the code around engine_stream and preserving client-disconnect handling, typed
error responses, tracing, and mark_error behavior.

🤖 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 `@lib/llm/src/http/service/openai.rs`:
- Around line 1965-1982: Wrap the check_for_backend_error(stream).await call in
the streaming setup with a bounded tokio::time::timeout so a stalled backend
cannot hold the request or permit indefinitely before HTTP 200. Preserve the
existing tracing, inflight_guard.mark_error, and err_response propagation for
backend errors, and map timeout expiration through the route’s established error
response path.

---

Nitpick comments:
In `@lib/llm/src/http/service/openai.rs`:
- Around line 1965-1982: Add a regression test for the streaming
chat-completions path covering a text-only model receiving image_url content
with stream enabled. Assert that the request returns the expected 4xx status and
typed backend error response rather than HTTP 200 followed by a generic 500 SSE
frame, using the existing test helpers and conventions.
- Around line 1965-1982: The arm, backend-error check, logging, and inflight
error marking are duplicated across streaming and non-streaming response paths.
Extract this sequence into a small shared helper, then use it from the affected
branches around check_for_backend_error while preserving the existing typed
error response and HTTP status behavior.
- Around line 2457-2471: Add a regression test for the Responses streaming
endpoint covering an image-content request sent to a text-only model with
stream: true, and assert it returns the expected typed 4xx backend error before
committing HTTP 200. Place it alongside the existing streaming error tests and
mirror the chat-completions coverage pattern referenced by
check_for_backend_error.
- Around line 2457-2471: The streaming branch duplicates the arm, backend-error
check, logging, and inflight error-marking logic already used by the
chat-completions path. Consolidate this flow into the shared helper or
abstraction introduced for that pattern, updating the code around engine_stream
and preserving client-disconnect handling, typed error responses, tracing, and
mark_error behavior.

🪄 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: 1eb19b42-9872-4a0d-a865-1150898577f2

📥 Commits

Reviewing files that changed from the base of the PR and between 8112c39 and 718b7d8.

📒 Files selected for processing (1)

• lib/llm/src/http/service/openai.rs

[datadog-official]

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 43.87% (-5.84%)

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 0f6f60a | Docs | Datadog PR Page | Give us feedback!}

[rmccorm4]

Hi @ritazh, thanks for the contribution!

1. Can you take a look into the failing test case: https://github.com/ai-dynamo/dynamo/actions/runs/29952953360/job/89035017580?pr=12036?
2. Can you check out the open review comments from Devin, dynamo-review-agent, and Coderabbit?

[ritazh]

Addressed review comments:

• replaced pre-peek stream_handle.arm() + check_for_backend_error(...).await with new streaming_preflight_peek helper: short 50ms tokio::select! window, Pin<Box<dyn Stream>> return so no borrowed lifetimes leak into Sse::new; falls through to SSE on timeout and lets monitor_for_disconnects own the long safety net. Extracted backend_error_response() shared with check_for_backend_error.

[devin-ai-integration]

Devin Review found 1 new potential issue.

[rmccorm4]

/ok to test b29377d

[ritazh]

@GuanLuo addressed your nits. PTAL. Good to merge?

[dynamo-ops]

/ok to test 0f6f60a

[devin-ai-integration]

Devin Review found 2 new potential issues.