diff --git a/src/Capability/Discovery/DocBlockParser.php b/src/Capability/Discovery/DocBlockParser.php index 5c867e80..1000378e 100644 --- a/src/Capability/Discovery/DocBlockParser.php +++ b/src/Capability/Discovery/DocBlockParser.php @@ -48,6 +48,7 @@ public function parseDocBlock(string|false|null $docComment): ?DocBlock // Log error or handle gracefully if invalid DocBlock syntax is encountered $this->logger->warning('Failed to parse DocBlock', [ 'error' => $e->getMessage(), + 'exception' => $e, 'exception_trace' => $e->getTraceAsString(), ]); diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index c0432a02..478b98e4 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -87,6 +87,7 @@ public function validateAgainstJsonSchema(mixed $data, array|object $schema): ar $result = $validator->validate($dataToValidate, $schemaObject); } catch (\Throwable $e) { $this->logger->error('MCP SDK: JSON Schema validation failed internally.', [ + 'exception' => $e, 'exception_message' => $e->getMessage(), 'exception_trace' => $e->getTraceAsString(), 'data' => json_encode($dataToValidate), diff --git a/src/Client/Handler/Request/SamplingRequestHandler.php b/src/Client/Handler/Request/SamplingRequestHandler.php index cda0f11e..a00fa47c 100644 --- a/src/Client/Handler/Request/SamplingRequestHandler.php +++ b/src/Client/Handler/Request/SamplingRequestHandler.php @@ -55,7 +55,7 @@ public function handle(Request $request): Response|Error return new Response($request->getId(), $result); } catch (SamplingException $e) { - $this->logger->error('Sampling failed: '.$e->getMessage()); + $this->logger->error('Sampling failed: '.$e->getMessage(), ['exception' => $e]); return Error::forInternalError($e->getMessage(), $request->getId()); } catch (\Throwable $e) { diff --git a/src/Client/Transport/HttpTransport.php b/src/Client/Transport/HttpTransport.php index ca2ff445..a4d9de68 100644 --- a/src/Client/Transport/HttpTransport.php +++ b/src/Client/Transport/HttpTransport.php @@ -170,7 +170,7 @@ public function close(): void $this->httpClient->sendRequest($request); $this->logger->info('Session closed', ['session_id' => $this->sessionId]); } catch (\Throwable $e) { - $this->logger->warning('Failed to close session', ['error' => $e->getMessage()]); + $this->logger->warning('Failed to close session', ['error' => $e->getMessage(), 'exception' => $e]); } } diff --git a/src/Server/Handler/Request/CallToolHandler.php b/src/Server/Handler/Request/CallToolHandler.php index a6a6cd23..e78ce1b9 100644 --- a/src/Server/Handler/Request/CallToolHandler.php +++ b/src/Server/Handler/Request/CallToolHandler.php @@ -65,7 +65,7 @@ public function handle(Request $request, SessionInterface $session): Response|Er try { $reference = $this->registry->getTool($toolName); } catch (ToolNotFoundException $e) { - $this->logger->error('Tool not found', ['name' => $toolName]); + $this->logger->error('Tool not found', ['name' => $toolName, 'exception' => $e]); return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage()); } @@ -112,6 +112,7 @@ public function handle(Request $request, SessionInterface $session): Response|Er $this->logger->error(\sprintf('Error while executing tool "%s": "%s".', $toolName, $e->getMessage()), [ 'tool' => $toolName, 'arguments' => $arguments, + 'exception' => $e, ]); $errorContent = [new TextContent($e->getMessage())]; diff --git a/src/Server/Handler/Request/GetPromptHandler.php b/src/Server/Handler/Request/GetPromptHandler.php index eb968a12..745b9b86 100644 --- a/src/Server/Handler/Request/GetPromptHandler.php +++ b/src/Server/Handler/Request/GetPromptHandler.php @@ -65,15 +65,15 @@ public function handle(Request $request, SessionInterface $session): Response|Er return new Response($request->getId(), new GetPromptResult($formatted)); } catch (PromptGetException $e) { - $this->logger->error(\sprintf('Error while handling prompt "%s": "%s".', $promptName, $e->getMessage())); + $this->logger->error(\sprintf('Error while handling prompt "%s": "%s".', $promptName, $e->getMessage()), ['exception' => $e]); return Error::forInternalError($e->getMessage(), $request->getId()); } catch (PromptNotFoundException $e) { - $this->logger->error('Prompt not found', ['prompt_name' => $promptName]); + $this->logger->error('Prompt not found', ['prompt_name' => $promptName, 'exception' => $e]); return Error::forResourceNotFound($e->getMessage(), $request->getId()); } catch (\Throwable $e) { - $this->logger->error(\sprintf('Unexpected error while handling prompt "%s": "%s".', $promptName, $e->getMessage())); + $this->logger->error(\sprintf('Unexpected error while handling prompt "%s": "%s".', $promptName, $e->getMessage()), ['exception' => $e]); return Error::forInternalError('Error while handling prompt', $request->getId()); } diff --git a/src/Server/Handler/Request/ReadResourceHandler.php b/src/Server/Handler/Request/ReadResourceHandler.php index fc5bf132..a9551eff 100644 --- a/src/Server/Handler/Request/ReadResourceHandler.php +++ b/src/Server/Handler/Request/ReadResourceHandler.php @@ -77,15 +77,15 @@ public function handle(Request $request, SessionInterface $session): Response|Er return new Response($request->getId(), new ReadResourceResult($formatted)); } catch (ResourceReadException $e) { - $this->logger->error(\sprintf('Error while reading resource "%s": "%s".', $uri, $e->getMessage())); + $this->logger->error(\sprintf('Error while reading resource "%s": "%s".', $uri, $e->getMessage()), ['exception' => $e]); return Error::forInternalError($e->getMessage(), $request->getId()); } catch (ResourceNotFoundException $e) { - $this->logger->error('Resource not found', ['uri' => $uri]); + $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); return Error::forResourceNotFound($e->getMessage(), $request->getId()); } catch (\Throwable $e) { - $this->logger->error(\sprintf('Unexpected error while reading resource "%s": "%s".', $uri, $e->getMessage())); + $this->logger->error(\sprintf('Unexpected error while reading resource "%s": "%s".', $uri, $e->getMessage()), ['exception' => $e]); return Error::forInternalError('Error while reading resource', $request->getId()); } diff --git a/src/Server/Handler/Request/ResourceSubscribeHandler.php b/src/Server/Handler/Request/ResourceSubscribeHandler.php index ba0c8793..b5421f87 100644 --- a/src/Server/Handler/Request/ResourceSubscribeHandler.php +++ b/src/Server/Handler/Request/ResourceSubscribeHandler.php @@ -55,7 +55,7 @@ public function handle(Request $request, SessionInterface $session): Response|Er try { $this->registry->getResource($uri); } catch (ResourceNotFoundException $e) { - $this->logger->error('Resource not found', ['uri' => $uri]); + $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); return Error::forResourceNotFound($e->getMessage(), $request->getId()); } diff --git a/src/Server/Handler/Request/ResourceUnsubscribeHandler.php b/src/Server/Handler/Request/ResourceUnsubscribeHandler.php index 26db50e7..50ab8bc1 100644 --- a/src/Server/Handler/Request/ResourceUnsubscribeHandler.php +++ b/src/Server/Handler/Request/ResourceUnsubscribeHandler.php @@ -55,7 +55,7 @@ public function handle(Request $request, SessionInterface $session): Response|Er try { $this->registry->getResource($uri); } catch (ResourceNotFoundException $e) { - $this->logger->error('Resource not found', ['uri' => $uri]); + $this->logger->error('Resource not found', ['uri' => $uri, 'exception' => $e]); return Error::forResourceNotFound($e->getMessage(), $request->getId()); } diff --git a/tests/Conformance/client.php b/tests/Conformance/client.php index 5c2ee2c7..74ac8e90 100644 --- a/tests/Conformance/client.php +++ b/tests/Conformance/client.php @@ -96,7 +96,7 @@ public function handle(Request $request): Response $logger->info('Disconnected'); exit(0); } catch (Throwable $e) { - $logger->error(sprintf('Error: %s', $e->getMessage())); + $logger->error(sprintf('Error: %s', $e->getMessage()), ['exception' => $e]); fwrite(\STDERR, sprintf("Error: %s\n%s\n", $e->getMessage(), $e->getTraceAsString())); try { diff --git a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php index cad7af15..b804f76a 100644 --- a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php @@ -166,16 +166,18 @@ public function testHandleToolCallWithComplexArguments(): void public function testHandleToolNotFoundExceptionReturnsError(): void { $request = $this->createCallToolRequest('nonexistent_tool', ['param' => 'value']); + $exception = new ToolNotFoundException('nonexistent_tool'); $this->registry ->expects($this->once()) ->method('getTool') ->with('nonexistent_tool') - ->willThrowException(new ToolNotFoundException('nonexistent_tool')); + ->willThrowException($exception); $this->logger ->expects($this->once()) - ->method('error'); + ->method('error') + ->with('Tool not found', ['name' => 'nonexistent_tool', 'exception' => $exception]); $response = $this->handler->handle($request, $this->session); @@ -206,7 +208,15 @@ public function testHandleToolCallExceptionReturnsResponseWithErrorResult(): voi $this->logger ->expects($this->once()) - ->method('error'); + ->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, + ], + ); $response = $this->handler->handle($request, $this->session); @@ -288,6 +298,7 @@ public function testHandleLogsErrorWithCorrectParameters(): void [ 'tool' => 'test_tool', 'arguments' => ['key1' => 'value1', 'key2' => 42, '_session' => $this->session, '_request' => $request], + 'exception' => $exception, ], ); diff --git a/tests/Unit/Server/Handler/Request/GetPromptHandlerTest.php b/tests/Unit/Server/Handler/Request/GetPromptHandlerTest.php index 75503e39..204f9280 100644 --- a/tests/Unit/Server/Handler/Request/GetPromptHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/GetPromptHandlerTest.php @@ -27,6 +27,7 @@ use Mcp\Server\Session\SessionInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class GetPromptHandlerTest extends TestCase { @@ -34,14 +35,16 @@ class GetPromptHandlerTest extends TestCase private RegistryInterface&MockObject $referenceProvider; private ReferenceHandlerInterface&MockObject $referenceHandler; private SessionInterface&MockObject $session; + private LoggerInterface&MockObject $logger; protected function setUp(): void { $this->referenceProvider = $this->createMock(RegistryInterface::class); $this->referenceHandler = $this->createMock(ReferenceHandlerInterface::class); $this->session = $this->createMock(SessionInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); - $this->handler = new GetPromptHandler($this->referenceProvider, $this->referenceHandler); + $this->handler = new GetPromptHandler($this->referenceProvider, $this->referenceHandler, $this->logger); } public function testSupportsGetPromptRequest(): void @@ -239,6 +242,11 @@ public function testHandlePromptNotFoundExceptionReturnsError(): void ->with('nonexistent_prompt') ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Prompt not found', ['prompt_name' => 'nonexistent_prompt', 'exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response); @@ -258,6 +266,11 @@ public function testHandlePromptGetExceptionReturnsError(): void ->with('failing_prompt') ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Error while handling prompt "failing_prompt": "Failed to get prompt".', ['exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response); diff --git a/tests/Unit/Server/Handler/Request/ReadResourceHandlerTest.php b/tests/Unit/Server/Handler/Request/ReadResourceHandlerTest.php index 7483f1c1..66a22961 100644 --- a/tests/Unit/Server/Handler/Request/ReadResourceHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/ReadResourceHandlerTest.php @@ -27,6 +27,7 @@ use Mcp\Server\Session\SessionInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class ReadResourceHandlerTest extends TestCase { @@ -34,14 +35,16 @@ class ReadResourceHandlerTest extends TestCase private RegistryInterface&MockObject $registry; private ReferenceHandlerInterface&MockObject $referenceHandler; private SessionInterface&MockObject $session; + private LoggerInterface&MockObject $logger; protected function setUp(): void { $this->registry = $this->createMock(RegistryInterface::class); $this->referenceHandler = $this->createMock(ReferenceHandlerInterface::class); $this->session = $this->createMock(SessionInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); - $this->handler = new ReadResourceHandler($this->registry, $this->referenceHandler); + $this->handler = new ReadResourceHandler($this->registry, $this->referenceHandler, $this->logger); } public function testSupportsReadResourceRequest(): void @@ -186,6 +189,11 @@ public function testHandleResourceNotFoundExceptionReturnsSpecificError(): void ->with($uri) ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Resource not found', ['uri' => $uri, 'exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response); @@ -206,6 +214,11 @@ public function testHandleResourceReadExceptionReturnsActualErrorMessage(): void ->with($uri) ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Error while reading resource "file://corrupted/file.txt": "Failed to read resource: corrupted data".', ['exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response); @@ -226,6 +239,11 @@ public function testHandleGenericExceptionReturnsGenericError(): void ->with($uri) ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Unexpected error while reading resource "file://problematic/file.txt": "Internal database connection failed".', ['exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response); diff --git a/tests/Unit/Server/Handler/Request/ResourceSubscribeTest.php b/tests/Unit/Server/Handler/Request/ResourceSubscribeTest.php index 00512fa4..02e2e5c4 100644 --- a/tests/Unit/Server/Handler/Request/ResourceSubscribeTest.php +++ b/tests/Unit/Server/Handler/Request/ResourceSubscribeTest.php @@ -26,6 +26,7 @@ use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class ResourceSubscribeTest extends TestCase { @@ -33,13 +34,15 @@ class ResourceSubscribeTest extends TestCase private RegistryInterface&MockObject $registry; private SessionInterface&MockObject $session; private SubscriptionManagerInterface&MockObject $subscriptionManager; + private LoggerInterface&MockObject $logger; protected function setUp(): void { $this->registry = $this->createMock(RegistryInterface::class); $this->subscriptionManager = $this->createMock(SubscriptionManagerInterface::class); $this->session = $this->createMock(SessionInterface::class); - $this->handler = new ResourceSubscribeHandler($this->registry, $this->subscriptionManager); + $this->logger = $this->createMock(LoggerInterface::class); + $this->handler = new ResourceSubscribeHandler($this->registry, $this->subscriptionManager, $this->logger); } #[TestDox('Client can successfully subscribe to a resource')] @@ -121,6 +124,11 @@ public function testHandleSubscribeResourceNotFoundException(): void ->with($uri) ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Resource not found', ['uri' => $uri, 'exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response); diff --git a/tests/Unit/Server/Handler/Request/ResourceUnsubscribeTest.php b/tests/Unit/Server/Handler/Request/ResourceUnsubscribeTest.php index 509c256d..d71dfb3b 100644 --- a/tests/Unit/Server/Handler/Request/ResourceUnsubscribeTest.php +++ b/tests/Unit/Server/Handler/Request/ResourceUnsubscribeTest.php @@ -26,6 +26,7 @@ use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class ResourceUnsubscribeTest extends TestCase { @@ -33,14 +34,16 @@ class ResourceUnsubscribeTest extends TestCase private RegistryInterface&MockObject $registry; private SessionInterface&MockObject $session; private SubscriptionManagerInterface&MockObject $subscriptionManager; + private LoggerInterface&MockObject $logger; protected function setUp(): void { $this->registry = $this->createMock(RegistryInterface::class); $this->subscriptionManager = $this->createMock(SubscriptionManagerInterface::class); $this->session = $this->createMock(SessionInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); - $this->handler = new ResourceUnsubscribeHandler($this->registry, $this->subscriptionManager); + $this->handler = new ResourceUnsubscribeHandler($this->registry, $this->subscriptionManager, $this->logger); } #[TestDox('Client can unsubscribe from a resource')] @@ -119,6 +122,11 @@ public function testHandleUnsubscribeResourceNotFoundException(): void ->with($uri) ->willThrowException($exception); + $this->logger + ->expects($this->once()) + ->method('error') + ->with('Resource not found', ['uri' => $uri, 'exception' => $exception]); + $response = $this->handler->handle($request, $this->session); $this->assertInstanceOf(Error::class, $response);