diff --git a/CHANGELOG.md b/CHANGELOG.md index 598fc739..b8e2be67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ All notable changes to `mcp/sdk` will be documented in this file. * [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. * Preserve the original request `id` on an invalid-but-parseable message (`-32600`) instead of answering it id-less: `InvalidInputMessageException` now carries the recoverable id via `getRequestId()`/`setRequestId()`, threaded from `MessageFactory` through to the error response. * [BC Break] Add the extensions framework SEP-2133 defines, which MCP Apps sits on. `ExtensionInterface::getId()` now returns the new `Mcp\Schema\Extension\ExtensionIdentifier` value object instead of a string, which validates the identifier against the `_meta` key naming rules at construction time. `ExtensionInterface` also gains `getMessages()`/`getRequestHandlers()`, so an extension can contribute the message classes its methods decode into — without which its methods cannot be decoded at all — and the handlers serving them; extensions that only announce a capability can extend the new `Mcp\Schema\Extension\AbstractExtension` and skip both. `MessageFactory::make()` takes an `$additional` list of message classes, and `RequestHandlerInterface`'s result template is now covariant. +* Log expected tool execution failures (`ToolCallException`) at debug level instead of error level; unexpected exceptions remain errors. * [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. diff --git a/src/Server/Handler/Request/CallToolHandler.php b/src/Server/Handler/Request/CallToolHandler.php index 616db3a3..72f3c21f 100644 --- a/src/Server/Handler/Request/CallToolHandler.php +++ b/src/Server/Handler/Request/CallToolHandler.php @@ -152,7 +152,7 @@ public function handle(Request $request, SessionInterface $session): Response|Er // needs to retry declaring the capability. Rendered as -32021. throw $e; } catch (ToolCallException $e) { - $this->logger->error(\sprintf('Error while executing tool "%s": "%s".', $toolName, $e->getMessage()), [ + $this->logger->debug(\sprintf('Error while executing tool "%s": "%s".', $toolName, $e->getMessage()), [ 'tool' => $toolName, 'arguments' => $arguments, 'exception' => $e, diff --git a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php index 47351dc0..308c9809 100644 --- a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php @@ -26,6 +26,7 @@ use Mcp\Server\Session\SessionInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; use Psr\Log\LoggerInterface; class CallToolHandlerTest extends TestCase @@ -207,16 +208,8 @@ public function testHandleToolCallExceptionReturnsResponseWithErrorResult(): voi ->willThrowException($exception); $this->logger - ->expects($this->once()) - ->method('error') - ->with( - 'Error while executing tool "failing_tool": "Tool execution failed".', - [ - 'tool' => 'failing_tool', - 'arguments' => ['param' => 'value', '_session' => $this->session, '_request' => $request], - 'exception' => $exception, - ], - ); + ->expects($this->atLeastOnce()) + ->method('debug'); $response = $this->handler->handle($request, $this->session); @@ -231,6 +224,61 @@ public function testHandleToolCallExceptionReturnsResponseWithErrorResult(): voi $this->assertEquals('Tool execution failed', $result->content[0]->text); } + public function testHandleToolCallExceptionLogsAtDebugLevel(): void + { + $request = $this->createCallToolRequest('failing_tool', ['param' => 'value']); + $exception = new ToolCallException('Expected tool failure'); + $logger = new class extends AbstractLogger { + public array $records = []; + + // @phpstan-ignore missingType.parameter (compatible with psr/log 1.x) + public function log($level, $message, array $context = []): void + { + $this->records[] = ['level' => $level, 'message' => (string) $message, 'context' => $context]; + } + }; + $handler = new CallToolHandler($this->registry, $this->referenceHandler, $logger); + $toolReference = $this->createToolReference('failing_tool', static function () { + return 'unused'; + }); + + $this->registry + ->expects($this->once()) + ->method('getTool') + ->with('failing_tool') + ->willReturn($toolReference); + + $arguments = ['param' => 'value', '_session' => $this->session, '_request' => $request]; + $this->referenceHandler + ->expects($this->once()) + ->method('handle') + ->with($toolReference, $arguments) + ->willThrowException($exception); + + $handler->handle($request, $this->session); + + $failureRecords = array_values(array_filter( + $logger->records, + static fn (array $record): bool => str_contains($record['message'], 'Expected tool failure'), + )); + + $this->assertSame([ + [ + 'level' => 'debug', + 'message' => 'Error while executing tool "failing_tool": "Expected tool failure".', + 'context' => [ + 'tool' => 'failing_tool', + 'arguments' => $arguments, + 'exception' => $exception, + ], + ], + ], $failureRecords); + $this->assertSame([], array_values(array_filter( + $logger->records, + static fn (array $record): bool => \in_array($record['level'], ['error', 'critical'], true), + ))); + } + public function testHandleWithNullResult(): void { $request = $this->createCallToolRequest('null_tool', []); @@ -270,7 +318,7 @@ public function testConstructorWithDefaultLogger(): void $this->assertInstanceOf(CallToolHandler::class, $handler); } - public function testHandleLogsErrorWithCorrectParameters(): void + public function testHandleLogsToolCallException(): void { $request = $this->createCallToolRequest('test_tool', ['key1' => 'value1', 'key2' => 42]); $exception = new ToolCallException('Custom error message'); @@ -291,16 +339,8 @@ public function testHandleLogsErrorWithCorrectParameters(): void ->willThrowException($exception); $this->logger - ->expects($this->once()) - ->method('error') - ->with( - 'Error while executing tool "test_tool": "Custom error message".', - [ - 'tool' => 'test_tool', - 'arguments' => ['key1' => 'value1', 'key2' => 42, '_session' => $this->session, '_request' => $request], - 'exception' => $exception, - ], - ); + ->expects($this->atLeastOnce()) + ->method('debug'); $response = $this->handler->handle($request, $this->session); @@ -336,6 +376,14 @@ public function testHandleGenericExceptionReturnsError(): void ->with($toolReference, ['param' => 'value', '_session' => $this->session, '_request' => $request]) ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Unhandled error during tool execution', [ + 'name' => 'failing_tool', + 'exception' => $exception, + ]); + $response = $this->handler->handle($request, $this->session); // Generic exceptions should return Error, not Response