Repository navigation
fix(agentserver): treat closed response stream as client disconnect - #2251
Conversation
A waited walk whose caller went away failed in sendResponseHeaders with an IOException that isClientDisconnect did not recognise, so it was logged at error level and the 500 retry threw again on the same exchange. Classify that message as a disconnect. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
Problem
Client telemetry group 5185 (1 event, 1 report, Microbot 2.6.56, client core, thread
AgentServer-1):Logged by
AgentHandlerasError handling {}at error level. An agent caller going away during a waited walk is a normal client disconnect, not an error. There is no functional breakage for the player, but it produces an error-level telemetry report and a secondIOExceptionthrown out ofhandle().Root cause
When a waited walk (
wait=true) has its wait interrupted,WalkHandler(lines 144-150) callssendJson(exchange, 500, ...)on an exchange that is already closed.exchange.sendResponseHeadersthen throwsIOException("stream is closed").AgentHandler.isClientDisconnectdid not recognise that JDK httpserver message, so:handle()fell through tolog.error("Error handling {}", ...).handle()then calledsendErrorResponse(exchange, 500, ...)on the same closed exchange, which threw the same unrecognisedIOExceptionagain and rethrew it out ofhandle().The disconnect classification was introduced in #1962; this message variant was not covered.
Change
One line in the shared helper
AgentHandler.isClientDisconnect:"stream is closed"is added to the recognisedIOExceptionmessages.handle()now logs at debug and returns without attempting an error response.sendErrorResponsealso swallows it.WalkHandleris untouched.Two things deliberately not done:
sendResponseHeadersis not wrapped inClientDisconnectException. That call also throwsIOExceptionfor non-disconnect cases (for example headers already sent), which would then be hidden as disconnects."stream closed"variant is not matched. Telemetry only shows"stream is closed"(the JDK httpserver message), and"Stream closed"is the genericjava.iomessage for unrelated closed file or input streams, which should still be logged as errors.Validation
./gradlew :client:runUnitTests --tests '*AgentHandlerClientDisconnectTest'-> BUILD SUCCESSFUL, 5/5 passed (2 new tests plus the 3 existing ones).New tests in
AgentHandlerClientDisconnectTest:handle()withsendResponseHeadersthrowingIOException("stream is closed")and asserts no exception escapes, no 500 retry and no body write;isClientDisconnectrecognises the message.Code review: approved (correct at root).
Known gaps
sendOpaqueNotFoundcalls inhandle()sit outside the try block, so a"stream is closed"failure there still propagates out ofhandle(). It never reacheslog.errorinAgentHandler, so it does not produce this telemetry group and is left unchanged."stream is closed"on an interrupted worker thread can also come from the agent server itself being stopped rather than the caller going away. The debug line "Client disconnected before ... response could be sent" is then slightly inaccurate; behaviour is correct either way because no response can be delivered.🤖 Generated with Claude Code