[Schema][Server] Preserve request id in JSON-RPC error responses instead of fabricating id:""

CHANGELOG.md

@@ -31,6 +31,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
 * [BC Break] `ResourceDefinition::__construct()` signature changed — `$title` parameter added between `$name` and `$description`. Callers using positional arguments must switch to named arguments.
 * [BC Break] `ResourceTemplate::__construct()` signature changed — `$title` parameter added between `$name` and `$description`. Callers using positional arguments must switch to named arguments.
 * [BC Break] `McpResource` and `McpResourceTemplate` attribute signatures changed — `$title` parameter added between `$name` and `$description`. Callers using positional arguments must switch to named arguments.
+* Fix JSON-RPC error responses fabricating an empty-string `id`. Unrecoverable parse errors (`-32700`) now return `id: null` per JSON-RPC 2.0, and invalid-but-parseable requests (`-32600`) preserve the original request `id`. `Error::$id` / `Error::getId()` and the `$id` parameter of `Error::forParseError()` / `Error::forInvalidRequest()` are widened to `string|int|null`; `InvalidInputMessageException` now carries the recoverable request id via `getRequestId()`/`setRequestId()`.
 
 0.5.0
 -----

src/Client/Protocol.php

@@ -240,6 +240,17 @@ private function handleResponse(Response|Error $response): void
     {
         $requestId = $response->getId();
 
+        if (null === $requestId) {
+            // Only an Error may carry id: null (the server could not determine the
+            // request id, e.g. it failed to parse our request). Pending requests are
+            // keyed by non-null ids, so there is nothing to correlate it with.
+            $this->logger->warning('Received error response with null id, unable to correlate it to a pending request.', [
+                'response' => $response->jsonSerialize(),
+            ]);
+
+            return;
+        }
+
         $this->logger->debug('Handling response', ['id' => $requestId]);
 
         $this->state->storeResponse($requestId, $response->jsonSerialize());

src/Exception/InvalidInputMessageException.php

@@ -16,4 +16,17 @@
  */
 class InvalidInputMessageException extends \InvalidArgumentException implements ExceptionInterface
 {
+    private string|int|null $requestId = null;
+
+    public function getRequestId(): string|int|null
+    {
+        return $this->requestId;
+    }
+
+    public function setRequestId(string|int|null $requestId): self
+    {
+        $this->requestId = $requestId;
+
+        return $this;
+    }
 }

src/JsonRpc/MessageFactory.php

@@ -111,6 +111,11 @@ public function create(string $input): array
             try {
                 $messages[] = $this->createMessage($message);
             } catch (InvalidInputMessageException $e) {
+                // Recover the id only when it's a valid JSON-RPC scalar;
+                // a null or malformed id is left at the exception's null default.
+                if (\is_array($message) && isset($message['id']) && (\is_string($message['id']) || \is_int($message['id']))) {
+                    $e->setRequestId($message['id']);
+                }
                 $messages[] = $e;
             }
         }

src/Schema/JsonRpc/Error.php

