From a29c1265165d3c55b2cb8ba49513e51bf448c387 Mon Sep 17 00:00:00 2001 From: Kyrian Obikwelu Date: Sun, 21 Sep 2025 15:30:09 +0100 Subject: [PATCH 1/2] fix: ensure JSON-RPC error responses include message IDs --- src/JsonRpc/Handler.php | 38 ++++++++++++++++++++++++++------------ src/Server.php | 17 +++++------------ tests/ServerTest.php | 17 ++++++----------- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/JsonRpc/Handler.php b/src/JsonRpc/Handler.php index e7e66964..47869532 100644 --- a/src/JsonRpc/Handler.php +++ b/src/JsonRpc/Handler.php @@ -94,9 +94,6 @@ public static function make( /** * @return iterable}> - * - * @throws ExceptionInterface When a handler throws an exception during message processing - * @throws \JsonException When JSON encoding of the response fails */ public function process(string $input, ?Uuid $sessionId): iterable { @@ -161,9 +158,11 @@ public function process(string $input, ?Uuid $sessionId): iterable } foreach ($messages as $message) { + $messageId = method_exists($message, 'getId') ? $message->getId() : 0; + if ($message instanceof InvalidInputMessageException) { $this->logger->warning('Failed to create message.', ['exception' => $message]); - $error = Error::forInvalidRequest($message->getMessage(), 0); + $error = Error::forInvalidRequest($message->getMessage(), $messageId); yield [$this->encodeResponse($error), []]; continue; } @@ -183,17 +182,17 @@ public function process(string $input, ?Uuid $sessionId): iterable ['exception' => $e], ); - $error = Error::forMethodNotFound($e->getMessage()); + $error = Error::forMethodNotFound($e->getMessage(), $messageId); yield [$this->encodeResponse($error), []]; } catch (\InvalidArgumentException $e) { $this->logger->warning(\sprintf('Invalid argument: %s', $e->getMessage()), ['exception' => $e]); - $error = Error::forInvalidParams($e->getMessage()); + $error = Error::forInvalidParams($e->getMessage(), $messageId); yield [$this->encodeResponse($error), []]; } catch (\Throwable $e) { $this->logger->critical(\sprintf('Uncaught exception: %s', $e->getMessage()), ['exception' => $e]); - $error = Error::forInternalError($e->getMessage()); + $error = Error::forInternalError($e->getMessage(), $messageId); yield [$this->encodeResponse($error), []]; } } @@ -202,7 +201,7 @@ public function process(string $input, ?Uuid $sessionId): iterable } /** - * @throws \JsonException When JSON encoding fails + * Encodes a response to JSON, handling encoding errors gracefully. */ private function encodeResponse(Response|Error|null $response): ?string { @@ -214,11 +213,26 @@ private function encodeResponse(Response|Error|null $response): ?string $this->logger->info('Encoding response.', ['response' => $response]); - if ($response instanceof Response && [] === $response->result) { - return json_encode($response, \JSON_THROW_ON_ERROR | \JSON_FORCE_OBJECT); - } + try { + if ($response instanceof Response && [] === $response->result) { + return json_encode($response, \JSON_THROW_ON_ERROR | \JSON_FORCE_OBJECT); + } - return json_encode($response, \JSON_THROW_ON_ERROR); + return json_encode($response, \JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + $this->logger->error('Failed to encode response to JSON.', [ + 'message_id' => $response->getId(), + 'exception' => $e, + ]); + + $fallbackError = new Error( + id: $response->getId(), + code: Error::INTERNAL_ERROR, + message: 'Response could not be encoded to JSON' + ); + + return json_encode($fallbackError, \JSON_THROW_ON_ERROR); + } } /** diff --git a/src/Server.php b/src/Server.php index 55b84159..239a3132 100644 --- a/src/Server.php +++ b/src/Server.php @@ -44,19 +44,12 @@ public function connect(TransportInterface $transport): void ]); $transport->onMessage(function (string $message, ?Uuid $sessionId) use ($transport) { - try { - foreach ($this->jsonRpcHandler->process($message, $sessionId) as [$response, $context]) { - if (null === $response) { - continue; - } - - $transport->send($response, $context); + foreach ($this->jsonRpcHandler->process($message, $sessionId) as [$response, $context]) { + if (null === $response) { + continue; } - } catch (\JsonException $e) { - $this->logger->error('Failed to encode response to JSON.', [ - 'message' => $message, - 'exception' => $e, - ]); + + $transport->send($response, $context); } }); diff --git a/tests/ServerTest.php b/tests/ServerTest.php index c65c3ca4..046583d1 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -14,27 +14,19 @@ use Mcp\JsonRpc\Handler; use Mcp\Server; use Mcp\Server\Transport\InMemoryTransport; -use PHPUnit\Framework\MockObject\Stub\Exception; use PHPUnit\Framework\TestCase; -use Psr\Log\NullLogger; class ServerTest extends TestCase { public function testJsonExceptions() { - $logger = $this->getMockBuilder(NullLogger::class) - ->disableOriginalConstructor() - ->onlyMethods(['error']) - ->getMock(); - $logger->expects($this->once())->method('error'); - $handler = $this->getMockBuilder(Handler::class) ->disableOriginalConstructor() ->onlyMethods(['process']) ->getMock(); $handler->expects($this->exactly(2))->method('process')->willReturnOnConsecutiveCalls( - new Exception(new \JsonException('foobar')), + [['{"jsonrpc":"2.0","id":0,"error":{"code":-32700,"message":"Parse error"}}', []]], [['success', []]] ); @@ -42,9 +34,12 @@ public function testJsonExceptions() ->setConstructorArgs([['foo', 'bar']]) ->onlyMethods(['send']) ->getMock(); - $transport->expects($this->once())->method('send')->with('success', []); + $transport->expects($this->exactly(2))->method('send')->willReturnOnConsecutiveCalls( + null, + null + ); - $server = new Server($handler, $logger); + $server = new Server($handler); $server->connect($transport); $transport->listen(); From 9f1213bc69e3c709ffe09a4d8dc8af11500d1923 Mon Sep 17 00:00:00 2001 From: Kyrian Obikwelu Date: Sun, 21 Sep 2025 22:51:25 +0100 Subject: [PATCH 2/2] fix: replace method_exists with instanceof for message ID extraction --- src/JsonRpc/Handler.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/JsonRpc/Handler.php b/src/JsonRpc/Handler.php index 47869532..a16f7347 100644 --- a/src/JsonRpc/Handler.php +++ b/src/JsonRpc/Handler.php @@ -23,6 +23,7 @@ use Mcp\Schema\Implementation; use Mcp\Schema\JsonRpc\Error; use Mcp\Schema\JsonRpc\HasMethodInterface; +use Mcp\Schema\JsonRpc\Request; use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\Request\InitializeRequest; use Mcp\Server\MethodHandlerInterface; @@ -158,11 +159,9 @@ public function process(string $input, ?Uuid $sessionId): iterable } foreach ($messages as $message) { - $messageId = method_exists($message, 'getId') ? $message->getId() : 0; - if ($message instanceof InvalidInputMessageException) { $this->logger->warning('Failed to create message.', ['exception' => $message]); - $error = Error::forInvalidRequest($message->getMessage(), $messageId); + $error = Error::forInvalidRequest($message->getMessage()); yield [$this->encodeResponse($error), []]; continue; } @@ -171,6 +170,8 @@ public function process(string $input, ?Uuid $sessionId): iterable 'method' => $message->getMethod(), ]); + $messageId = $message instanceof Request ? $message->getId() : 0; + try { $response = $this->handle($message, $session); yield [$this->encodeResponse($response), ['session_id' => $session->getId()]];