Handle structured output grammar compilation failures

[jeffye-dev]

Record exceptions from asynchronous structured-output grammar compilation instead of leaving requests blocked indefinitely waiting for a grammar that will never become available.

When a grammar future fails, finish the waiting request with FINISHED_ERROR and emit a request-local EngineCoreOutput with FinishReason.ERROR so the frontend can wake the generate task and surface the existing internal generation error path instead of timing out.

Keep blocked-request promotion focused on moving ready requests back to schedulable states, and handle grammar failures in a separate scheduler path. Use explicit structured-output fakes in scheduler tests to avoid Mock-fabricated grammar_error attributes.

Check streaming outputs for error finish reasons before emitting role, empty, or token chunks so streaming chat and completion requests surface the same internal generation error instead of starting a successful response.

Add regression coverage for failed grammar futures, scheduler traversal, frontend error-output delivery, streaming first-error outputs, and parallel-sampling parent state updates.

Purpose

This PR fixes a request hang that can happen when asynchronous structured-output
grammar compilation fails.

Before this change, a request waiting in
WAITING_FOR_STRUCTURED_OUTPUT_GRAMMAR could remain blocked forever if the
grammar future raised an exception. EngineCore would finish the request
internally in some paths, but the frontend was not guaranteed to receive a
request-level error output. As a result, the OpenAI serving layer could keep
waiting for a response until the client timed out.

This change records grammar compilation exceptions on StructuredOutputRequest
and handles them in a dedicated scheduler path. When a grammar future fails, the
scheduler finishes the request with FINISHED_ERROR and emits a request-local
EngineCoreOutput with FinishReason.ERROR. This reuses the existing frontend
internal generation error handling instead of introducing a new
EngineCore-to-frontend protocol.

The PR also updates streaming OpenAI chat and completion paths to check for
finish_reason="error" before emitting role, empty, or token chunks. This
prevents streaming requests from starting a successful-looking response before
surfacing the internal generation error.

Test Plan

• Run formatting/diff validation:

git diff --check

• Run focused scheduler and output-processor tests:

uv run --no-sync pytest \
  tests/v1/core/test_structured_output_grammar_failure.py \
  tests/v1/engine/test_output_processor.py::test_output_processor_delivers_error_finish_to_queue \
  tests/v1/engine/test_output_processor.py::test_output_processor_error_finish_updates_parallel_sampling_parent \
  tests/v1/core/test_scheduler.py::test_remote_kv_promotion_keeps_fcfs_with_grammar_prefix \
  tests/v1/core/test_scheduler.py::test_fcfs_mixed_skipped_waiting_types_keep_order

• Run focused OpenAI streaming error tests:

uv run --no-sync pytest \
  tests/entrypoints/openai/chat_completion/test_chat_error.py::test_chat_error_stream_first_empty_output \
  tests/entrypoints/openai/chat_completion/test_chat_error.py::test_chat_error_stream \
  tests/entrypoints/openai/completion/test_completion_error.py::test_completion_error_stream_first_empty_output \
  tests/entrypoints/openai/completion/test_completion_error.py::test_completion_error_stream

Test Result

passed

[jeffye-dev]

@russellb please take a review

[russellb]

Code Review

I've completed a comprehensive multi-angle review of this PR using automated analysis. Here are the 8 most critical findings ranked by severity:

---

Critical Correctness Issues

1. Grammar property can raise unexpected exceptions

File: vllm/v1/structured_output/request.py:587
Impact: Type contract violation - crashes instead of returning None
Scenario: When async grammar compilation fails, accessing .grammar property re-raises compilation exceptions (xgrammar errors). The return type annotation StructuredOutputGrammar | None promises only two states, but callers will receive unexpected exceptions. Code at __init__.py:251 expects None or object, not exceptions.
Fix: Catch exceptions in the grammar property getter and return None, storing error in grammar_error field.

2. Unatomic error output handling creates race window

File: vllm/v1/core/sched/scheduler.py:560
Impact: Potential duplicate error outputs or orphaned requests
Scenario: Error output appended at line 548, then finish_requests() called at line 560. If exception occurs between (OOM during EngineCoreOutput construction), request remains in WAITING status but error output is queued. Next schedule() call processes same request again, creating duplicate error outputs.
Fix: Use try/finally or move append inside finish_requests() to ensure atomicity.

