Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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 src/Capability/Discovery/DocBlockParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we can skip the others then I'd guess:

Suggested change
'error' => $e->getMessage(),
'exception' => $e,
'exception_trace' => $e->getTraceAsString(),
'exception' => $e,

]);

Expand Down
1 change: 1 addition & 0 deletions src/Capability/Discovery/SchemaValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
'exception' => $e,
'exception_message' => $e->getMessage(),
'exception_trace' => $e->getTraceAsString(),
'exception' => $e,

'data' => json_encode($dataToValidate),
Expand Down
2 changes: 1 addition & 1 deletion src/Client/Handler/Request/SamplingRequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/Client/Transport/HttpTransport.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$this->logger->warning('Failed to close session', ['error' => $e->getMessage(), 'exception' => $e]);
$this->logger->warning('Failed to close session', ['exception' => $e]);

}
}

Expand Down
3 changes: 2 additions & 1 deletion src/Server/Handler/Request/CallToolHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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())];
Expand Down
6 changes: 3 additions & 3 deletions src/Server/Handler/Request/GetPromptHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
6 changes: 3 additions & 3 deletions src/Server/Handler/Request/ReadResourceHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/Server/Handler/Request/ResourceSubscribeHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Conformance/client.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 14 additions & 3 deletions tests/Unit/Server/Handler/Request/CallToolHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -288,6 +298,7 @@ public function testHandleLogsErrorWithCorrectParameters(): void
[
'tool' => 'test_tool',
'arguments' => ['key1' => 'value1', 'key2' => 42, '_session' => $this->session, '_request' => $request],
'exception' => $exception,
],
);

Expand Down
15 changes: 14 additions & 1 deletion tests/Unit/Server/Handler/Request/GetPromptHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,24 @@
use Mcp\Server\Session\SessionInterface;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

class GetPromptHandlerTest extends TestCase
{
private GetPromptHandler $handler;
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
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
20 changes: 19 additions & 1 deletion tests/Unit/Server/Handler/Request/ReadResourceHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,24 @@
use Mcp\Server\Session\SessionInterface;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

class ReadResourceHandlerTest extends TestCase
{
private ReadResourceHandler $handler;
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
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion tests/Unit/Server/Handler/Request/ResourceSubscribeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,23 @@
use PHPUnit\Framework\Attributes\TestDox;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

class ResourceSubscribeTest extends TestCase
{
private ResourceSubscribeHandler $handler;
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')]
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,24 @@
use PHPUnit\Framework\Attributes\TestDox;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

class ResourceUnsubscribeTest extends TestCase
{
private ResourceUnsubscribeHandler $handler;
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')]
Expand Down Expand Up @@ -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);
Expand Down