Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 7 additions & 7 deletions .github/workflows/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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"}'
10 changes: 9 additions & 1 deletion src/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +31 to +39
Copy link
Member

Choose a reason for hiding this comment

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

what about unifying this with the other exception handling?

until now the exception handling is isolated within the JsonRpcHandler, but I'd agree that return null is somehow a bit odd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, the other exception handling may throw a json exception.

That is why I need it here... or double try/catch


if (null === $response) {
continue;
Expand Down
11 changes: 9 additions & 2 deletions src/Server/JsonRpcHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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]);
Expand All @@ -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) {
Expand Down
41 changes: 41 additions & 0 deletions tests/Fixtures/InMemoryTransport.php
Original file line number Diff line number Diff line change
@@ -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
{
}
}
2 changes: 1 addition & 1 deletion tests/Message/ErrorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/Message/FactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/Message/ResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions tests/ServerTest.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading