[Server] Log failures in CompletionCompleteHandler (#471)
This commit is contained in:
parent
b1c7272761
commit
c882401ee7
3 changed files with 47 additions and 1 deletions
|
|
@ -1079,7 +1079,7 @@ final class Builder
|
|||
|
||||
$requestHandlers = array_merge($this->requestHandlers, [
|
||||
new Handler\Request\CallToolHandler($registry, $referenceHandler, $logger),
|
||||
new Handler\Request\CompletionCompleteHandler($registry, $container),
|
||||
new Handler\Request\CompletionCompleteHandler($registry, $container, $logger),
|
||||
new Handler\Request\GetPromptHandler($registry, $referenceHandler, $logger),
|
||||
new Handler\Request\InitializeHandler($configuration),
|
||||
new Handler\Request\ListPromptsHandler($registry, $this->paginationLimit),
|
||||
|
|
|
|||
|
|
@ -25,6 +25,8 @@ use Mcp\Schema\ResourceReference;
|
|||
use Mcp\Schema\Result\CompletionCompleteResult;
|
||||
use Mcp\Server\Session\SessionInterface;
|
||||
use Psr\Container\ContainerInterface;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Psr\Log\NullLogger;
|
||||
|
||||
/**
|
||||
* Handles completion/complete requests.
|
||||
|
|
@ -38,6 +40,7 @@ final class CompletionCompleteHandler implements RequestHandlerInterface
|
|||
public function __construct(
|
||||
private readonly RegistryInterface $registry,
|
||||
private readonly ?ContainerInterface $container = null,
|
||||
private readonly LoggerInterface $logger = new NullLogger(),
|
||||
) {
|
||||
}
|
||||
|
||||
|
|
@ -87,8 +90,12 @@ final class CompletionCompleteHandler implements RequestHandlerInterface
|
|||
} catch (PromptNotFoundException|ResourceNotFoundException $e) {
|
||||
// The reference names something the server does not have, which is
|
||||
// a bad parameter rather than a missing resource.
|
||||
$this->logger->warning(\sprintf('Completion requested for unknown reference: %s', $e->getMessage()), ['exception' => $e]);
|
||||
|
||||
return Error::forInvalidParams($e->getMessage(), $request->getId());
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger->error(\sprintf('Error while handling completion request: %s', $e->getMessage()), ['exception' => $e]);
|
||||
|
||||
return Error::forInternalError('Error while handling completion request', $request->getId());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,9 +11,11 @@
|
|||
|
||||
namespace Mcp\Tests\Unit\Server\Handler\Request;
|
||||
|
||||
use Mcp\Capability\Completion\ProviderInterface;
|
||||
use Mcp\Capability\Registry\ResourceReference;
|
||||
use Mcp\Capability\Registry\ResourceTemplateReference;
|
||||
use Mcp\Capability\RegistryInterface;
|
||||
use Mcp\Schema\JsonRpc\Error;
|
||||
use Mcp\Schema\JsonRpc\Response;
|
||||
use Mcp\Schema\Request\CompletionCompleteRequest;
|
||||
use Mcp\Schema\ResourceTemplate;
|
||||
|
|
@ -23,6 +25,7 @@ use Mcp\Server\Session\SessionInterface;
|
|||
use Mcp\Tests\Unit\Capability\Attribute\CompletionProviderFixture;
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
class CompletionCompleteHandlerTest extends TestCase
|
||||
{
|
||||
|
|
@ -79,6 +82,42 @@ class CompletionCompleteHandlerTest extends TestCase
|
|||
$this->assertEquals(new CompletionCompleteResult(['alpha'], 1, false), $response->result);
|
||||
}
|
||||
|
||||
public function testLogsProviderFailureBeforeReturningInternalError(): void
|
||||
{
|
||||
$uri = 'file://users/alice';
|
||||
$request = $this->createCompletionRequest($uri, ['name' => 'id', 'value' => 'al']);
|
||||
|
||||
$exception = new \RuntimeException('Provider blew up');
|
||||
$provider = $this->createMock(ProviderInterface::class);
|
||||
$provider->method('getCompletions')->willThrowException($exception);
|
||||
|
||||
$templateReference = new ResourceTemplateReference(
|
||||
new ResourceTemplate('file://users/{id}', 'user'),
|
||||
static fn () => null,
|
||||
['id' => $provider],
|
||||
);
|
||||
|
||||
$this->registry
|
||||
->method('getResource')
|
||||
->with($uri)
|
||||
->willReturn($templateReference);
|
||||
|
||||
$logger = $this->createMock(LoggerInterface::class);
|
||||
$logger
|
||||
->expects($this->once())
|
||||
->method('error')
|
||||
->with(
|
||||
'Error while handling completion request: Provider blew up',
|
||||
['exception' => $exception],
|
||||
);
|
||||
|
||||
$handler = new CompletionCompleteHandler($this->registry, null, $logger);
|
||||
$response = $handler->handle($request, $this->session);
|
||||
|
||||
$this->assertInstanceOf(Error::class, $response);
|
||||
$this->assertSame('Error while handling completion request', $response->message);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array{ name: string, value: string } $argument
|
||||
*/
|
||||
|
|
|
|||
Loading…
Reference in a new issue