3. Parallel output channel can desync

File: vllm/v1/core/sched/scheduler.py:548
Impact: Error outputs may be delivered out-of-order or lost
Scenario: finished_error_outputs list populated at 548, cleared at 1517. If update_from_output() isn't called after schedule(), or if exception occurs between append and clear, stale outputs accumulate. Creates temporal coupling between schedule() and update_from_output().
Fix: Consider making error outputs part of SchedulerOutput return value instead of side-channel list.

---

Performance Issues

4. Error check in hot streaming loop adds ~1000x overhead

File: vllm/entrypoints/openai/chat_completion/serving.py:413
Impact: Measurable CPU overhead for high-throughput streaming
Scenario: For 1000-token streaming response, _raise_if_error now called 1000 times (once per chunk) instead of once at finish. Each call does dictionary lookup and conditional. Scales poorly with concurrent streaming requests.
Assessment: This appears intentional per PR description ("check streaming outputs for error finish reasons before emitting"), but the performance trade-off should be acknowledged. Consider caching finish_reason or early-exit optimization.

---

Architectural / Maintenance Issues

5. Special-case error handling instead of generalized abstraction

File: vllm/v1/core/sched/scheduler.py:496
Impact: Maintenance burden grows with each async resource type
Scenario: Grammar errors get dedicated _finish_request_on_structured_output_grammar_error() method. Future async resources (tokenizer compilation, model loading, remote KV) will each need similar special cases, embedding subsystem knowledge into scheduler.
Recommendation: Consider generalizing as async resource error handler that works for any Future-based resource.

6. Duplicate error-checking across endpoints

Files: vllm/entrypoints/openai/chat_completion/serving.py:413, vllm/entrypoints/openai/completion/serving.py:438
Impact: Maintenance overhead, inconsistent evolution
Scenario: Same error-check-before-streaming pattern at completion:438 and chat:413. Future endpoints (embeddings, rerank) will cargo-cult one version, leading to drift.
Fix: Extract to shared helper method in base serving class.

7. Untyped state coupling with grammar_error field

File: vllm/v1/structured_output/request.py:574
Impact: State explosion, no type safety for invariants
Scenario: Three states (not-ready, success, failed) represented by two fields. Python typing can't enforce mutual exclusion. Adding more states (retry, degraded) creates combinatorial explosion.
Recommendation: Consider Result[Grammar, Exception] or tagged union pattern for type safety.

---

Test Quality Issues

8. 60-line test duplication

Files: tests/entrypoints/openai/chat_completion/test_chat_error.py:10, tests/entrypoints/openai/completion/test_completion_error.py:76
Impact: Parallel maintenance burden
Scenario: test_chat_error_stream_first_empty_output duplicates existing test_chat_error_stream with only assertions differing. Same in completion tests. Changes to mock setup must be applied twice.
Fix: Extract common setup to fixture or parameterize the existing tests.

---

Summary

The PR addresses a real bug (grammar compilation hangs), but the implementation has several issues:

• Critical: Fix the grammar property exception handling (finding # 1) to prevent crashes
• Important: Address the race condition in error output handling (finding # 2)
• Consider: The performance impact of hot-loop error checks may be acceptable for correctness, but should be measured
• Refactor: The architectural issues (# 5-# 7) create technical debt but don't block merging

The tests pass, which is good, but the findings suggest edge cases that aren't covered (exceptions during error output construction, grammar property exceptions propagating to callers).

[jeffye-dev]

Thanks @russellb , I've done some updates according to your feedback. and here are some comments:

1. Grammar property can raise unexpected exceptions: Actually this is the issue the PR focusing on resolving. It catches the exception from compile_xgrammar() to void crashing vLLM. With this PR, the grammar property does not raise exception any more.

2, 3, error output handling: The high-level step() in the EngineCore can ensure the schedule() and update_from_output() are called sequently in normal workflow. And I cleanup the staled finished_error_outputs at the begin of schedule() to avoid previous failed errors if have.

4, 6, updated the logics to correctly return 500 if there is error in the streamed chunks

5, update the function name to generic _finish_request_on_error() and can extend it in the future

7, update the logic, reset the grammar_error in the setter

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @jeffye-dev.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork