Repository navigation
fix(sse): non-stream OpenAI-compatible requests no longer coerced to SSE when Accept lists application/json (#5305) #5309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { describe, it } from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import { | ||
| resolveStreamFlag, | ||
| acceptHeaderForcesStream, | ||
| } from "../../open-sse/utils/aiSdkCompat.ts"; | ||
|
|
||
| // #5305: the Vercel AI SDK / OpenAI SDK non-stream path (doGenerate/generateText) | ||
| // OMITS `stream` in the body and sends `Accept: application/json, text/event-stream`, | ||
| // then parses the response as JSON. OmniRoute was coercing such requests into SSE | ||
| // (route-level Accept override in src/sse/handlers/chat.ts + resolveStreamFlag), | ||
| // so the caller got `data: {...}` and failed with "Invalid JSON response". | ||
| // | ||
| // Rule: an Accept header that explicitly lists `application/json` is a JSON opt-in, | ||
| // even when it ALSO lists `text/event-stream`. Only a PURE SSE Accept header | ||
| // (text/event-stream WITHOUT application/json) forces streaming when `stream` is | ||
| // omitted. An explicit body `stream` value always wins. | ||
|
|
||
| describe("#5305 acceptHeaderForcesStream — route-level Accept streaming opt-in", () => { | ||
| it("does NOT force stream for the Vercel/OpenAI SDK non-stream signature (json + sse, stream omitted)", () => { | ||
| assert.equal(acceptHeaderForcesStream("application/json, text/event-stream", undefined), false); | ||
| }); | ||
|
|
||
| it("forces stream for a pure-SSE Accept header (text/event-stream only, stream omitted)", () => { | ||
| assert.equal(acceptHeaderForcesStream("text/event-stream", undefined), true); | ||
| }); | ||
|
|
||
| it("does NOT force stream for a pure-JSON Accept header", () => { | ||
| assert.equal(acceptHeaderForcesStream("application/json", undefined), false); | ||
| }); | ||
|
|
||
| it("never overrides an explicit body stream value (false or true)", () => { | ||
| assert.equal(acceptHeaderForcesStream("text/event-stream", false), false); | ||
| assert.equal(acceptHeaderForcesStream("text/event-stream", true), false); | ||
| }); | ||
|
|
||
| it("does not force stream when there is no Accept header", () => { | ||
| assert.equal(acceptHeaderForcesStream(undefined, undefined), false); | ||
| assert.equal(acceptHeaderForcesStream("", undefined), false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("#5305 resolveStreamFlag — openai non-stream with mixed Accept", () => { | ||
| it("defaults to NON-stream (JSON) for openai + omitted stream + `application/json, text/event-stream`", () => { | ||
| assert.equal(resolveStreamFlag(undefined, "application/json, text/event-stream", "openai"), false); | ||
| }); | ||
|
|
||
| it("keeps streaming for openai + omitted stream + pure text/event-stream Accept", () => { | ||
| assert.equal(resolveStreamFlag(undefined, "text/event-stream", "openai"), true); | ||
| }); | ||
|
|
||
| it("keeps streaming for openai + omitted stream + no/`*/*` Accept (legacy default unchanged)", () => { | ||
| assert.equal(resolveStreamFlag(undefined, undefined, "openai"), true); | ||
| assert.equal(resolveStreamFlag(undefined, "*/*", "openai"), true); | ||
| }); | ||
|
|
||
| it("explicit body stream:true still wins over a json-leaning Accept", () => { | ||
| assert.equal(resolveStreamFlag(true, "application/json, text/event-stream", "openai"), true); | ||
| }); | ||
|
|
||
| it("a forceStream provider still streams regardless of the json Accept (#2081 preserved)", () => { | ||
| assert.equal( | ||
| resolveStreamFlag(undefined, "application/json, text/event-stream", "openai", { | ||
| providerRequiresStreaming: true, | ||
| }), | ||
| true | ||
| ); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repository uses Vitest as its primary testing framework (as indicated by
vitest.config.tsin the root and the repository style guide references to Vitest files). Importing fromnode:testandnode:assertintroduces inconsistency and can cause issues with Vitest's test runner, coverage tools, or mocking features. Usingvitestinstead of the Node.js built-in test runner ensures consistency across the test suite.