Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
0.8.0
-----

* [BC Break] `Mcp\Schema\JsonRpc\Error` accepts `null` as its `$id`, and `getId()` may return it. An error response whose id could not be read now omits the member instead of sending `"id": ""` — which claimed the peer had issued a request with an empty-string id. All the `for*()` factories default to `null`, `fromArray()` accepts a missing or explicitly-null id, and `MessageFactory` decodes both as an id-less error rather than rejecting them.
* [BC Break] Drop the SDK-only name pattern on `ResourceDefinition`/`ResourceTemplate` `$name` — the spec allows any string (its own examples use `main.rs` and `Project Files`). URI/URI-template validation is unchanged.
* Add `ClientGateway::supportsExtension()`, `Client\Builder::enableExtension()`, and `ClientCapabilities::withExtensions()` so clients can negotiate and check protocol extensions (e.g. MCP Apps) the same way servers already do. [BC Break] `ServerExtensionInterface` is replaced by the side-agnostic `Mcp\Schema\Extension\ExtensionInterface`.
* Deprecate Roots, Sampling and Logging per SEP-2577 (protocol revision `2026-07-28`, earliest removal `2027-07-28`). They keep working but using them now triggers a deprecation notice — migrate to tool arguments/resource URIs, a direct LLM provider API, and stderr/OpenTelemetry respectively.
Expand Down
6 changes: 6 additions & 0 deletions src/Client/Protocol.php
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,12 @@ private function handleResponse(Response|Error $response): void
{
$requestId = $response->getId();

if (null === $requestId) {
$this->logger->warning('Received an id-less error response; cannot correlate it to a request.', ['response' => $response->jsonSerialize()]);

return;
}

$this->logger->debug('Handling response', ['id' => $requestId]);

$this->state->storeResponse($requestId, $response->jsonSerialize());
Expand Down
63 changes: 36 additions & 27 deletions src/Schema/JsonRpc/Error.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@ class Error implements MessageInterface
public const UNSUPPORTED_PROTOCOL_VERSION = -32022;

/**
* @param int $code the error type that occurred
* @param string $message a short description of the error
* @param mixed|null $data additional information about the error
* @param string|int|null $id The id of the request this answers. `null` only when it could not be
* read — a malformed body, or a notification that was refused — in which
* case the member is omitted rather than sent as an id nobody issued.
* @param int $code the error type that occurred
* @param string $message a short description of the error
* @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,
Expand All @@ -77,10 +80,9 @@ 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'])) {
throw new InvalidArgumentException('Invalid or missing "id" in Error data.');
}
if (!\is_string($data['id']) && !\is_int($data['id'])) {
// An error response carrying no id is well-formed: it is what a
// receiver sends when the id could not be read off the request.
if (isset($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'])) {
Expand All @@ -93,45 +95,45 @@ final public static function fromArray(array $data): self
throw new InvalidArgumentException('Invalid or missing "message" in Error data.');
}

return new self($data['id'], $data['error']['code'], $data['error']['message'], $data['error']['data'] ?? null);
return new self($data['id'] ?? null, $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);
}

final public static function forHeaderMismatch(string $message, string|int $id = ''): self
final public static function forHeaderMismatch(string $message, string|int|null $id = null): self
{
return new self($id, self::HEADER_MISMATCH, $message);
}
Expand All @@ -142,7 +144,7 @@ final public static function forHeaderMismatch(string $message, string|int $id =
final public static function forMissingRequiredClientCapability(
string $message,
ClientCapabilities $requiredCapabilities,
string|int $id = '',
string|int|null $id = null,
): self {
return new self($id, self::MISSING_REQUIRED_CLIENT_CAPABILITY, $message, [
'requiredCapabilities' => $requiredCapabilities,
Expand All @@ -159,28 +161,28 @@ final public static function forMissingRequiredClientCapability(
final public static function forUnsupportedProtocolVersion(
string $requested,
array $supported,
string|int $id = '',
string|int|null $id = null,
): self {
return new self($id, self::UNSUPPORTED_PROTOCOL_VERSION, 'Unsupported protocol version', [
'requested' => $requested,
'supported' => array_values(array_map(static fn (ProtocolVersion $v): string => $v->value, $supported)),
]);
}

public function getId(): string|int
public function getId(): string|int|null
{
return $this->id;
}
Comment on lines +172 to 175

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in e3ff482 — both Client and Server Protocol now guard the null-id case instead of passing it into storeResponse()/the session key.


/**
* @return array{
* jsonrpc: string,
* id: string|int,
* id?: string|int,
* error: array{
* code: int,
* message: string,
Comment on lines 179 to 183

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in e3ff482 — moved data back inside error in the phpdoc shape.

* data?: mixed,
* },
* data?: mixed,
* }
*/
public function jsonSerialize(): array
Expand All @@ -194,10 +196,17 @@ public function jsonSerialize(): array
$error['data'] = $this->data;
}

