diff --git a/src/Capability/Tool/ToolCaller.php b/src/Capability/Tool/ToolCaller.php index 24bc3995..f03c4bb4 100644 --- a/src/Capability/Tool/ToolCaller.php +++ b/src/Capability/Tool/ToolCaller.php @@ -13,6 +13,7 @@ use Mcp\Capability\Registry\ReferenceHandlerInterface; use Mcp\Capability\Registry\ReferenceProviderInterface; +use Mcp\Exception\RegistryException; use Mcp\Exception\ToolCallException; use Mcp\Exception\ToolNotFoundException; use Mcp\Schema\Content\AudioContent; @@ -68,14 +69,20 @@ public function call(CallToolRequest $request): CallToolResult ]); return new CallToolResult($formattedResult); + } catch (RegistryException $e) { + throw new ToolCallException($request, $e); } catch (\Throwable $e) { + if ($e instanceof ToolCallException) { + throw $e; + } + $this->logger->error('Tool execution failed', [ 'name' => $toolName, 'exception' => $e->getMessage(), 'trace' => $e->getTraceAsString(), ]); - throw new ToolCallException($request, $e); + throw new ToolCallException($request, RegistryException::internalError('Error while executing tool', $e)); } } } diff --git a/src/Capability/ToolChain.php b/src/Capability/ToolChain.php index e500ff00..83286317 100644 --- a/src/Capability/ToolChain.php +++ b/src/Capability/ToolChain.php @@ -16,6 +16,7 @@ use Mcp\Capability\Tool\MetadataInterface; use Mcp\Capability\Tool\ToolCallerInterface; use Mcp\Exception\InvalidCursorException; +use Mcp\Exception\RegistryException; use Mcp\Exception\ToolCallException; use Mcp\Exception\ToolNotFoundException; use Mcp\Schema\Request\CallToolRequest; @@ -66,8 +67,10 @@ public function call(CallToolRequest $request): CallToolResult if ($item instanceof ToolCallerInterface && $request->name === $item->getName()) { try { return $item->call($request); + } catch (ToolCallException|ToolNotFoundException $e) { + throw $e; } catch (\Throwable $e) { - throw new ToolCallException($request, $e); + throw new ToolCallException($request, RegistryException::internalError('Error while executing tool', $e)); } } } diff --git a/src/Exception/ReferenceExecutionException.php b/src/Exception/ReferenceExecutionException.php new file mode 100644 index 00000000..525cc525 --- /dev/null +++ b/src/Exception/ReferenceExecutionException.php @@ -0,0 +1,25 @@ + $messages + */ + public function __construct( + public readonly array $messages, + ?\Throwable $previous = null, + ) { + parent::__construct(implode("\n", $this->messages), previous: $previous); + } +} diff --git a/src/Exception/RegistryException.php b/src/Exception/RegistryException.php index c483a01e..9a5cdf67 100644 --- a/src/Exception/RegistryException.php +++ b/src/Exception/RegistryException.php @@ -13,7 +13,7 @@ use Mcp\Schema\JsonRpc\Error; -final class RegistryException extends \Exception implements ExceptionInterface +class RegistryException extends \Exception implements ExceptionInterface { public static function invalidParams(string $message = 'Invalid params', ?\Throwable $previous = null): self { diff --git a/src/Exception/ToolCallException.php b/src/Exception/ToolCallException.php index 71978d9d..7151fd6d 100644 --- a/src/Exception/ToolCallException.php +++ b/src/Exception/ToolCallException.php @@ -20,8 +20,12 @@ final class ToolCallException extends \RuntimeException implements ExceptionInte { public function __construct( public readonly CallToolRequest $request, - ?\Throwable $previous = null, + public readonly RegistryException $registryException, ) { - parent::__construct(\sprintf('Tool call "%s" failed with error: "%s".', $request->name, $previous?->getMessage() ?? ''), previous: $previous); + $message = ($this->registryException->getPrevious() ?? $this->registryException)->getMessage(); + parent::__construct( + \sprintf('Tool call "%s" failed with error: "%s".', $request->name, $message), + previous: $this->registryException->getPrevious(), + ); } } diff --git a/src/Exception/ToolExecutionExceptionInterface.php b/src/Exception/ToolExecutionExceptionInterface.php new file mode 100644 index 00000000..7c75c13e --- /dev/null +++ b/src/Exception/ToolExecutionExceptionInterface.php @@ -0,0 +1,20 @@ + + */ + public function getErrorMessages(): array; +} diff --git a/src/JsonRpc/Handler.php b/src/JsonRpc/Handler.php index 8699eab2..57a0c32d 100644 --- a/src/JsonRpc/Handler.php +++ b/src/JsonRpc/Handler.php @@ -74,7 +74,7 @@ public static function make( new RequestHandler\GetPromptHandler($promptGetter), new RequestHandler\ListResourcesHandler($referenceProvider), new RequestHandler\ReadResourceHandler($resourceReader), - new RequestHandler\CallToolHandler($toolCaller, $logger), + new RequestHandler\CallToolHandler($toolCaller), new RequestHandler\ListToolsHandler($referenceProvider), ], logger: $logger, diff --git a/src/Server/RequestHandler/CallToolHandler.php b/src/Server/RequestHandler/CallToolHandler.php index 28aab382..b8b63be7 100644 --- a/src/Server/RequestHandler/CallToolHandler.php +++ b/src/Server/RequestHandler/CallToolHandler.php @@ -12,14 +12,16 @@ namespace Mcp\Server\RequestHandler; use Mcp\Capability\Tool\ToolCallerInterface; -use Mcp\Exception\ExceptionInterface; +use Mcp\Exception\ReferenceExecutionException; +use Mcp\Exception\ToolCallException; +use Mcp\Exception\ToolNotFoundException; +use Mcp\Schema\Content\TextContent; use Mcp\Schema\JsonRpc\Error; use Mcp\Schema\JsonRpc\HasMethodInterface; use Mcp\Schema\JsonRpc\Response; use Mcp\Schema\Request\CallToolRequest; +use Mcp\Schema\Result\CallToolResult; use Mcp\Server\MethodHandlerInterface; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; /** * @author Christopher Hertel @@ -29,7 +31,6 @@ final class CallToolHandler implements MethodHandlerInterface { public function __construct( private readonly ToolCallerInterface $toolCaller, - private readonly LoggerInterface $logger = new NullLogger(), ) { } @@ -44,16 +45,19 @@ public function handle(CallToolRequest|HasMethodInterface $message): Response|Er try { $content = $this->toolCaller->call($message); - } catch (ExceptionInterface $exception) { - $this->logger->error( - \sprintf('Error while executing tool "%s": "%s".', $message->name, $exception->getMessage()), - [ - 'tool' => $message->name, - 'arguments' => $message->arguments, - ], - ); - - return Error::forInternalError('Error while executing tool', $message->getId()); + } catch (ToolNotFoundException $exception) { + return Error::forInvalidParams($exception->getMessage(), $message->getId()); + } catch (ToolCallException $exception) { + $registryException = $exception->registryException; + + if ($registryException instanceof ReferenceExecutionException) { + return new Response($message->getId(), CallToolResult::error(array_map( + fn (string $message): TextContent => new TextContent($message), + $registryException->messages, + ))); + } + + return new Error($message->getId(), $registryException->getCode(), $registryException->getMessage()); } return new Response($message->getId(), $content); diff --git a/tests/Capability/Tool/ToolCallerTest.php b/tests/Capability/Tool/ToolCallerTest.php index 8894dc93..323acfa6 100644 --- a/tests/Capability/Tool/ToolCallerTest.php +++ b/tests/Capability/Tool/ToolCallerTest.php @@ -15,6 +15,7 @@ use Mcp\Capability\Registry\ReferenceProviderInterface; use Mcp\Capability\Registry\ToolReference; use Mcp\Capability\Tool\ToolCaller; +use Mcp\Exception\ReferenceExecutionException; use Mcp\Exception\ToolCallException; use Mcp\Exception\ToolNotFoundException; use Mcp\Schema\Content\TextContent; @@ -175,6 +176,31 @@ public function testCallThrowsToolNotFoundExceptionWhenToolNotFound(): void $this->toolCaller->call($request); } + public function testCallThrowsToolExecutionException(): void + { + $request = new CallToolRequest('test_tool', ['param' => 'value']); + $tool = $this->createValidTool('test_tool'); + $exception = new ReferenceExecutionException(['test error']); + $toolReference = new ToolReference($tool, fn () => throw $exception); + + $this->referenceProvider + ->expects($this->once()) + ->method('getTool') + ->with('test_tool') + ->willReturn($toolReference); + + $this->referenceHandler + ->expects($this->once()) + ->method('handle') + ->with($toolReference, ['param' => 'value']) + ->willThrowException($exception); + + $this->expectException(ToolCallException::class); + $this->expectExceptionMessage('Tool call "test_tool" failed with error: "test error".'); + + $this->toolCaller->call($request); + } + public function testCallThrowsToolExecutionExceptionWhenHandlerThrowsException(): void { $request = new CallToolRequest('failing_tool', ['param' => 'value']); diff --git a/tests/Server/RequestHandler/CallToolHandlerTest.php b/tests/Server/RequestHandler/CallToolHandlerTest.php index e8f13622..23133f4c 100644 --- a/tests/Server/RequestHandler/CallToolHandlerTest.php +++ b/tests/Server/RequestHandler/CallToolHandlerTest.php @@ -12,6 +12,7 @@ namespace Mcp\Tests\Server\RequestHandler; use Mcp\Capability\Tool\ToolCallerInterface; +use Mcp\Exception\RegistryException; use Mcp\Exception\ToolCallException; use Mcp\Exception\ToolNotFoundException; use Mcp\Schema\Content\TextContent; @@ -22,23 +23,17 @@ use Mcp\Server\RequestHandler\CallToolHandler; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; class CallToolHandlerTest extends TestCase { private CallToolHandler $handler; private ToolCallerInterface|MockObject $toolExecutor; - private LoggerInterface|MockObject $logger; protected function setUp(): void { $this->toolExecutor = $this->createMock(ToolCallerInterface::class); - $this->logger = $this->createMock(LoggerInterface::class); - $this->handler = new CallToolHandler( - $this->toolExecutor, - $this->logger, - ); + $this->handler = new CallToolHandler($this->toolExecutor); } public function testSupportsCallToolRequest(): void @@ -59,10 +54,6 @@ public function testHandleSuccessfulToolCall(): void ->with($request) ->willReturn($expectedResult); - $this->logger - ->expects($this->never()) - ->method('error'); - $response = $this->handler->handle($request); $this->assertInstanceOf(Response::class, $response); @@ -122,29 +113,18 @@ public function testHandleToolNotFoundExceptionReturnsError(): void ->with($request) ->willThrowException($exception); - $this->logger - ->expects($this->once()) - ->method('error') - ->with( - 'Error while executing tool "nonexistent_tool": "Tool not found for call: "nonexistent_tool".".', - [ - 'tool' => 'nonexistent_tool', - 'arguments' => ['param' => 'value'], - ], - ); - $response = $this->handler->handle($request); $this->assertInstanceOf(Error::class, $response); $this->assertEquals($request->getId(), $response->id); - $this->assertEquals(Error::INTERNAL_ERROR, $response->code); - $this->assertEquals('Error while executing tool', $response->message); + $this->assertEquals(Error::INVALID_PARAMS, $response->code); + $this->assertEquals('Tool not found for call: "nonexistent_tool".', $response->message); } public function testHandleToolExecutionExceptionReturnsError(): void { $request = $this->createCallToolRequest('failing_tool', ['param' => 'value']); - $exception = new ToolCallException($request, new \RuntimeException('Tool execution failed')); + $exception = new ToolCallException($request, RegistryException::internalError('Tool execution failed', previous: new \RuntimeException())); $this->toolExecutor ->expects($this->once()) @@ -152,23 +132,12 @@ public function testHandleToolExecutionExceptionReturnsError(): void ->with($request) ->willThrowException($exception); - $this->logger - ->expects($this->once()) - ->method('error') - ->with( - 'Error while executing tool "failing_tool": "Tool call "failing_tool" failed with error: "Tool execution failed".".', - [ - 'tool' => 'failing_tool', - 'arguments' => ['param' => 'value'], - ], - ); - $response = $this->handler->handle($request); $this->assertInstanceOf(Error::class, $response); $this->assertEquals($request->getId(), $response->id); $this->assertEquals(Error::INTERNAL_ERROR, $response->code); - $this->assertEquals('Error while executing tool', $response->message); + $this->assertEquals('Internal error: Tool execution failed (See server logs)', $response->message); } public function testHandleWithNullResult(): void @@ -216,24 +185,13 @@ public function testConstructorWithDefaultLogger(): void public function testHandleLogsErrorWithCorrectParameters(): void { $request = $this->createCallToolRequest('test_tool', ['key1' => 'value1', 'key2' => 42]); - $exception = new ToolCallException($request, new \RuntimeException('Custom error message')); + $exception = new ToolCallException($request, RegistryException::internalError(previous: new \RuntimeException('Custom error message'))); $this->toolExecutor ->expects($this->once()) ->method('call') ->willThrowException($exception); - $this->logger - ->expects($this->once()) - ->method('error') - ->with( - 'Error while executing tool "test_tool": "Tool call "test_tool" failed with error: "Custom error message".".', - [ - 'tool' => 'test_tool', - 'arguments' => ['key1' => 'value1', 'key2' => 42], - ], - ); - $this->handler->handle($request); }