test(sdk-java): close the connection on body-less fixture responses - #10472
Conversation
The Java 11 lane of SDK Java fails intermittently in the daemon client tests: a session teardown raises DetachOutcomeUnknownException or MutationOutcomeUnknownException wrapping "HTTP/1.1 header parser received no bytes". It has hit five different test methods over the past day on unrelated branches, while Java 17, Java 21, macOS and Windows stay green in the same runs. The fixture causes it. On Java 11 the JDK's own HTTP server drops the connection after a response with no body at all -- a 204 detach, a 204 cancel, a 204 session delete, an empty event stream -- while the Java 11 HttpClient keeps that connection pooled. The next mutation the SDK sends over it reads EOF before any response byte, and since the request is not idempotent the SDK correctly declines to retry and reports the outcome as unknown. Pairing the halves across JDKs shows both must be Java 11: a Java 11 server with a Java 21 client never fails, nor does a Java 21 server with a Java 11 client, so only the fixture is affected. Route every body-less response through a helper that marks the response non-persistent, so the client never pools a connection the fixture is about to drop. This generalises #10365, which applied the same treatment to two handlers of a single test. On Java 11, pinned to one core: the three test methods seen failing in CI failed 50 of 200 rounds before this change and 0 of 200 after.
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed, with unusually strong evidence. The Java 11 lane failure has hit five test methods across unrelated branches in the last day, and the PR pins the root cause with cross-JDK pairing (fails only when both the JDK HTTP server and the Java 11 HttpClient are on 11), plus a before/after loop (50/200 rounds failing → 0/200). This is the same flake #10365 patched for two handlers, recurring elsewhere in the fixture. Direction: aligned. A flaky fixture lane blocks unrelated branches; this finishes what #10365 started rather than adding anything new. Test-only, no SDK behavior changes. Size: not applicable — single Java test file, +30/-37, zero production lines. Approach: scope is exactly right. One shared Risk: no elevated risk signals (no high-risk paths touched). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实观测到的问题，且证据相当充分。Java 11 lane 的失败在最近一天内命中了三个互不相关分支上的五个测试方法；PR 通过跨 JDK 组合实验定位了根因（仅当 JDK HTTP server 与 Java 11 HttpClient 两端都是 Java 11 时才失败），并给出 before/after 循环数据（200 轮中 50 轮失败 → 0 轮失败）。这正是 #10365 为两个 handler 修过的同一个 flake，在夹具其他位置再次出现。 方向：对齐。不稳定的夹具会阻塞无关分支；本 PR 是 #10365 的收尾，没有新增任何东西。纯测试改动，SDK 行为不变。 规模：不适用——单个 Java 测试文件，+30/-37，生产代码零改动。 方案：范围恰到好处。一个共享的 风险：无升级风险信号（未触及高风险路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — no findings. My independent take before reading the diff: the fixture needs every body-less response to end its connection with the response, through one helper, generalizing what #10365 did for two handlers. That is exactly what this PR does. Verified against the base file: all eleven Testing — the PR's own CI, read via the API on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 About the one red check, Note: the loop evidence in the PR body (200 rounds before/after) is the author's own measurement, cited here as context, not as independently re-run evidence. The CI signal that matters — the Java 11 lane passing the full suite on this commit — is real and green. The sandboxed lanes don't apply here: there is no product behavior change to A/B, and the race lives in the Java 11 JDK fixture, which 中文说明代码审查——未发现问题。 在读 diff 之前我的独立想法是：夹具需要让每一个无响应体的返回在响应的同时结束连接，并且统一走一个辅助方法，把 #10365 对两个 handler 做的事情推广开来。这个 PR 正是这么做的。 已对照基线文件核实：夹具中全部十一处 测试——通过 API 读取该提交（ 关于唯一的红检 另注：PR 正文中的循环数据（200 轮 before/after）是作者自己的测量，此处仅作为背景引用，不是独立复跑的证据。真正关键的 CI 信号——Java 11 lane 在该提交上完整跑通测试套件——是真实的且为绿色。沙箱 lane 在此不适用：没有可供 A/B 的产品行为变化，且竞态存在于 Java 11 JDK 夹具中， — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal fix that finishes #10365's job; one unrelated-looking red check and two CI runs still in flight keep this from a 5. This is exactly the kind of PR you want to see for a flake: a real, repeatedly observed failure, a root cause proven by cross-JDK pairing rather than guessed, and the smallest change that closes the hole for good — one helper, every body-less response routed through it, net -7 lines. My independent proposal before reading the diff was the same approach, and I found no simpler path. Nothing about it tries too hard; in six months the javadoc on The reason it isn't 5/5 is the Approval is deferred until CI lands green on 中文说明这正是处理 flake 时希望看到的 PR：真实且反复观测到的失败、通过跨 JDK 组合实验而非猜测证明的根因，以及一次性堵住漏洞的最小改动——一个辅助方法、所有无响应体返回都走它、净 -7 行。我在读 diff 之前独立想到的方案与此相同，也没有找到更简的路径。它没有任何过度设计；六个月后 没给到 5/5 的原因是 — Qwen Code · qwen3.8-max Reviewed at |
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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 4": none — all checks I planned completed within budget (~5 of ~32 tool calls used)..
Test Plan (not a blocker): HTTP/1.1 — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 4"：none — all checks I planned completed within budget (~5 of ~32 tool calls used).。
Test Plan（非阻断）：HTTP/1.1 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| */ | ||
| private static void sendNoContent(HttpExchange exchange, int status) | ||
| throws IOException { | ||
| exchange.getResponseHeaders().set("Connection", "close"); |
There was a problem hiding this comment.
[Suggestion] R1-1: No committed test pins this fix's only behavioural line — the Connection: close header set inside the new sendNoContent helper. Nothing goes red if the header line is removed, and nothing enforces that future body-less fixture responses go through the helper (the consolidation is a convention, not an enforced one). If a later edit deletes the line — or a new 204/empty-stream handler is written with raw sendResponseHeaders(status, -1) — the Java 11 pool race this PR removes silently comes back: per the PR's own measurements the flake lands roughly once per twelve two-core class runs and intermittently in CI while Java 17/21 lanes stay green, so ordinary full-suite runs pass and the regression surfaces only as sporadic DetachOutcomeUnknownException failures on unrelated branches — the exact symptom this PR exists to end.
Witness:
intact tree — header-assertion probe: Tests run: 1, Failures: 0 (Connection: close observed)
mutant tree — header line deleted:
probe: AssertionFailedError: response carries a Connection header ==> expected: <true> but was: <false>
committed: DaemonSessionClientTest Tests run: 103, Failures: 0, Errors: 0 — BUILD SUCCESS
(nothing else pins the line on JDK 21)
A small deterministic test can pin the mechanism: serve the fixture's body-less handler on an HttpServer and assert the java.net.http.HttpClient response carries Connection: close — this is the probe-verified shape (it flips red/green exactly with the header line). The optional stronger variant is a Java-11-gated loop test of "body-less response, then a mutation request" in the shape of the pairing harness from the PR description.
The new assertion test itself must go red when the Connection: close line is removed from sendNoContent — please prove it with the mutation: remove the header line, run that test, confirm it goes red.
中文说明
没有任何已提交的测试钉住本次修复唯一的行为改动——新 sendNoContent 辅助方法里设置 Connection: close 响应头的那一行。如果这行被删掉，不会有任何测试变红；也没有任何机制强制未来新增的"无响应体"返回必须走这个辅助方法（目前的归并只是约定，并非强制）。一旦后续编辑删掉这行——或者有人用裸的 sendResponseHeaders(status, -1) 新写了一个 204/空事件流 handler——本 PR 消除的 Java 11 连接池竞态就会悄悄回来：按本 PR 自己的测量，该 flake 在双核下大约每十二次类级运行命中一次，在 CI 中间歇性出现，而 Java 17/21 lane 始终为绿——所以普通的全量测试会通过，回归只会以不相干分支上偶发的 DetachOutcomeUnknownException 失败形式浮现，而这正是本 PR 要终结的症状。
验证证据（探针翻转）：完整代码树上，响应头断言探针 Tests run: 1, Failures: 0（观察到 Connection: close）；删除该行响应头的突变树上，探针报 AssertionFailedError: response carries a Connection header ==> expected: <true> but was: <false>，而已提交的完整 DaemonSessionClientTest 在 JDK 21 上仍为 Tests run: 103, Failures: 0, Errors: 0 — BUILD SUCCESS——说明没有任何其他测试钉住这一行。
一个小的确定性测试即可钉住该机制：把夹具的无响应体 handler 挂到一个 HttpServer 上，断言 java.net.http.HttpClient 收到的响应携带 Connection: close——这正是本次评审探针验证过的形态（它随该行响应头的存亡精确翻转）。可选的更强形态是按 PR 描述中配对工具的形状，写一个 Java 11 专属的"无响应体响应 + 随后一个 mutation 请求"循环测试。
新增的断言测试本身必须在 sendNoContent 中的 Connection: close 一行被移除后变红——请用突变验证：删掉该行响应头，运行该测试，确认它变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code-review-bot approve this pr if no blockers |
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
4 participants
What this PR does
Every body-less response in the daemon client test fixture now ends its connection along with the response, through one shared helper. That helper absorbs the one-off "close the connection" variant added for a single test's detach handlers in #10365, and the ten body-less responses that were written out by hand elsewhere in the fixture now go through it too.
Why it's needed
The Java 11 lane of SDK Java fails intermittently, and always the same way: a test tears its session down, and the detach or destroy call raises
DetachOutcomeUnknownException/MutationOutcomeUnknownExceptionwrappingHTTP/1.1 header parser received no bytes. Over the last day alone it has hit three different test methods on three unrelated branches — the backpressure test on this week's web-shell branch, the destroy-after-detach test twice, and the heartbeat fairness test — a fourth reproduces locally, and #10365 patched a fifth two days ago. The same runs stay green on Java 17, Java 21, macOS and Windows.The cause is the fixture, not the SDK. On Java 11 the JDK's own HTTP server drops the TCP connection after it answers with no body at all, which is what a 204 detach, a 204 cancel, a 204 session delete and an empty event stream all do here. The Java 11 HttpClient keeps that same connection in its pool regardless, so the next request the SDK sends over it reads EOF before a single response byte arrives. Because that request is a non-idempotent mutation, the SDK deliberately does not retry it and reports the outcome as unknown — correct behaviour, reached for a reason that only exists inside the test fixture.
Pairing the two halves across JDKs confirms both of them have to be Java 11 for this to happen: a Java 11 server with a Java 21 client never fails, and neither does a Java 21 server with a Java 11 client. The daemon the SDK actually talks to is the Node serve process, so nothing outside these tests is exposed, and newer JDKs are unaffected in both roles.
Marking these responses non-persistent removes the race outright — the client is told the connection is finished, so it never pools a connection the fixture is about to drop. That is what #10365 did for two handlers; this change applies it wherever the fixture answers without a body, so the next body-less response added to the fixture cannot reintroduce the flake.
Reviewer Test Plan
How to verify
Run the daemon client tests on Java 11, repeatedly and pinned to a single core so the race has room to land:
taskset -c 2 mvn --batch-mode --no-transfer-progress clean testfrompackages/sdk-java/qwencode, or drive the three most affected test methods in a loop through the JUnit launcher. Before the change the teardown failures reproduce within a handful of rounds; after it they stop. A reviewer only wants to see the suite stay green and the loop stop failing — no behaviour of the SDK changes.Evidence (Before & After)
Java 11 (Temurin 11.0.32.1), same machine, each row run identically before and after:
One core is what makes this land quickly; on two cores the class-level loop only fails every dozen runs, which is roughly the rate CI shows.
Failure signature before the change, identical to CI:
DetachOutcomeUnknownException: POST /session/:id/detach may have reached the daemon; the SDK did not retry it, caused byHTTP/1.1 header parser received no bytes.Cross-JDK pairing of a body-less response followed by a POST, 150 iterations each: Java 11 server + Java 11 client — 48 failures; Java 11 server + Java 21 client — 0; Java 21 server + Java 11 client — 0; Java 21 both — 0.
Tested on
Environment (optional)
Temurin 11.0.32.1 and Maven 3.9, unit tests only.
Risk & Scope
Linked Issues
Follow-up to #10365, which fixed the same failure for two handlers of one test.
中文说明
这个 PR 做了什么
daemon 客户端测试夹具里所有"无响应体"的返回，现在都通过同一个辅助方法在响应的同时结束连接。该辅助方法吸收了 #10365 为某个测试的两个 detach handler 单独加的"关闭连接"变体，夹具里另外十处手写的无响应体返回也一并改为走它。
为什么需要
SDK Java 的 Java 11 lane 会间歇性失败，而且失败形态始终一样：测试在拆除 session 时，detach 或 destroy 调用抛出
DetachOutcomeUnknownException/MutationOutcomeUnknownException，内层是HTTP/1.1 header parser received no bytes。仅最近一天，它就在三个互不相关的分支上命中了三个不同的测试方法——backpressure 测试、destroy-after-detach 测试（两次）、heartbeat 公平性测试；第四个在本地可以复现，而 #10365 两天前刚修过第五个。同一次 run 里 Java 17、Java 21、macOS 和 Windows 都是绿的。根因在夹具，不在 SDK。Java 11 上 JDK 自带的 HTTP server 在回复一个完全没有响应体的响应之后会断掉 TCP 连接，而这里的 204 detach、204 cancel、204 session delete 和空事件流全都是这种响应。Java 11 的 HttpClient 却仍然把这条连接留在连接池里，于是 SDK 下一个复用它的请求还没读到任何响应字节就先读到 EOF。由于该请求是非幂等的 mutation，SDK 按设计不重试，只报告结果未知——行为本身是对的，只是触发它的原因只存在于测试夹具中。
把 server 和 client 两端跨 JDK 组合可以确认必须两端都是 Java 11 才会复现：Java 11 server 配 Java 21 client 不会失败，Java 21 server 配 Java 11 client 同样不会。SDK 真正对接的 daemon 是 Node 的 serve 进程，所以这些测试之外不受影响，更高版本的 JDK 在两个角色上都不受影响。
把这些响应标记为非持久连接可以直接消除竞态——客户端被告知连接已结束，就不会再去复用一条夹具马上要断掉的连接。#10365 对两个 handler 做的正是这件事；本次改动把它推广到夹具中所有无响应体的返回，这样以后新增的无响应体返回也不会把这个 flake 带回来。
评审验证计划
如何验证
在 Java 11 上反复跑 daemon 客户端测试，并绑定到单核以便竞态有机会出现：在
packages/sdk-java/qwencode下执行taskset -c 2 mvn --batch-mode --no-transfer-progress clean test，或者用 JUnit launcher 循环驱动受影响最多的那三个测试方法。改动前，拆除阶段的失败几轮之内就会复现；改动后不再出现。评审只需确认测试套件保持绿色、循环不再失败——SDK 的行为没有任何变化。证据（改动前后）
Java 11（Temurin 11.0.32.1），同一台机器，每一行在改动前后用完全相同的方式运行：
绑定到单核才能让问题快速复现；双核下类级别的循环大约每十几次才失败一次，与 CI 上观察到的频率接近。
改动前的失败签名与 CI 完全一致：
DetachOutcomeUnknownException: POST /session/:id/detach may have reached the daemon; the SDK did not retry it，起因是HTTP/1.1 header parser received no bytes。"无响应体响应 + 随后一个 POST"的跨 JDK 组合各跑 150 次：Java 11 server + Java 11 client——48 次失败；Java 11 server + Java 21 client——0；Java 21 server + Java 11 client——0；两端 Java 21——0。
测试平台
环境（可选）
Temurin 11.0.32.1 与 Maven 3.9，仅单元测试。
风险与范围
关联 Issue
是 #10365 的后续，该 PR 为某个测试的两个 handler 修复了同一个问题。