René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"HttpServletStreamableServerTransportProvider orphans sessions on first transient SSE write failure (no grace period)","articleBody":"## Summary\n\nIn `HttpServletStreamableServerTransportProvider`, a single failed SSE write\nimmediately removes the session from the in-memory map, so the client's next\nPOST gets `Session not found` even though the failure was transient (LB\nresponse-timeout, NEG rebalance, pod eviction, laptop sleep, mobile network\nblip). The client is forced into a full `initialize` round-trip and loses\nany server-side session state.\n\nAdding a short, configurable **grace period** before `sessions.remove(...)`\n— during which a reconnect with the same session id reattaches — would solve\nthis without touching where sessions are stored or how resumability works.\n\nA secondary, unrelated observation about a duplicate `asyncContext.complete()`\ncall is noted at the bottom.\n\n## Relationship to existing work\n\nI want to flag upfront how this differs from related open issues/PRs, so it's\neasy to triage.\n\n- **#914** (`McpSessionStore` SPI) — changes *where* sessions live\n  (pluggable store: Redis, JDBC, Hazelcast). Great for restart/cluster\n  scenarios. Orthogonal to this issue: even with a Redis-backed store, one\n  failed write to one client still triggers `sessions.remove(sessionId)`\n  and orphans **that** client's session.\n- **#830** (`Last-Event-ID` resumability) — adds an event store and wires\n  replay through `GET /mcp`. Complementary to this issue but requires the\n  session to still exist when the client reconnects. With today's eager\n  removal, replay is unreachable for the case described here.\n- **#107** (server-restart `Session not found`) — same symptom, different\n  trigger (process restart vs in-process transient write failure). #914\n  addresses #107; neither addresses the scenario below.\n- **#274 / #738 / #376 / #201** — persistence / clustering angles, all in\n  the same family as #914.\n\nThe ask here is narrower than any of those: **don't drop the session on the\nfirst transient write failure**.\n\n## Version\n\n- `io.modelcontextprotocol.sdk:mcp-core` 1.1.0\n- JDK 25, Tomcat (Spring Boot 4), behind a GKE HTTP(S) LB\n\n## Reproduction\n\n1. Run any gateway built on `HttpServletStreamableServerTransportProvider`\n   behind an L7 LB that caps per-response duration below the session lifetime\n   (GKE `BackendConfig.timeoutSec` ≤ 60 s is a clean repro).\n2. Connect an MCP client (seen with `claude-code`, but any streamable-HTTP\n   client triggers it).\n3. Wait for the LB to close the SSE stream (~60 s in our case).\n4. Observe — server log:\n\n   ```\n   KeepAliveScheduler: Failed to send keep-alive ping to session ...:\n     Did not observe any item or terminal signal within 10000ms in 'source(MonoCreate)'\n   ServletStreamableServerTransportProvider:\n     Failed to send message to session ...: Client disconnected\n   ```\n\n   Client gets `Session not found` on its next POST.\n\n## Root cause\n\n`HttpServletStreamableMcpSessionTransport.sendMessage` (v1.1.0, lines 738–767)\nhard-codes session removal in the catch block:\n\n```java\n@Override\npublic Mono\u003cVoid\u003e sendMessage(McpSchema.JSONRPCMessage message, String messageId) {\n    return Mono.fromRunnable(() -\u003e {\n        ...\n        try {\n            ...\n            String jsonText = jsonMapper.writeValueAsString(message);\n            HttpServletStreamableServerTransportProvider.this.sendEvent(writer, MESSAGE_EVENT_TYPE, jsonText,\n                    messageId != null ? messageId : this.sessionId);\n            ...\n        }\n        catch (Exception e) {\n            logger.error(\"Failed to send message to session {}: {}\", this.sessionId, e.getMessage());\n            HttpServletStreamableServerTransportProvider.this.sessions.remove(this.sessionId); // \u003c— here\n            this.asyncContext.complete();\n        }\n        ...\n    });\n}\n```\n\nNo grace, no policy hook, no listener. `sessions` is a `private final Map`\nand `HttpServletStreamableMcpSessionTransport` is a private inner class, so\ndownstream apps cannot override the behaviour without reflection.\n\n## Proposal\n\nTwo shapes, from least to most invasive:\n\n### 1. Configurable session-retention grace period (minimal, recommended)\n\nAdd `Builder.sessionReconnectGracePeriod(Duration)`, defaulting to\n`Duration.ZERO` (current behaviour, fully backward compatible). On write\nfailure:\n\n- mark the session as detached (new flag on the session transport);\n- schedule a removal task at `now + grace` on a shared scheduled executor;\n- on an incoming `GET /mcp` whose session id matches, clear the detach flag,\n  cancel the scheduled removal, and let the existing `Last-Event-ID` replay\n  path (SDK lines 327–349) or #830's event-store path run.\n\nWith `grace = Duration.ZERO`, behaviour is identical to today. With\n`grace \u003e 0`, transient blips no longer orphan clients. Composes naturally\nwith #914 (pluggable store) and #830 (replay).\n\n### 2. Session lifecycle listener (larger, general-purpose)\n\nAdd a `SessionLifecycleListener` interface on the builder with\n`onSessionDetached`, `onSessionReconnected`, `onSessionClosed`. Ship the\ncurrent eager-remove behaviour as the default listener; apps can register\na custom listener with whatever retention policy suits their deployment.\n\nI'd prefer option 1 — it solves the concrete problem without opening a\nbroader API-surface question.\n\n## Secondary observation — duplicate `asyncContext.complete()`\n\nSame file: the catch block in `sendMessage` (line 761) and `close()`\n(line ~811) both call `asyncContext.complete()`. When a write failure is\nfollowed by a close, the second call races with the servlet container's\nstate machine and produces:\n\n```\nFailed to complete async context ... Async state [COMPLETING]\n```\n\nCosmetic, but pollutes logs. A flag (e.g. `asyncContextCompleted` set on\nfirst call, checked before the second) would silence it.\n\n## Willing to contribute\n\nHappy to open a PR for either of the above if the maintainers would welcome\nthe contribution — just want to confirm the approach and that option 1 is\nthe direction you'd prefer before writing code.","author":{"url":"https://github.com/andersenleo","@type":"Person","name":"andersenleo"},"datePublished":"2026-04-15T07:51:32.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/920/java-sdk/issues/920"}