fix(core): detect non-SSE HTTP 200 responses in OpenAI streaming pipeline - #6466
Merged
Conversation
…line When a gateway/proxy intercepts a streaming request and returns HTTP 200 with a non-SSE content-type (e.g. text/html), the OpenAI SDK silently produces zero chunks, resulting in a confusing "Model stream ended without a finish reason" error and an empty interaction log (response: null, error: null). Add NonSSEResponseError that is thrown early when the response content-type is incompatible with SSE (not text/event-stream, application/json, etc.). The error carries diagnostic metadata: HTTP status, content-type, bounded body prefix, and request-id header — allowing users and maintainers to distinguish "empty model stream" from "gateway returned non-SSE page". Uses the OpenAI SDK's withResponse() API to access the raw HTTP response headers before consuming the stream iterator. Falls back gracefully when withResponse() is unavailable (e.g. in mocked environments). Closes #6465
Copilot
AI
changed the title
[WIP] Fix logging for empty OpenAI interaction on non-SSE 200 responses
fix(core): detect non-SSE HTTP 200 responses in OpenAI streaming pipeline
Jul 7, 2026
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
yiliang114
marked this pull request as ready for review
July 8, 2026 02:13
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present, linked issue #6465, risk analysis included. Problem: Observed bug with detailed reproduction. Issue #6465 documents a real gateway interception scenario — HTTP 200 with Direction: Clearly aligned. Detecting non-SSE responses early in the streaming pipeline is a focused, well-scoped fix for a real diagnostic blind spot. This is squarely within core's responsibility — the OpenAI streaming pipeline should surface transport-layer anomalies, not silently swallow them. Size: 2 files changed. Production: 114 additions + 3 deletions = 117 lines in Approach: Scope feels right. The change adds exactly what's needed: an Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有章节齐全，关联了 issue #6465，包含风险分析。 问题： 已观测到的 bug，有详细复现。Issue #6465 记录了真实的网关拦截场景 — HTTP 200 + 方向： 明确对齐。在 streaming pipeline 中提前检测非 SSE 响应，是一个针对真实诊断盲区的小范围修复。这属于 core 的职责范围 — OpenAI streaming pipeline 应该暴露传输层异常，而不是静默吞掉。 规模： 2 个文件。生产代码：114 行新增 + 3 行删除 = 117 行（ 方案： 范围合理。变更恰好添加了所需内容： 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewThe implementation is clean and well-targeted.
The Reuse check: No existing shared function covers content-type validation for SSE — this is genuinely new logic. The implementation is local to No blockers found. TestingUnit TestsAll 90 tests pass, including 7 new NonSSEResponse test cases covering: non-SSE detection with TypeCheckReal-Scenario Testing (tmux)This PR adds an error path triggered only when an upstream gateway returns HTTP 200 with a non-SSE content-type. In normal operation (real SSE endpoint), both before and after behave identically — the new code is a no-op for legitimate streams. A tmux before/after is not meaningful here since the bug requires a specific gateway proxy configuration that can't be simulated with a plain The 7 unit tests provide thorough coverage of the error path, including edge cases (body already consumed, 中文说明代码审查实现简洁且目标明确。
Reuse 检查： 没有现有的共享函数覆盖 SSE 的 content-type 验证 — 这确实是新逻辑。实现放在 未发现阻塞性问题。 测试单元测试全部 90 个测试通过，包含 7 个新的 NonSSEResponse 测试用例。 类型检查
真实场景测试（tmux）此 PR 添加的错误路径仅在网关返回 HTTP 200 + 非 SSE content-type 时触发。正常操作下（真实 SSE 端点），前后行为完全一致 — 新代码对合法 stream 是无操作。tmux before/after 在此处无意义，因为该 bug 需要特定的网关代理配置，无法通过普通的 7 个单元测试提供了对错误路径的全面覆盖，包括边界情况。这是传输层错误检测变更的适当验证策略。 — Qwen Code · qwen3.7-max |
Collaborator
|
This is a textbook bug fix: real user problem observed in production (issue #6465, filed by a maintainer), focused solution, thorough test coverage. The before/after story is clear: on main, a gateway block page returning My independent proposal before reading the diff was essentially the same approach: check content-type before iterating the stream, throw a descriptive error for non-SSE responses, handle the The 7 unit tests cover the meaningful paths: detection, passthrough, body prefix extraction, body read failure, and The dual field names ( Approving. ✅ 中文说明这是一个教科书式的 bug 修复：真实用户在生产中遇到的问题（issue #6465，由 maintainer 提交），聚焦的解决方案，充分的测试覆盖。 前后对比清晰：在 main 分支上，网关拦截页返回 我在读 diff 之前的独立方案基本相同：在迭代 stream 前检查 content-type，对非 SSE 响应抛出描述性错误，优雅处理 7 个单元测试覆盖了有意义的路径。TypeCheck 无错误。117 行生产代码用于一个聚焦的、单一目的的修复 — 没有多余内容。 双重字段名（ 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
approved these changes
Jul 8, 2026
yiliang114
enabled auto-merge
July 8, 2026 09:43
Collaborator
Manual tmux Test Report UpdateLatest PR commit: Automated verification after review fixes:
Review follow-up coverage added:
Real tmux interactive testing passed for both the non-SSE error path and the normal SSE happy path. These tmux cases were run at tmux case 1 - non-SSE gateway/block page:
Evidence from tmux capture: tmux case 2 - normal SSE happy path:
Evidence from tmux capture: Screenshot decision: no screenshot attached. These are text-only terminal error/success-path validations, and tmux text capture is more precise and searchable than a screenshot. There is no visual layout state that needs image evidence. 中文说明已补充真实 tmux 交互测试，覆盖两条端到端路径：
本轮 review 修复后，最新 commit 截图不需要附加：本次验证对象是终端文本错误/成功输出，tmux 文本捕获比截图更精确、可搜索，也不存在需要截图证明的 UI 布局状态。 |
wenshao
reviewed
Jul 8, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
pipeline.ts (PR description) |
PR description claims application/json is whitelisted ("The whitelist is intentionally broad (application/json etc.)") but the code explicitly excludes it. The test verifies application/json throws NonSSEResponseError. |
Update the PR description to match the code — remove the application/json claim from the whitelist description. |
pipeline.ts:101-107 |
Raw bodyPrefix (up to 512 chars of gateway response body) is embedded in Error.message with no sanitization. Propagates to terminal output, stderr, debug logs, and telemetry. Corporate gateway block pages may contain internal hostnames, tokens, or ANSI escape sequences. |
Keep bodyPrefix as a structured property only and exclude it from Error.message, or sanitize control characters and known secret patterns before embedding. |
pipeline.ts:83-84 |
application/x-ndjson and application/stream+json are whitelisted as SSE-compatible, but the OpenAI SDK's SSE parser expects data: prefixed lines. These formats don't use that prefix and would produce empty parsed output. |
Verify whether any real provider uses these content types for SSE-compatible streaming. If not, narrow the whitelist to only text/event-stream. If yes, add a code comment documenting the justification. |
pipeline.test.ts |
Test coverage gaps: no test for null/absent content-type (passes through as SSE-compatible), no test for fallback rejection path (no withResponse + network error), no test asserting error.message content. |
Add tests for null content-type and the fallback rejection path. Add at least one error.message assertion. |
pipeline.ts:353-355 |
bodyPrefix truncation uses .slice(0, 512) on decoded string (characters), but NON_SSE_BODY_PREFIX_LIMIT JSDoc says "Maximum bytes". For multi-byte content, actual byte count could exceed 512. |
Slice the Uint8Array before decoding (value.slice(0, 512)), or rename the constant and update JSDoc to clarify it's a character limit. |
— qwen3.7-max via Qwen Code /review
Collaborator
|
Disposition for the suggestion summary:
|
yiliang114
approved these changes
Jul 8, 2026
wenshao
reviewed
Jul 8, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
pipeline.ts:96-108 |
bodyPrefix embedded in Error.message triggers false positive in isTimeoutError() — gateway HTML containing "Request Timeout" causes the handler to re-wrap as a generic Error, destroying NonSSEResponseError prototype and all structured metadata. Also leaks up to 512 bytes of upstream body into every log sink. |
Remove bodyPrefix from the super() message; keep it only on the readonly bodyPrefix property. |
— qwen3.7-max via Qwen Code /review
Collaborator
|
@qwen-code /triage |
wenshao
approved these changes
Jul 8, 2026
Collaborator
|
@qwen-code /triage |
qqqys
approved these changes
Jul 8, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
No blocking issues found. I reviewed the non-SSE response detection path, error metadata propagation, and the new unit coverage. The targeted pipeline tests, core typecheck, core build, and diff whitespace check passed locally.
— GPT-5 via Qwen Code /review
qwen-code-ci-bot
approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does
Adds early detection of non-SSE responses in the OpenAI streaming pipeline. When
executeStreamreceives an HTTP 200 with a content-type incompatible with SSE (e.g.text/htmlfrom a gateway block page), it now throws aNonSSEResponseErrorwith diagnostic metadata instead of silently producing an empty stream.NonSSEResponseError— carriescontentType,httpStatus, boundedbodyPrefix(≤512 bytes), andrequestIdisSSECompatibleContentType()— whitelist check fortext/event-stream,application/json,application/x-ndjson,application/stream+jsonexecuteStreamuses.withResponse()— accesses raw HTTP headers via the OpenAI SDK'sAPIPromise.withResponse()before iterating; falls back to plainawaitwhen unavailable (mocks, older SDK versions)Why it's needed
When an upstream gateway/proxy intercepts a streaming request and returns HTTP 200 with
text/html, the OpenAI SDK's SSE parser silently produces zero chunks. Downstream this manifests as:{ response: null, error: null }in OpenAI interaction logs — indistinguishable from a genuine empty model responseReviewer Test Plan
How to verify
4 new tests cover: non-SSE detection with
text/html, body prefix extraction, passthrough fortext/event-stream, and graceful fallback without.withResponse().Evidence (Before & After)
N/A — error path only triggered by non-SSE upstream responses; validated via unit tests.
Tested on
Environment (optional)
Unit tests only (
npx vitest run).Risk & Scope
application/jsonetc.) to minimize false positives.NonSSEResponseError(currently treated as non-retryable by the error handler — appropriate since the condition won't self-resolve).Error; existing catch-all handlers will handle it. The fallback path preserves exact prior behavior when.withResponse()is unavailable.Linked Issues
Closes #6465
中文说明
做了什么
在 OpenAI streaming pipeline 中增加了对非 SSE 响应的早期检测。当
executeStream收到 HTTP 200 但 content-type 不兼容 SSE（如网关拦截页返回的text/html）时，立即抛出NonSSEResponseError，携带 HTTP 状态码、content-type、有界 body 前缀（≤512 字节）和 request-id。通过 OpenAI SDK 的
APIPromise.withResponse()在消费 stream iterator 之前获取原始 HTTP 响应头；当该方法不可用时优雅降级为原有行为。为什么需要
当上游网关/代理拦截 streaming 请求并返回 HTTP 200 +
text/html时，SDK 的 SSE 解析器静默产生零 chunk，导致：{ response: null, error: null }，无法与真正的空流区分风险