[Server] Keep inbound request intact when queueing an outbound one (#486)
This commit is contained in:
parent
1bd7895b5f
commit
749c0353ef
2 changed files with 63 additions and 2 deletions
|
|
@ -303,9 +303,11 @@ class Protocol
|
||||||
$notification = $result['notification'];
|
$notification = $result['notification'];
|
||||||
$this->sendNotification($notification, $session);
|
$this->sendNotification($notification, $session);
|
||||||
} elseif ('request' === $result['type']) {
|
} elseif ('request' === $result['type']) {
|
||||||
$request = $result['request'];
|
// Keep $request untouched: it is the inbound request the catch
|
||||||
|
// blocks below answer under, not this outbound one.
|
||||||
|
$outboundRequest = $result['request'];
|
||||||
$timeout = $result['timeout'] ?? 120;
|
$timeout = $result['timeout'] ?? 120;
|
||||||
$this->sendRequest($request, $timeout, $session);
|
$this->sendRequest($outboundRequest, $timeout, $session);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -25,7 +25,9 @@ use Mcp\Schema\Request\PingRequest;
|
||||||
use Mcp\Server\Handler\Notification\NotificationHandlerInterface;
|
use Mcp\Server\Handler\Notification\NotificationHandlerInterface;
|
||||||
use Mcp\Server\Handler\Request\RequestHandlerInterface;
|
use Mcp\Server\Handler\Request\RequestHandlerInterface;
|
||||||
use Mcp\Server\Protocol;
|
use Mcp\Server\Protocol;
|
||||||
|
use Mcp\Server\Session\InMemorySessionStore;
|
||||||
use Mcp\Server\Session\SessionInterface;
|
use Mcp\Server\Session\SessionInterface;
|
||||||
|
use Mcp\Server\Session\SessionManager;
|
||||||
use Mcp\Server\Session\SessionManagerInterface;
|
use Mcp\Server\Session\SessionManagerInterface;
|
||||||
use Mcp\Server\Transport\TransportInterface;
|
use Mcp\Server\Transport\TransportInterface;
|
||||||
use Mcp\Tests\Unit\Fixtures\ThrowingRequest;
|
use Mcp\Tests\Unit\Fixtures\ThrowingRequest;
|
||||||
|
|
@ -793,6 +795,63 @@ final class ProtocolTest extends TestCase
|
||||||
$this->assertStringNotContainsString('Unexpected error', $message['error']['message']);
|
$this->assertStringNotContainsString('Unexpected error', $message['error']['message']);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[TestDox('Failure while dispatching an outbound request is answered under the inbound request id')]
|
||||||
|
public function testOutboundRequestFailureIsAnsweredUnderInboundRequestId(): void
|
||||||
|
{
|
||||||
|
$handler = $this->createMock(RequestHandlerInterface::class);
|
||||||
|
$handler->method('supports')->willReturn(true);
|
||||||
|
$handler->method('handle')->willReturnCallback(static function (): Response {
|
||||||
|
// Suspend with an outbound, id-less request, as sampling/elicitation handlers do.
|
||||||
|
\Fiber::suspend(['type' => 'request', 'request' => new PingRequest(), 'timeout' => 5]);
|
||||||
|
|
||||||
|
return new Response(1, []);
|
||||||
|
});
|
||||||
|
|
||||||
|
$exception = new \RuntimeException('Transport unavailable');
|
||||||
|
$this->transport->method('attachFiberToSession')->willThrowException($exception);
|
||||||
|
|
||||||
|
$sessionManager = new SessionManager(new InMemorySessionStore());
|
||||||
|
|
||||||
|
$events = [];
|
||||||
|
$dispatcher = $this->createMock(EventDispatcherInterface::class);
|
||||||
|
$dispatcher->method('dispatch')->willReturnCallback(static function (object $event) use (&$events): object {
|
||||||
|
$events[] = $event;
|
||||||
|
|
||||||
|
return $event;
|
||||||
|
});
|
||||||
|
|
||||||
|
$protocol = new Protocol(
|
||||||
|
requestHandlers: [$handler],
|
||||||
|
notificationHandlers: [],
|
||||||
|
messageFactory: MessageFactory::make(),
|
||||||
|
sessionManager: $sessionManager,
|
||||||
|
eventDispatcher: $dispatcher,
|
||||||
|
);
|
||||||
|
|
||||||
|
$session = $sessionManager->create();
|
||||||
|
$session->save();
|
||||||
|
$sessionId = $session->getId();
|
||||||
|
$protocol->processInput(
|
||||||
|
$this->transport,
|
||||||
|
'{"jsonrpc": "2.0", "id": 1, "method": "ping"}',
|
||||||
|
$sessionId
|
||||||
|
);
|
||||||
|
|
||||||
|
$errorEvents = array_values(array_filter($events, static fn (object $event): bool => $event instanceof ErrorEvent));
|
||||||
|
$this->assertCount(1, $errorEvents);
|
||||||
|
$this->assertSame($exception, $errorEvents[0]->getThrowable());
|
||||||
|
$this->assertSame(1, $errorEvents[0]->getError()->getId());
|
||||||
|
|
||||||
|
$outgoing = $protocol->consumeOutgoingMessages($sessionId);
|
||||||
|
$errors = array_values(array_filter(
|
||||||
|
array_map(static fn (array $outgoingMessage): array => json_decode($outgoingMessage['message'], true), $outgoing),
|
||||||
|
static fn (array $message): bool => isset($message['error']),
|
||||||
|
));
|
||||||
|
$this->assertCount(1, $errors);
|
||||||
|
$this->assertSame(1, $errors[0]['id']);
|
||||||
|
$this->assertSame(Error::INTERNAL_ERROR, $errors[0]['error']['code']);
|
||||||
|
}
|
||||||
|
|
||||||
#[TestDox('Notification handler exceptions are caught and logged')]
|
#[TestDox('Notification handler exceptions are caught and logged')]
|
||||||
public function testNotificationHandlerExceptionsAreCaught(): void
|
public function testNotificationHandlerExceptionsAreCaught(): void
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue