From 9c79e2e089b1ace6d3977c0c42c4650214743473 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sun, 25 May 2025 14:14:52 +0200 Subject: [PATCH 1/2] feat: handle json exceptions --- .github/workflows/pipeline.yml | 14 +++++----- src/Server.php | 10 +++++++- src/Server/JsonRpcHandler.php | 11 ++++++-- tests/Fixtures/InMemoryTransport.php | 38 ++++++++++++++++++++++++++++ tests/Message/ErrorTest.php | 2 +- tests/Message/FactoryTest.php | 2 +- tests/Message/ResponseTest.php | 2 +- tests/ServerTest.php | 37 +++++++++++++++++++++++++++ 8 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 tests/Fixtures/InMemoryTransport.php create mode 100644 tests/ServerTest.php diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 68dd972..1008313 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -42,13 +42,6 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Conventional Commit - uses: ytanikin/pr-conventional-commits@1.4.1 - with: - task_types: '["feat", "fix", "docs", "test", "ci", "style", "refactor", "perf", "chore", "revert"]' - add_label: 'false' - custom_labels: '{"feat": "feature", "fix": "bug", "docs": "documentation", "test": "test", "ci": "CI/CD", "style": "codestyle", "refactor": "refactor", "perf": "performance", "chore": "chore", "revert": "revert"}' - - name: Setup PHP uses: shivammathur/setup-php@v2 with: @@ -72,3 +65,10 @@ jobs: - name: PHPStan run: vendor/bin/phpstan analyse + + - name: Conventional Commit + uses: ytanikin/pr-conventional-commits@1.4.1 + with: + task_types: '["feat", "fix", "docs", "test", "ci", "style", "refactor", "perf", "chore", "revert"]' + add_label: 'true' + custom_labels: '{"feat": "feature", "fix": "bug", "docs": "documentation", "test": "test", "ci": "CI/CD", "style": "codestyle", "refactor": "refactor", "perf": "performance", "chore": "chore", "revert": "revert"}' diff --git a/src/Server.php b/src/Server.php index cdf81e1..06312b8 100644 --- a/src/Server.php +++ b/src/Server.php @@ -28,7 +28,15 @@ public function connect(Transport $transport): void continue; } - $response = $this->jsonRpcHandler->process($message); + try { + $response = $this->jsonRpcHandler->process($message); + } catch (\JsonException $e) { + $this->logger->error('Failed to process message', [ + 'message' => $message, + 'exception' => $e, + ]); + continue; + } if (null === $response) { continue; diff --git a/src/Server/JsonRpcHandler.php b/src/Server/JsonRpcHandler.php index c3ef7e5..06c1df6 100644 --- a/src/Server/JsonRpcHandler.php +++ b/src/Server/JsonRpcHandler.php @@ -11,7 +11,10 @@ use PhpLlm\McpSdk\Message\Response; use Psr\Log\LoggerInterface; -final readonly class JsonRpcHandler +/** + * @final + */ +readonly class JsonRpcHandler { /** * @var array @@ -37,6 +40,9 @@ public function __construct( $this->notificationHandlers = $notificationHandlers instanceof \Traversable ? iterator_to_array($notificationHandlers) : $notificationHandlers; } + /** + * @throws \JsonException + */ public function process(string $message): ?string { $this->logger->info('Received message to process', ['message' => $message]); @@ -57,7 +63,8 @@ public function process(string $message): ?string try { return $message instanceof Notification - ? $this->handleNotification($message) : $this->encodeResponse($this->handleRequest($message)); + ? $this->handleNotification($message) + : $this->encodeResponse($this->handleRequest($message)); } catch (\DomainException) { return null; } catch (\InvalidArgumentException $e) { diff --git a/tests/Fixtures/InMemoryTransport.php b/tests/Fixtures/InMemoryTransport.php new file mode 100644 index 0000000..df2da5b --- /dev/null +++ b/tests/Fixtures/InMemoryTransport.php @@ -0,0 +1,38 @@ +connected; + } + + public function receive(): \Generator + { + yield from $this->messages; + $this->connected = false; + } + + public function send(string $data): void + { + } + + public function close(): void + { + } +} diff --git a/tests/Message/ErrorTest.php b/tests/Message/ErrorTest.php index 014d380..23745b5 100644 --- a/tests/Message/ErrorTest.php +++ b/tests/Message/ErrorTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace App\Tests\Mcp\Message; +namespace PhpLlm\McpSdk\Tests\Message; use PhpLlm\McpSdk\Message\Error; use PHPUnit\Framework\TestCase; diff --git a/tests/Message/FactoryTest.php b/tests/Message/FactoryTest.php index b9ed1a0..8a7c632 100644 --- a/tests/Message/FactoryTest.php +++ b/tests/Message/FactoryTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace App\Tests\Mcp\Message; +namespace PhpLlm\McpSdk\Tests\Message; use PhpLlm\McpSdk\Message\Factory; use PhpLlm\McpSdk\Message\Notification; diff --git a/tests/Message/ResponseTest.php b/tests/Message/ResponseTest.php index bb48a15..b35e2b1 100644 --- a/tests/Message/ResponseTest.php +++ b/tests/Message/ResponseTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace App\Tests\Mcp\Message; +namespace PhpLlm\McpSdk\Tests\Message; use PhpLlm\McpSdk\Message\Response; use PHPUnit\Framework\TestCase; diff --git a/tests/ServerTest.php b/tests/ServerTest.php new file mode 100644 index 0000000..d211410 --- /dev/null +++ b/tests/ServerTest.php @@ -0,0 +1,37 @@ +getMockBuilder(NullLogger::class) + ->disableOriginalConstructor() + ->onlyMethods(['error']) + ->getMock(); + $logger->expects($this->once())->method('error'); + + $handler = $this->getMockBuilder(JsonRpcHandler::class) + ->disableOriginalConstructor() + ->onlyMethods(['process']) + ->getMock(); + $handler->expects($this->exactly(2))->method('process')->willReturnOnConsecutiveCalls(new Exception(new \JsonException('foobar')), 'success'); + + $transport = $this->getMockBuilder(InMemoryTransport::class) + ->setConstructorArgs([['foo', 'bar']]) + ->onlyMethods(['send']) + ->getMock(); + $transport->expects($this->once())->method('send')->with('success'); + + $server = new Server($handler, $logger); + $server->connect($transport); + } +} From 948f58e25d78bf8bf206854eea6bf8939d3968d1 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sun, 25 May 2025 16:00:41 +0200 Subject: [PATCH 2/2] fix: ci --- tests/Fixtures/InMemoryTransport.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Fixtures/InMemoryTransport.php b/tests/Fixtures/InMemoryTransport.php index df2da5b..72df57f 100644 --- a/tests/Fixtures/InMemoryTransport.php +++ b/tests/Fixtures/InMemoryTransport.php @@ -8,6 +8,9 @@ class InMemoryTransport implements Transport { private bool $connected = true; + /** + * @param list $messages + */ public function __construct( private readonly array $messages = [], ) {