Add PSR-3 compliant exception context when logging exceptions (#300)
* Add PSR-3 exception context to caught error logs * Apply review feedback
This commit is contained in:
parent
bfc4728792
commit
73414d65f6
16 changed files with 88 additions and 27 deletions
|
|
@ -117,7 +117,6 @@ final class Discoverer implements DiscovererInterface
|
|||
} catch (\Throwable $e) {
|
||||
$this->logger->error('Error during file finding process for MCP discovery'.json_encode($e->getTrace(), \JSON_PRETTY_PRINT), [
|
||||
'exception' => $e,
|
||||
'trace' => $e->getTraceAsString(),
|
||||
]);
|
||||
}
|
||||
|
||||
|
|
@ -199,7 +198,6 @@ final class Discoverer implements DiscovererInterface
|
|||
'file' => $file->getPathname(),
|
||||
'class' => $className,
|
||||
'exception' => $e,
|
||||
'trace' => $e->getTraceAsString(),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
|
@ -300,9 +298,15 @@ final class Discoverer implements DiscovererInterface
|
|||
break;
|
||||
}
|
||||
} catch (ExceptionInterface $e) {
|
||||
$this->logger->error("Failed to process MCP attribute on {$className}::{$methodName}", ['attribute' => $attributeClassName, 'exception' => $e, 'trace' => $e->getPrevious() ? $e->getPrevious()->getTraceAsString() : $e->getTraceAsString()]);
|
||||
$this->logger->error("Failed to process MCP attribute on {$className}::{$methodName}", [
|
||||
'attribute' => $attributeClassName,
|
||||
'exception' => $e,
|
||||
]);
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger->error("Unexpected error processing attribute on {$className}::{$methodName}", ['attribute' => $attributeClassName, 'exception' => $e, 'trace' => $e->getTraceAsString()]);
|
||||
$this->logger->error("Unexpected error processing attribute on {$className}::{$methodName}", [
|
||||
'attribute' => $attributeClassName,
|
||||
'exception' => $e,
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -47,8 +47,7 @@ class DocBlockParser
|
|||
} catch (\Throwable $e) {
|
||||
// Log error or handle gracefully if invalid DocBlock syntax is encountered
|
||||
$this->logger->warning('Failed to parse DocBlock', [
|
||||
'error' => $e->getMessage(),
|
||||
'exception_trace' => $e->getTraceAsString(),
|
||||
'exception' => $e,
|
||||
]);
|
||||
|
||||
return null;
|
||||
|
|
|
|||
|
|
@ -87,8 +87,7 @@ class SchemaValidator
|
|||
$result = $validator->validate($dataToValidate, $schemaObject);
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger->error('MCP SDK: JSON Schema validation failed internally.', [
|
||||
'exception_message' => $e->getMessage(),
|
||||
'exception_trace' => $e->getTraceAsString(),
|
||||
'exception' => $e,
|
||||
'data' => json_encode($dataToValidate),
|
||||
'schema' => json_encode($schemaObject),
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -55,7 +55,7 @@ class SamplingRequestHandler implements RequestHandlerInterface
|
|||
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -170,7 +170,7 @@ class HttpTransport extends BaseTransport
|
|||
$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', ['exception' => $e]);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -65,7 +65,7 @@ final class CallToolHandler implements RequestHandlerInterface
|
|||
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 @@ final class CallToolHandler implements RequestHandlerInterface
|
|||
$this->logger->error(\sprintf('Error while executing tool "%s": "%s".', $toolName, $e->getMessage()), [
|
||||
'tool' => $toolName,
|
||||
'arguments' => $arguments,
|
||||
'exception' => $e,
|
||||
]);
|
||||
|
||||
$errorContent = [new TextContent($e->getMessage())];
|
||||
|
|
|
|||
|
|
@ -65,15 +65,15 @@ final class GetPromptHandler implements RequestHandlerInterface
|
|||
|
||||
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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -77,15 +77,15 @@ final class ReadResourceHandler implements RequestHandlerInterface
|
|||
|
||||
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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -55,7 +55,7 @@ final class ResourceSubscribeHandler implements RequestHandlerInterface
|
|||
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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -55,7 +55,7 @@ final class ResourceUnsubscribeHandler implements RequestHandlerInterface
|
|||
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());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -96,7 +96,7 @@ try {
|
|||
$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 {
|
||||
|
|
|
|||
|
|
@ -166,16 +166,18 @@ class CallToolHandlerTest extends TestCase
|
|||
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 @@ class CallToolHandlerTest extends TestCase
|
|||
|
||||
$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 @@ class CallToolHandlerTest extends TestCase
|
|||
[
|
||||
'tool' => 'test_tool',
|
||||
'arguments' => ['key1' => 'value1', 'key2' => 42, '_session' => $this->session, '_request' => $request],
|
||||
'exception' => $exception,
|
||||
],
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ use Mcp\Server\Handler\Request\GetPromptHandler;
|
|||
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 @@ class GetPromptHandlerTest extends TestCase
|
|||
->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 @@ class GetPromptHandlerTest extends TestCase
|
|||
->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);
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ use Mcp\Server\Handler\Request\ReadResourceHandler;
|
|||
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 @@ class ReadResourceHandlerTest extends TestCase
|
|||
->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 @@ class ReadResourceHandlerTest extends TestCase
|
|||
->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 @@ class ReadResourceHandlerTest extends TestCase
|
|||
->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);
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ use Mcp\Server\Session\SessionInterface;
|
|||
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 @@ class ResourceSubscribeTest extends TestCase
|
|||
->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);
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ use Mcp\Server\Session\SessionInterface;
|
|||
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 @@ class ResourceUnsubscribeTest extends TestCase
|
|||
->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);
|
||||
|
|
|
|||
Loading…
Reference in a new issue