feat: handle JSON exceptions (#15)

* feat: handle json exceptions

* fix: ci

Author: Nyholm

.github/workflows/pipeline.yml

@@ -42,13 +42,6 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
-      - name: Conventional Commit
-        uses: ytanikin/[email protected]
-        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/[email protected]
+        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"}'

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;

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<int, RequestHandler>
@@ -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) {

tests/Fixtures/InMemoryTransport.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace PhpLlm\McpSdk\Tests\Fixtures;
+
+use PhpLlm\McpSdk\Server\Transport;
+
+class InMemoryTransport implements Transport
+{
+    private bool $connected = true;
+
+    /**
+     * @param list<string> $messages
+     */
+    public function __construct(
+        private readonly array $messages = [],
+    ) {
+    }
+
+    public function initialize(): void
+    {
+    }
+
+    public function isConnected(): bool
+    {
+        return $this->connected;
+    }
+
+    public function receive(): \Generator
+    {
+        yield from $this->messages;
+        $this->connected = false;
+    }
+
+    public function send(string $data): void
+    {
+    }
+
+    public function close(): void
+    {
+    }
+}

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;

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;

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;

tests/ServerTest.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace PhpLlm\McpSdk\Tests;
+
+use PhpLlm\McpSdk\Server;
+use PhpLlm\McpSdk\Server\JsonRpcHandler;
+use PhpLlm\McpSdk\Tests\Fixtures\InMemoryTransport;
+use PHPUnit\Framework\MockObject\Stub\Exception;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+
+class ServerTest extends TestCase
+{
+    public function testJsonExceptions(): void
+    {
+        $logger = $this->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);
+    }
+}