@@ -18,7 +18,7 @@
  *
  * @phpstan-type ErrorData array{
  *     jsonrpc: string,
- *     id: string|int,
+ *     id: string|int|null,
  *     code: int,
  *     message: string,
  *     data?: mixed,
@@ -42,7 +42,7 @@ class Error implements MessageInterface
      * @param mixed|null $data    additional information about the error
      */
     public function __construct(
-        public readonly string|int $id,
+        public readonly string|int|null $id,
         public readonly int $code,
         public readonly string $message,
         public readonly mixed $data = null,
@@ -57,10 +57,13 @@ final public static function fromArray(array $data): self
         if (!isset($data['jsonrpc']) || MessageInterface::JSONRPC_VERSION !== $data['jsonrpc']) {
             throw new InvalidArgumentException('Invalid or missing "jsonrpc" in Error data.');
         }
-        if (!isset($data['id'])) {
+        // JSON-RPC requires the "id" key on a response, but its value may be null
+        // (parse error / invalid request where the id could not be determined), so the
+        // key must be present yet allowed to hold null alongside string|int.
+        if (!\array_key_exists('id', $data)) {
             throw new InvalidArgumentException('Invalid or missing "id" in Error data.');
         }
-        if (!\is_string($data['id']) && !\is_int($data['id'])) {
+        if (null !== $data['id'] && !\is_string($data['id']) && !\is_int($data['id'])) {
             throw new InvalidArgumentException('Invalid "id" type in Error data.');
         }
         if (!isset($data['error']) || !\is_array($data['error'])) {
@@ -76,50 +79,50 @@ final public static function fromArray(array $data): self
         return new self($data['id'], $data['error']['code'], $data['error']['message'], $data['error']['data'] ?? null);
     }
 
-    final public static function forParseError(string $message, string|int $id = ''): self
+    final public static function forParseError(string $message, string|int|null $id = null): self
     {
         return new self($id, self::PARSE_ERROR, $message);
     }
 
-    final public static function forInvalidRequest(string $message, string|int $id = ''): self
+    final public static function forInvalidRequest(string $message, string|int|null $id = null): self
     {
         return new self($id, self::INVALID_REQUEST, $message);
     }
 
-    final public static function forMethodNotFound(string $message, string|int $id = ''): self
+    final public static function forMethodNotFound(string $message, string|int|null $id = null): self
     {
         return new self($id, self::METHOD_NOT_FOUND, $message);
     }
 
-    final public static function forInvalidParams(string $message, string|int $id = '', mixed $data = null): self
+    final public static function forInvalidParams(string $message, string|int|null $id = null, mixed $data = null): self
     {
         return new self($id, self::INVALID_PARAMS, $message, $data);
     }
 
-    final public static function forInternalError(string $message, string|int $id = ''): self
+    final public static function forInternalError(string $message, string|int|null $id = null): self
     {
         return new self($id, self::INTERNAL_ERROR, $message);
     }
 
-    final public static function forServerError(string $message, string|int $id = ''): self
+    final public static function forServerError(string $message, string|int|null $id = null): self
     {
         return new self($id, self::SERVER_ERROR, $message);
     }
 
-    final public static function forResourceNotFound(string $message, string|int $id = ''): self
+    final public static function forResourceNotFound(string $message, string|int|null $id = null): self
     {
         return new self($id, self::RESOURCE_NOT_FOUND, $message);
     }
 
-    public function getId(): string|int
+    public function getId(): string|int|null
     {
         return $this->id;
     }
 
     /**
      * @return array{
      *     jsonrpc: string,
-     *     id: string|int,
+     *     id: string|int|null,
      *     error: array{
      *         code: int,
      *         message: string,

src/Server/Protocol.php

@@ -151,7 +151,7 @@ private function handleInvalidMessage(TransportInterface $transport, InvalidInpu
     {
         $this->logger->warning('Failed to create message.', ['exception' => $exception]);
 
-        $error = Error::forInvalidRequest($exception->getMessage());
+        $error = Error::forInvalidRequest($exception->getMessage(), $exception->getRequestId());
         $this->sendResponse($transport, $error, $session);
     }
 

tests/Unit/Client/ProtocolTest.php

@@ -0,0 +1,63 @@
+<?php
+
+/*
+ * This file is part of the official PHP MCP SDK.
+ *
+ * A collaboration between Symfony and the PHP Foundation.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Mcp\Tests\Unit\Client;
+
+use Mcp\Client\Protocol;
+use Mcp\Schema\JsonRpc\Error;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\AbstractLogger;
+
+final class ProtocolTest extends TestCase
+{
+    public function testNullIdErrorResponseIsLoggedAndDropped(): void
+    {
+        $logger = $this->createSpyLogger();
+        $protocol = new Protocol(logger: $logger);
+
+        $protocol->processMessage('{"jsonrpc": "2.0", "id": null, "error": {"code": -32700, "message": "Parse error"}}');
+
+        $warnings = array_values(array_filter($logger->records, static fn (array $record): bool => 'warning' === $record['level']));
+
+        $this->assertCount(1, $warnings);
+        $this->assertNull($warnings[0]['context']['response']['id']);
+        $this->assertSame(Error::PARSE_ERROR, $warnings[0]['context']['response']['error']['code']);
+    }
+
+    public function testErrorResponseWithIdIsStoredForItsPendingRequest(): void
+    {
+        $protocol = new Protocol();
+
+        $protocol->processMessage('{"jsonrpc": "2.0", "id": 7, "error": {"code": -32601, "message": "Method not found"}}');
+
+        $response = $protocol->getState()->consumeResponse(7);
+
+        $this->assertInstanceOf(Error::class, $response);
+        $this->assertSame(7, $response->getId());
+        $this->assertSame(Error::METHOD_NOT_FOUND, $response->code);
+    }
+
+    /**
+     * @return AbstractLogger&object{records: list<array{level: string, message: string, context: array<string, mixed>}>}
+     */
+    private function createSpyLogger(): AbstractLogger
+    {
+        return new class extends AbstractLogger {
+            /** @var list<array{level: string, message: string, context: array<string, mixed>}> */
+            public array $records = [];
+
+            public function log($level, mixed $message, array $context = []): void
+            {
+                $this->records[] = ['level' => (string) $level, 'message' => (string) $message, 'context' => $context];
+            }
+        };
+    }
+}

tests/Unit/JsonRpc/MessageFactoryTest.php

@@ -346,9 +346,23 @@ public function testResponseWithInvalidIdType(): void
         $this->assertStringContainsString('id', $results[0]->getMessage());
     }
 
+    public function testErrorWithNullIdIsAccepted(): void
+    {
+        // JSON-RPC 2.0 mandates id: null on an error response when the request id
+        // could not be determined (parse error / invalid request), so an incoming
+        // error with a null id must deserialize, not be rejected.
+        $json = '{"jsonrpc": "2.0", "id": null, "error": {"code": -32700, "message": "Parse error"}}';
+
+        $results = $this->factory->create($json);
+
+        $this->assertCount(1, $results);
+        $this->assertInstanceOf(Error::class, $results[0]);
+        $this->assertNull($results[0]->getId());
+    }
+
     public function testErrorWithInvalidIdType(): void
     {
-        $json = '{"jsonrpc": "2.0", "id": null, "error": {"code": -32600, "message": "Invalid"}}';
+        $json = '{"jsonrpc": "2.0", "id": true, "error": {"code": -32600, "message": "Invalid"}}';
 
         $results = $this->factory->create($json);
 

tests/Unit/Server/ProtocolTest.php

@@ -27,6 +27,7 @@
 use Mcp\Server\Session\SessionInterface;
 use Mcp\Server\Session\SessionManagerInterface;
 use Mcp\Server\Transport\TransportInterface;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\TestDox;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -297,6 +298,41 @@ public function testInvalidJsonReturnsParseError(): void
         );
     }
 
+    #[TestDox('Unrecoverable parse error does not fabricate an empty-string id')]
+    public function testParseErrorDoesNotFabricateEmptyStringId(): void
+    {
+        $sentPayload = null;
+        $this->transport->expects($this->once())
+            ->method('send')
+            ->willReturnCallback(static function ($data) use (&$sentPayload) {
+                $sentPayload = $data;
+            });
+
+        $protocol = new Protocol(
+            requestHandlers: [],
+            notificationHandlers: [],
+            messageFactory: MessageFactory::make(),
+            sessionManager: $this->sessionManager,
+        );
+
+        // Well-formed JSON nested past PHP's json_decode() depth limit (512), mirroring
+        // issue #333: json_decode() throws "Maximum stack depth exceeded" so the request
+        // carries a real numeric id (900512) that cannot be recovered once decoding fails.
+        $deeplyNested = str_repeat('[', 600).str_repeat(']', 600);
+        $input = '{"jsonrpc":"2.0","id":900512,"method":"initialize","params":'.$deeplyNested.'}';
+
+        $protocol->processInput($this->transport, $input, null);
+
+        $this->assertNotNull($sentPayload);
+        $decoded = json_decode($sentPayload, true);
+        $this->assertSame(Error::PARSE_ERROR, $decoded['error']['code']);
+        // The original id is genuinely unrecoverable after a parse failure: per JSON-RPC 2.0
+        // the response must emit id: null, never a fabricated empty string and never a dropped
+        // key (clients that strictly validate the JSON-RPC shape rely on the key being present).
+        $this->assertArrayHasKey('id', $decoded);
+        $this->assertNull($decoded['id'], 'Unrecoverable parse error must emit id: null');
+    }
+
     #[TestDox('Invalid message structure returns error')]
     public function testInvalidMessageStructureReturnsError(): void
     {
@@ -349,6 +385,66 @@ public function testInvalidMessageStructureReturnsError(): void
         $this->assertEquals(Error::INVALID_REQUEST, $message['error']['code']);
     }
 
+    /**
+     * @return iterable<string, array{string, string|int}>
+     */
+    public static function recoverableIdProvider(): iterable
+    {
+        yield 'positive int' => ['{"jsonrpc": "2.0", "id": 42, "params": {}}', 42];
+        yield 'zero int (truthiness trap)' => ['{"jsonrpc": "2.0", "id": 0, "params": {}}', 0];
+        yield 'string id' => ['{"jsonrpc": "2.0", "id": "req-1", "params": {}}', 'req-1'];
+    }
+
+    #[DataProvider('recoverableIdProvider')]
+    #[TestDox('Invalid but parseable message preserves its recoverable id')]
+    public function testInvalidMessagePreservesRecoverableId(string $input, string|int $expectedId): void
+    {
+        $session = $this->createMock(SessionInterface::class);
+
+        $this->sessionManager->method('createWithId')->willReturn($session);
+        $this->sessionManager->method('exists')->willReturn(true);
+
+        // Configure session mock for queue operations (mirrors testInvalidMessageStructureReturnsError).
+        $queue = [];
+        $session->method('get')->willReturnCallback(static function ($key, $default = null) use (&$queue) {
+            if ('_mcp.outgoing_queue' === $key) {
+                return $queue;
+            }
+
+            return $default;
+        });
+
+        $session->method('set')->willReturnCallback(static function ($key, $value) use (&$queue) {
+            if ('_mcp.outgoing_queue' === $key) {
+                $queue = $value;
+            }
+        });
+
+        $protocol = new Protocol(
+            requestHandlers: [],
+            notificationHandlers: [],
+            messageFactory: MessageFactory::make(),
+            sessionManager: $this->sessionManager,
+        );
+
+        $sessionId = Uuid::v4();
+        // Valid JSON carrying a real id but missing method/result/error: the message is
+        // structurally invalid, yet its id IS recoverable from the decoded payload.
+        $protocol->processInput(
+            $this->transport,
+            $input,
+            $sessionId
+        );
+
+        $outgoing = $protocol->consumeOutgoingMessages($sessionId);
+        $this->assertCount(1, $outgoing);
+
+        $message = json_decode($outgoing[0]['message'], true);
+        $this->assertArrayHasKey('error', $message);
+        $this->assertEquals(Error::INVALID_REQUEST, $message['error']['code']);
+        $this->assertSame($expectedId, $message['id'], 'Invalid-but-parseable message must preserve its recoverable id, not return ""');
+    }
+
     #[TestDox('Request without handler returns method not found error')]
     public function testRequestWithoutHandlerReturnsMethodNotFoundError(): void
     {