diff --git a/CHANGELOG.md b/CHANGELOG.md index be4ac902..b75b6580 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * Fix a client request that timed out (elicitation, sampling) staying in the session's pending requests: `Protocol::checkResponse()` now expires it, answering with the `Request timed out` error, so the transports no longer implement the timeout themselves. * Fix a handshake-era Streamable HTTP stream pinning its PHP worker until the elicitation (or other client request) times out after the client disconnected: the idle stream now sends a keep-alive comment and stops once the connection is aborted. * [BC Break] Fix concurrent stdio tool calls that report progress, log, elicit or sample never finishing: `StdioTransport` keeps every suspended fiber and resumes each with the answer to its own request. A transport passes the suspended `\Fiber` as last argument to the pending-requests provider and the fiber yield handler. -* Add an opt-in per-session lock, `Builder::setSessionLock()`, serializing the concurrent requests of one session so none of them overwrites what another saved: pending server requests, client answers, client info. `FileSessionLock` locks with `flock()` next to `FileSessionStore`, `SymfonyLockSessionLock` uses a symfony/lock store for sessions shared across machines; `SessionLockInterface` takes custom ones. Off by default. A request that cannot get the lock in time is answered with `503` and a JSON-RPC error; a failing lock backend with `503` and `The session lock is unavailable.`. `SessionLockTimeoutException`, a `SessionLockException`, tells a timeout from a backend failure. `SymfonyLockSessionLock` releases its locks from a shutdown function after a fatal error in a handler. +* Add an opt-in per-session lock, `Builder::setSessionLock()`, serializing the concurrent requests of one session so none of them overwrites what another saved: pending server requests, client answers, client info. `FileSessionLock` locks with `flock()` next to `FileSessionStore`, `SymfonyLockSessionLock` uses a symfony/lock store for sessions shared across machines; `SessionLockInterface` takes custom ones. Off by default. A request that cannot get the lock in time is answered with `503` and a JSON-RPC error; a failing lock backend with `503` and `Session lock is unavailable.`, like a failing session store. `SessionLockTimeoutException`, a `SessionLockException`, tells a timeout from a backend failure. `SymfonyLockSessionLock` releases its locks from a shutdown function after a fatal error in a handler. * `Protocol::__construct()` takes an optional `SessionLockInterface $sessionLock` as its last argument. * `FileSessionStore::gc()` also removes the lock files of expired sessions a `FileSessionLock` in the same directory left behind, each only while no request holds it. * Resume a suspended handler with an error when the transport fails to store what it sends to the client next, instead of resuming it with `null`; `ClientGateway::notify()` then throws a `ClientException`. diff --git a/docs/run/sessions.md b/docs/run/sessions.md index 2b8a5327..0803af49 100644 --- a/docs/run/sessions.md +++ b/docs/run/sessions.md @@ -177,7 +177,7 @@ one after the other. A request that cannot get the lock within the timeout (30 seconds by default, the constructors' `$timeout`) is answered with HTTP `503` and a JSON-RPC error, instead of running on a session another request is about to overwrite. A lock backend that fails, e.g. a Redis that refuses connections, is answered with `503` too, -but at once, with the error message `The session lock is unavailable.`, and logged as an error with its cause; +but at once, with the error message `Session lock is unavailable.`, and logged as an error with its cause; a timeout is only logged as a warning. A handler that resumed after the client answered gets an error when the lock cannot be had to store its next request or notification: the `ClientGateway` method throws a `ClientException`, and the message does not reach the client. diff --git a/src/Server/Protocol.php b/src/Server/Protocol.php index 7c8a7be0..1d89caca 100644 --- a/src/Server/Protocol.php +++ b/src/Server/Protocol.php @@ -78,7 +78,6 @@ class Protocol /** Answer to a request that could not get its session's lock in time, see {@see SessionLockInterface}. */ private const SESSION_BUSY_MESSAGE = 'Another request of this session is still being processed.'; - private const SESSION_LOCK_UNAVAILABLE_MESSAGE = 'The session lock is unavailable.'; /** * Seconds past its timeout after which a pending client request counts as abandoned by its stream. @@ -257,13 +256,13 @@ private function doProcessInput(TransportInterface $transport, string $input, ?U if ($takesLock) { try { - $refusal = $this->acquireSessionLock($sessionId) ? null : self::SESSION_BUSY_MESSAGE; + $refusal = $this->acquireSessionLock($sessionId) ? null : Error::forServerError(self::SESSION_BUSY_MESSAGE, self::findResponseId($input)); } catch (SessionLockException) { - $refusal = self::SESSION_LOCK_UNAVAILABLE_MESSAGE; + $refusal = Error::forInternalError('Session lock is unavailable.', self::findResponseId($input)); } if (null !== $refusal) { - $this->sendResponse($transport, Error::forServerError($refusal, self::findResponseId($input)), null, ['status_code' => 503]); + $this->sendResponse($transport, $refusal, null, ['status_code' => 503]); return; } diff --git a/tests/Unit/Server/ProtocolSessionLockTest.php b/tests/Unit/Server/ProtocolSessionLockTest.php index 039fca4d..b3dbf8a3 100644 --- a/tests/Unit/Server/ProtocolSessionLockTest.php +++ b/tests/Unit/Server/ProtocolSessionLockTest.php @@ -197,8 +197,8 @@ public function testUnavailableLockIsRefusedAndLoggedWithItsCause(): void $error = json_decode($transport->sent[0]['message'], true); $this->assertSame(1, $error['id']); - $this->assertSame(Error::SERVER_ERROR, $error['error']['code']); - $this->assertSame('The session lock is unavailable.', $error['error']['message']); + $this->assertSame(Error::INTERNAL_ERROR, $error['error']['code']); + $this->assertSame('Session lock is unavailable.', $error['error']['message']); $errors = $logger->messagesAt(LogLevel::ERROR); $this->assertCount(1, $errors);