diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dd7ec41..4a7349a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix the client request timeout firing up to a second early and throwing a generic `RequestException`: it now throws `TimeoutException`, which extends `RequestException`, and sends `notifications/cancelled` for the abandoned request. * [BC Break] `TimeoutException` extends `RequestException` instead of `Exception`, so a `catch (RequestException)` now also catches timeouts. `ClientStateInterface::getPendingRequests()` reports `timestamp` as a `float` from `microtime(true)` instead of an `int`. * Fix the client's `HttpTransport` waiting out the request timeout when a non-streamed response to a request ends without its answer (truncated, empty, unreadable or not JSON): the call now fails at once with a `RequestException`, also when the body of an error status fails to read. +* Fix an unreachable `Psr16SessionStore` cache being answered as "session not found" (`404`): the failure is logged and answered with `503`, so clients retry instead of re-initializing. `SessionStoreInterface::exists()` may throw the new `SessionStoreException`. * Serve both protocol eras over stdio: `StdioTransport` settles the era on the client's first request and serves `2026-07-28` requests, `subscriptions/listen` and `notifications/cancelled` on the one channel. * [BC Break] `StatelessAwareTransportInterface` declares `setHandshakeVersions()`, so a server without the modern era names only the revisions it negotiates when refusing a `2026-07-28` request, e.g. the one set with `Builder::setProtocolVersion()`. * Answer a bare `initialize` on a `2026-07-28`-only endpoint with `-32022` naming the served revisions, and a request without a session on the handshake leg with its id. diff --git a/src/Exception/SessionStoreException.php b/src/Exception/SessionStoreException.php new file mode 100644 index 00000000..f5b0d92a --- /dev/null +++ b/src/Exception/SessionStoreException.php @@ -0,0 +1,21 @@ + + */ +class SessionStoreException extends Exception +{ +} diff --git a/src/Server/Protocol.php b/src/Server/Protocol.php index 3d5aaa16..2359b8ea 100644 --- a/src/Server/Protocol.php +++ b/src/Server/Protocol.php @@ -19,6 +19,7 @@ use Mcp\Event\ServerRequestEvent; use Mcp\Exception\InvalidInputMessageException; use Mcp\Exception\RuntimeException; +use Mcp\Exception\SessionStoreException; use Mcp\JsonRpc\MessageFactory; use Mcp\Schema\JsonRpc\Error; use Mcp\Schema\JsonRpc\Notification; @@ -830,19 +831,27 @@ private function resolveSession(TransportInterface $transport, ?Uuid $sessionId, if (!$sessionId) { // Echo the id so a client probing with `server/discover` can correlate the refusal. - $id = match (true) { - 1 !== \count($messages) => null, - $messages[0] instanceof Request => $messages[0]->getId(), - $messages[0] instanceof InvalidInputMessageException => $messages[0]->getRequestId(), - default => null, - }; - $error = Error::forInvalidRequest('A valid session id is REQUIRED for non-initialize requests.', $id); + $error = Error::forInvalidRequest('A valid session id is REQUIRED for non-initialize requests.', self::findRequestId($messages)); $this->sendResponse($transport, $error, null, ['status_code' => 400]); return null; } - if (!$this->sessionManager->exists($sessionId)) { + try { + $exists = $this->sessionManager->exists($sessionId); + } catch (SessionStoreException $e) { + // Not a 404: the session may well exist, so the client should retry instead of re-initializing. + $this->logger->error('Session store is unavailable.', [ + 'session_id' => $sessionId->toRfc4122(), + 'exception' => $e, + ]); + $error = Error::forInternalError('Session store is unavailable.', self::findRequestId($messages)); + $this->sendResponse($transport, $error, null, ['status_code' => 503]); + + return null; + } + + if (!$exists) { $error = Error::forInvalidRequest('Session not found or has expired.'); $this->sendResponse($transport, $error, null, ['status_code' => 404]); @@ -852,6 +861,21 @@ private function resolveSession(TransportInterface $transport, ?Uuid $sessionId, return $this->sessionManager->createWithId($sessionId); } + /** + * The id of a single request, so a refusal of the whole input can be correlated with it. + * + * @param array $messages + */ + private static function findRequestId(array $messages): string|int|null + { + return match (true) { + 1 !== \count($messages) => null, + $messages[0] instanceof Request => $messages[0]->getId(), + $messages[0] instanceof InvalidInputMessageException => $messages[0]->getRequestId(), + default => null, + }; + } + /** * Destroy a specific session. */ diff --git a/src/Server/Session/Psr16SessionStore.php b/src/Server/Session/Psr16SessionStore.php index 29ae8ac3..08ab0e3c 100644 --- a/src/Server/Session/Psr16SessionStore.php +++ b/src/Server/Session/Psr16SessionStore.php @@ -11,6 +11,7 @@ namespace Mcp\Server\Session; +use Mcp\Exception\SessionStoreException; use Psr\SimpleCache\CacheInterface; use Symfony\Component\Uid\Uuid; @@ -35,8 +36,8 @@ public function exists(Uuid $id): bool { try { return $this->cache->has($this->getKey($id)); - } catch (\Throwable) { - return false; + } catch (\Throwable $e) { + throw new SessionStoreException(\sprintf('Session store is unavailable: %s', $e->getMessage()), previous: $e); } } diff --git a/src/Server/Session/SessionStoreInterface.php b/src/Server/Session/SessionStoreInterface.php index 13f5f161..88fe6037 100644 --- a/src/Server/Session/SessionStoreInterface.php +++ b/src/Server/Session/SessionStoreInterface.php @@ -11,6 +11,7 @@ namespace Mcp\Server\Session; +use Mcp\Exception\SessionStoreException; use Symfony\Component\Uid\Uuid; /** @@ -24,6 +25,8 @@ interface SessionStoreInterface * @param Uuid $id the session id * * @return bool true if the session exists, false otherwise + * + * @throws SessionStoreException if the store cannot be reached */ public function exists(Uuid $id): bool; diff --git a/tests/Unit/Server/ProtocolTest.php b/tests/Unit/Server/ProtocolTest.php index 68444eab..5c230f1c 100644 --- a/tests/Unit/Server/ProtocolTest.php +++ b/tests/Unit/Server/ProtocolTest.php @@ -31,6 +31,7 @@ use Mcp\Server\Handler\Request\RequestHandlerInterface; use Mcp\Server\Protocol; use Mcp\Server\Session\InMemorySessionStore; +use Mcp\Server\Session\Psr16SessionStore; use Mcp\Server\Session\Session; use Mcp\Server\Session\SessionInterface; use Mcp\Server\Session\SessionManager; @@ -48,6 +49,7 @@ use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\AbstractLogger; use Psr\Log\LogLevel; +use Psr\SimpleCache\CacheInterface; use Symfony\Component\Uid\Uuid; final class ProtocolTest extends TestCase @@ -295,6 +297,45 @@ public function testNonExistentSessionIdReturnsError(): void ); } + #[TestDox('An unavailable session store answers 503 instead of 404 and is logged')] + public function testUnavailableSessionStoreReturnsServiceUnavailable(): void + { + $cache = $this->createMock(CacheInterface::class); + $cache->method('has')->willThrowException(new \RuntimeException('Connection refused')); + + $this->transport->expects($this->once()) + ->method('send') + ->with( + $this->callback(static function ($data) { + $decoded = json_decode($data, true); + + return Error::INTERNAL_ERROR === ($decoded['error']['code'] ?? null) + && 1 === ($decoded['id'] ?? null); + }), + $this->callback(static function ($context) { + return 503 === ($context['status_code'] ?? null); + }) + ); + + $logger = new LevelRecordingLogger(); + $protocol = new Protocol( + requestHandlers: [], + notificationHandlers: [], + messageFactory: MessageFactory::make(), + sessionManager: new SessionManager(new Psr16SessionStore($cache), gcProbability: 0), + logger: $logger, + ); + + $sessionId = Uuid::v4(); + $protocol->processInput( + $this->transport, + '{"jsonrpc": "2.0", "id": 1, "method": "tools/list"}', + $sessionId + ); + + $this->assertStringContainsString($sessionId->toRfc4122(), $logger->contextsAt([LogLevel::ERROR])); + } + #[TestDox('Invalid JSON returns parse error')] public function testInvalidJsonReturnsParseError(): void { diff --git a/tests/Unit/Server/Session/Psr16SessionStoreTest.php b/tests/Unit/Server/Session/Psr16SessionStoreTest.php new file mode 100644 index 00000000..f0a44ccb --- /dev/null +++ b/tests/Unit/Server/Session/Psr16SessionStoreTest.php @@ -0,0 +1,43 @@ +createMock(CacheInterface::class); + $cache->method('has')->willReturn(false); + + $this->assertFalse((new Psr16SessionStore($cache))->exists(Uuid::v4())); + } + + public function testExistsThrowsWhenTheCacheIsUnavailable(): void + { + $failure = new \RuntimeException('Connection refused'); + $cache = $this->createMock(CacheInterface::class); + $cache->method('has')->willThrowException($failure); + + try { + (new Psr16SessionStore($cache))->exists(Uuid::v4()); + $this->fail('Expected a SessionStoreException.'); + } catch (SessionStoreException $e) { + $this->assertSame($failure, $e->getPrevious()); + } + } +}