return [
'jsonrpc' => MessageInterface::JSONRPC_VERSION,
'id' => $this->id,
'error' => $error,
];
$data = ['jsonrpc' => MessageInterface::JSONRPC_VERSION];

// Omitted, not empty: `"id": ""` claims the sender issued a request
// with an empty-string id, which is a different statement from "the
// id could not be read".
if (null !== $this->id) {
$data['id'] = $this->id;
}

$data['error'] = $error;

return $data;
}
}
6 changes: 6 additions & 0 deletions src/Server/Protocol.php
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,12 @@ private function handleResponse(Response|Error $response, SessionInterface $sess

$messageId = $response->getId();

if (null === $messageId) {
$this->logger->warning('Received an id-less error response from client; cannot correlate it to a pending request.', ['response' => $response->jsonSerialize()]);

return;
}

$session->set(self::SESSION_RESPONSES.".{$messageId}", $response->jsonSerialize());
$session->forget(self::SESSION_ACTIVE_REQUEST_META);

Expand Down
13 changes: 13 additions & 0 deletions tests/Unit/Client/ProtocolTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@ public static function provideUnusableCounterOffers(): iterable
yield 'modern revision' => [ProtocolVersion::V2026_07_28->value];
}

#[TestDox('logs and ignores an id-less error response instead of crashing on it')]
public function testIgnoresIdLessErrorResponse(): void
{
$protocol = new Protocol(logger: $logger = new CollectingLogger());

$protocol->processMessage(json_encode([
'jsonrpc' => MessageInterface::JSONRPC_VERSION,
'error' => ['code' => -32700, 'message' => 'Parse error'],
], \JSON_THROW_ON_ERROR));

$this->assertCount(1, $logger->warnings);
}

private function createConfiguration(ProtocolVersion $protocolVersion): Configuration
{
return new Configuration(
Expand Down
20 changes: 17 additions & 3 deletions tests/Unit/JsonRpc/MessageFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,15 @@ public function testNotificationMethodUsedAsRequest(): void

public function testErrorMissingId(): void
{
// Well-formed: an error response leaves the member out when the id
// could not be read off the request it answers.
$json = '{"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid"}}';

$results = $this->factory->create($json);

$this->assertCount(1, $results);
$this->assertInstanceOf(InvalidInputMessageException::class, $results[0]);
$this->assertStringContainsString('id', $results[0]->getMessage());
$this->assertInstanceOf(Error::class, $results[0]);
$this->assertNull($results[0]->getId());
}

public function testErrorMissingCode(): void
Expand Down Expand Up @@ -365,12 +367,24 @@ public function testResponseWithInvalidIdType(): void
$this->assertStringContainsString('id', $results[0]->getMessage());
}

public function testErrorWithInvalidIdType(): void
public function testErrorWithNullId(): void
{
// JSON-RPC 2.0 spells the same thing as an explicit null.
$json = '{"jsonrpc": "2.0", "id": null, "error": {"code": -32600, "message": "Invalid"}}';

$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": {"not": "an id"}, "error": {"code": -32600, "message": "Invalid"}}';

$results = $this->factory->create($json);

$this->assertCount(1, $results);
$this->assertInstanceOf(InvalidInputMessageException::class, $results[0]);
$this->assertStringContainsString('id', $results[0]->getMessage());
Expand Down
24 changes: 24 additions & 0 deletions tests/Unit/Server/ProtocolTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,30 @@ public function testNotificationHandledByMultipleHandlers(): void
);
}

#[TestDox('An id-less error response from the client is logged and ignored, not stored under a collapsed session key')]
public function testIdLessErrorResponseIsIgnored(): void
{
$session = $this->createMock(SessionInterface::class);
$session->expects($this->never())->method('set');

$this->sessionManager->method('exists')->willReturn(true);
$this->sessionManager->method('createWithId')->willReturn($session);

$protocol = new Protocol(
requestHandlers: [],
notificationHandlers: [],
messageFactory: MessageFactory::make(),
sessionManager: $this->sessionManager,
);

$sessionId = Uuid::v4();
$protocol->processInput(
$this->transport,
'{"jsonrpc": "2.0", "error": {"code": -32700, "message": "Parse error"}}',
$sessionId
);
}

#[TestDox('A single request is handled only by the first matching handler')]
public function testRequestHandledByFirstMatchingHandler(): void
{
Expand Down