Close WebMvcStreamableServerTransportProvider transports on disconnect - #6871
lxq19991111 wants to merge 2 commits into
Conversation
WebMvcStreamableServerTransportProvider transports on disconnect
2858e30 to
b10647a
Compare
b10647a to
e4325f5
Compare
Register SSE lifecycle cleanup so a client that disconnects without a clean DELETE no longer leaves the listening stream, replay stream, or streaming-response transport dangling. Route every lifecycle event through the same idempotent transport close() instead of a separate markClosed() flag, and route SSE write failures through the close path so a write failure closes only the current transport without removing the logical MCP session. Closes spring-projects#6860 Signed-off-by: lxq19991111 <15060002560@163.com>
e4325f5 to
9601a93
Compare
An SSE write failure only closed the current WebMVC session transport and then let the send complete normally, so callers kept writing to a stream that no longer leads anywhere. Surface the failure as a McpTransportException instead. The transport still closes only the current connection and the logical MCP session is preserved, so a client can still delete or reconnect with the same session id and Last-Event-ID after a transient write failure. This matches what modelcontextprotocol/java-sdk#1130 settled on upstream. Broadcasting is unaffected: notifyClients logs and swallows per-session failures, and the keep-alive scheduler completes ping failures, so propagating a single write failure cannot break other sessions. The write-failure test now asserts the propagated exception rather than verifying that the SseBuilder was completed on the mock. That assertion was a false positive: DefaultSseBuilder sets sendFailed on the first failed write, after which complete() returns immediately, so the mock verified a call that cannot happen on a real connection. Signed-off-by: lxq19991111 <15060002560@163.com>
|
Pushed a follow-up commit propagating SSE write failures: the transport still closes only the current connection and keeps the logical session, but Two notes on how this relates to modelcontextprotocol/java-sdk#1130 ("Streamable HTTP: Add session sweeping and close hanging streams"). It landed after the v2.0.1 tag, and it only changes The write-failure test now asserts the propagated exception and no longer verifies that the |
Fixes #6860
Makes the WebMVC Streamable HTTP SSE transport close when the SSE response completes, times out, or errors, and routes SSE write failures through the transport
close()path instead of only reporting the error to theSseBuilder.It mirrors modelcontextprotocol/java-sdk#1021: listening streams close on timeout/error as well as completion, replay and streaming responses close on lifecycle events, and every lifecycle event uses the same idempotent
close()(no separatemarkClosed()flag).Key design point: a write failure closes only the current HTTP/SSE transport and does NOT remove the logical MCP session. Removing the session on a request-specific write failure would break the next request carrying the same
mcp-session-idand Last-Event-ID replay, so session eviction is intentionally left to DELETE / keep-alive eviction.Testing
Rewrote both tests without reflection, using a package-private transport factory seam (the provider exposes a
RouterFunction, so the SDK-style servlet entry-point mocking is not applicable here):sseLifecycleCallbacksCloseStreamOnCompleteTimeoutAndErrorasserts all three lifecycle callbacks trigger the close callback.sseWriteFailureClosesOnlyCurrentTransportWithoutRemovingSessioninitializes a session over the router, triggers an SSE write failure, verifies the transport is completed, and then deletes the session with the same id, asserting 200 to prove the session was retained.mvn -pl mcp/transport/mcp-spring-webmvc -am \ -Dtest=WebMvcStreamableServerTransportProviderTests \ -Dsurefire.failIfNoSpecifiedTests=false testModule unit tests pass, and WebMcpStreamableAsyncServerTransportIT, WebMcpStreamableSyncServerTransportIT, and WebMvcStreamableIT pass.