[Bugfix] Handle HarmonyError in process_chunk to fix gpt-oss streaming 500s - #52055
[Bugfix] Handle HarmonyError in process_chunk to fix gpt-oss streaming 500s#52055rajathpi wants to merge 3 commits into
Conversation
…g 500s process_chunk() called the Harmony streamable parser with no exception handling, so a malformed message header (for example a duplicated `to=` clause) raised HarmonyError all the way out to the API server and failed the request with a 500. flush() already catches HarmonyError and recovers by returning the raw decoded text. Give process_chunk() the same contract: decode the buffered tokens plus the unconsumed tail of the chunk as raw final-channel text, emit it as a delta segment plus a completed message so streaming, non-streaming parse(), and the Responses message loop all surface it, reset parser state, and stop processing the chunk. Fixes vllm-project#51977. Assisted-by: AI coding assistant Signed-off-by: rajathpi <rajathpai2000@gmail.com>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
|
@claude review |
|
for me this looks good |
|
Looks good to me. |
|
/ci run |
|
❌ @rajathpi, A reviewer with write access must run |
FIX #51977
Purpose
gpt-oss requests fail with HTTP 500 (
openai_harmony.HarmonyError: unexpected tokens remaining in message header) when the model samples a malformed Harmonymessage header.
HarmonyParser.flush()already catchesHarmonyErrorandrecovers by returning the raw decoded text, but
process_chunk()callsself._harmony_parser.process(token_id)with no handling, so a mid-stream parsefailure escapes to the API server and kills the request.
This gives
process_chunk()the same recovery contract asflush(): onHarmonyError, decode the buffered message tokens plus the unconsumed tail ofthe chunk as raw final-channel text, emit it as a delta segment plus a completed
message, reset parser state, and stop processing the chunk. The dual-segment
shape matches what
flush()already emits, so streaming, non-streamingparse(), and the Responses message loop all surface the text.parse()andHarmonyContext.append_outputread onlycompleted_message, so a delta-onlysegment would have turned the 500 into an empty 200 instead.
Nothing the model generated is dropped, and the request completes.
Note on the trigger: the issue points at a hyphen in the tool name, but
openai_harmony 0.0.8 accepts hyphenated recipients. Reproducing directly against
the library shows the real trigger is leftover tokens in the header. A duplicated
to=clause reproduces the reporter's exact error string, which also explains whythe failure was intermittent rather than reliable.
Scope: vLLM cannot stop a model sampling a malformed header, so it has to tolerate
HarmonyErrorfrom the library.flush()established that defensive contractalready; this completes it for the mid-stream path. A leniency fix upstream in
openai_harmony would be complementary, not a substitute.
Not duplicating existing work: #51977 is unclaimed, and no open PR references it or
touches
vllm/parser/harmony.py.Test Plan
New regression test
tests/parser/test_harmony.py::test_process_chunk_recovers_from_malformed_headerdrives a duplicated
to=header throughprocess_chunk, asserts the generatedtext comes back as a final-channel delta segment plus a completed message instead
of an exception, then asserts a normal message parses afterwards to prove the
state reset. It fails without the source change.
Test Result
With
vllm/parser/harmony.pyreverted, the new test fails:ruff checkandruff format --checkclean on both files; mypy (CIconfiguration) reports no issues on either file.
No model evaluation run: only the error-recovery path changes, and behaviour on
well-formed output is untouched, covered by the 65 pre-existing tests in this file.
This change was developed with AI assistance; I reviewed and tested every line.