Skip to content

[Server] Answer an unreachable session store with 503 instead of 404 - #570

Merged
chr-hertel merged 2 commits into
modelcontextprotocol:mainfrom
chr-hertel:fix-session-store-outage
Oct 13, 2026
Merged

chr-hertel merged 2 commits into
modelcontextprotocol:mainfrom
chr-hertel:fix-session-store-outage

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

While the cache behind Psr16SessionStore is down, every request gets a 404 and nothing is logged - clients re-initialize instead of retrying.

  • Psr16SessionStore::exists() throws the new SessionStoreException instead of returning false on a cache failure
  • Protocol logs it and answers with an internal error and 503

Fixes chr-hertel#64

🤖 Generated with Claude Code

@chr-hertel chr-hertel added this to the 0.9.0 milestone Oct 10, 2026
@chr-hertel chr-hertel added bug Something isn't working Server Issues & PRs related to the Server component labels Oct 10, 2026
@chr-hertel
chr-hertel requested a balanced review from Copilot October 12, 2026 20:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The 503 JSON-RPC response loses known request IDs and responds to notification-only input.

1 open finding
What changed in this PR

Updates session-store outage handling so clients receive a logged 503 instead of an erroneous 404.

Changes:

  • Adds SessionStoreException and propagates cache failures.
  • Converts session-store failures into HTTP 503 responses.
  • Adds unit tests and changelog documentation.
File Description
CHANGELOG.md Documents the corrected outage behavior.
src/​Exception/​SessionStoreException.php Defines the new exception.
src/​Server/​Protocol.php Logs store failures and returns 503.
src/​Server/​Session/​Psr16SessionStore.php Throws on cache availability failures.
src/​Server/​Session/​SessionStoreInterface.php Documents the exception contract.
tests/​Unit/​Server/​ProtocolTest.php Tests 503 and logging behavior.
tests/​Unit/​Server/​Session/​Psr16SessionStoreTest.php Tests missing sessions and cache failures.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Server/Protocol.php Outdated
soyuka
soyuka previously approved these changes Oct 13, 2026
@chr-hertel
chr-hertel force-pushed the fix-session-store-outage branch from a147d27 to 106292b Compare October 13, 2026 21:31
@chr-hertel
chr-hertel merged commit d7698fc into modelcontextprotocol:main Oct 13, 2026
54 of 56 checks passed
@chr-hertel
chr-hertel deleted the fix-session-store-outage branch October 13, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Server] Session store errors are answered as "session not found"

3 participants