Skip to content

test(sdk-java): close the connection on body-less fixture responses #10472

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.

Sign up for GitHub

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
yiliang114 merged 1 commit into from
Aug 29, 2026
Merged

test(sdk-java): close the connection on body-less fixture responses #10472

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 30 additions & 37 deletions ...sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/daemon/DaemonSessionClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,14 +261,10 @@ void slowSessionCreationDoesNotBlockExistingSessionMutation()
CountDownLatch cancelReceived = new CountDownLatch(1);
server.createContext("/session/session-1/cancel", exchange -> {
cancelReceived.countDown();
exchange.getResponseHeaders().set("Connection", "close");
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
});
server.createContext("/session/session-1/detach",
noContentAndCloseConnection());
server.createContext("/session/session-2/detach",
noContentAndCloseConnection());
server.createContext("/session/session-1/detach", noContent());
server.createContext("/session/session-2/detach", noContent());

try (DaemonClient daemon = newClient();
DaemonSessionClient first = daemon.createSession()) {
Expand Down Expand Up @@ -312,8 +308,7 @@ void clientCloseDetachesSessionCreatedByLosingRace() throws Exception {
detachedClient.set(exchange.getRequestHeaders()
.getFirst("X-Qwen-Client-Id"));
detachReceived.countDown();
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
});

DaemonClient daemon = newClient();
Expand Down Expand Up @@ -631,8 +626,7 @@ void retainsServerRetryDelayAcrossSseConnections() {
} else if (attempt == 2) {
exchange.getResponseHeaders().set("Content-Type",
"text/event-stream");
exchange.sendResponseHeaders(200, -1);
exchange.close();
sendNoContent(exchange, 200);
} else {
sendSse(exchange, terminalEvent(1));
}
Expand Down Expand Up @@ -1068,10 +1062,8 @@ void unexpectedSuccessfulSseStatusIsProtocolFailure() {
server.createContext("/session/session-1/prompt", exchange ->
sendJson(exchange, 202,
"{\"promptId\":\"prompt-1\",\"lastEventId\":0}"));
server.createContext("/session/session-1/events", exchange -> {
exchange.sendResponseHeaders(204, -1);
exchange.close();
});
server.createContext("/session/session-1/events", exchange ->
sendNoContent(exchange, 204));
server.createContext("/session/session-1/detach", noContent());

try (DaemonClient daemon = newClient();
Expand Down Expand Up @@ -1217,8 +1209,7 @@ void localObservationTimeoutDoesNotSendCancel() {
});
server.createContext("/session/session-1/cancel", exchange -> {
cancels.incrementAndGet();
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
});
server.createContext("/session/session-1/detach", noContent());

Expand Down Expand Up @@ -1629,8 +1620,7 @@ void closeAttemptsDetachOnlyOnceAndDestroyUsesDelete() {
server.createContext("/session/session-1", exchange -> {
if ("DELETE".equals(exchange.getRequestMethod())) {
deletes.incrementAndGet();
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
return;
}
sendJson(exchange, 404, "{}");
Expand Down Expand Up @@ -1908,8 +1898,7 @@ void closeWaitsForInFlightAdmissionBeforeDetaching() throws Exception {
sendSse(exchange, terminalEvent(1)));
server.createContext("/session/session-1/detach", exchange -> {
detaches.incrementAndGet();
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
});

try (DaemonClient daemon = newClient()) {
Expand Down Expand Up @@ -2277,8 +2266,7 @@ void clientCloseRejectsPromptFromSessionNotYetSwept() throws Exception {
sendSse(exchange, terminalEventForSession(1, sessionId)));
server.createContext("/session/" + sessionId + "/cancel", exchange -> {
mutationRequests.incrementAndGet();
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
});
server.createContext("/session/" + sessionId + "/heartbeat", exchange -> {
mutationRequests.incrementAndGet();
Expand All @@ -2295,8 +2283,7 @@ void clientCloseRejectsPromptFromSessionNotYetSwept() throws Exception {
detachingSession.compareAndSet(null, sessionId);
detachStarted.countDown();
await(releaseDetach);
exchange.sendResponseHeaders(204, -1);
exchange.close();
sendNoContent(exchange, 204);
});
}

Expand Down Expand Up @@ -2946,18 +2933,24 @@ private DaemonClient.Builder clientBuilder() {
}

private static HttpHandler noContent() {
return exchange -> {
exchange.sendResponseHeaders(204, -1);
exchange.close();
};
}

private static HttpHandler noContentAndCloseConnection() {
return exchange -> {
exchange.getResponseHeaders().set("Connection", "close");
exchange.sendResponseHeaders(204, -1);
exchange.close();
};
return exchange -> sendNoContent(exchange, 204);
}

/**
* Sends a body-less response and ends the connection with it. On Java 11
* the JDK's own HTTP server drops the connection after a response that
* carries no body, while the Java 11 HttpClient keeps that same connection
* in its pool; whichever request reuses it next reads EOF before any
* response byte and fails with "HTTP/1.1 header parser received no bytes".
* Marking these responses non-persistent keeps the client from pooling a
* connection the fixture is about to drop. Newer JDKs do this in neither
* role, and neither does the daemon the fixture stands in for.
*/
private static void sendNoContent(HttpExchange exchange, int status)
throws IOException {
exchange.getResponseHeaders().set("Connection", "close");

@qwen-code-ci-bot qwen-code-ci-bot Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

exchange.sendResponseHeaders(status, -1);
exchange.close();
}

private static void sendJson(HttpExchange exchange, int status, String body)
Expand Down
Loading