From 15afbc33913514a3017842162b6f1ae9499b2ea9 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 01:04:49 +0200 Subject: [PATCH 1/6] [Client] Implement setMaxRetries connection retries --- docs/client.md | 24 +++- src/Client.php | 43 ++++++- src/Client/Builder.php | 6 +- src/Client/Protocol.php | 32 +++-- tests/Unit/ClientTest.php | 245 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 333 insertions(+), 17 deletions(-) create mode 100644 tests/Unit/ClientTest.php diff --git a/docs/client.md b/docs/client.md index 494edf1f..bbc5cf8a 100644 --- a/docs/client.md +++ b/docs/client.md @@ -57,10 +57,32 @@ $client = Client::builder() ->setClientInfo('My Application', '1.0.0', 'Description of my client') ->setInitTimeout(30) // Seconds to wait for initialization ->setRequestTimeout(120) // Seconds to wait for request responses - ->setMaxRetries(3) // Retry attempts for failed connections + ->setMaxRetries(3) // Retries for failed connections ->build(); ``` +### Connection Retries + +`setMaxRetries()` controls how often `connect()` retries a failed connection. It +counts retries rather than attempts, so the default of `3` means one initial +attempt plus up to three retries — four in total — before the `ConnectionException` +of the last attempt is rethrown: + +```php +$client = Client::builder() + ->setMaxRetries(0) // Fail on the first failed attempt + ->build(); +``` + +Between two attempts the transport is closed, so a retry never reuses a +half-established connection: a `StdioTransport` spawns a fresh server process and +an `HttpTransport` discards the session ID of the failed attempt. Each retry is +preceded by a short, linearly growing delay (100ms, 200ms, 300ms, …). + +Only the connection handshake is retried. Individual requests such as +`callTool()` are always sent once — retrying them is unsafe as tool calls are not +necessarily idempotent. + ### Client Information Set the client's identity reported to servers during initialization: diff --git a/src/Client.php b/src/Client.php index dd290698..ea5d7df6 100644 --- a/src/Client.php +++ b/src/Client.php @@ -55,6 +55,13 @@ */ class Client { + /** + * Base delay between two connection attempts, in milliseconds. + * + * Scaled by the attempt number, so consecutive retries back off linearly. + */ + private const RETRY_BASE_DELAY_MS = 100; + private ?TransportInterface $transport = null; public function __construct( @@ -75,16 +82,46 @@ public static function builder(): Builder /** * Connect to an MCP server using the provided transport. * - * @throws ConnectionException If connection or initialization fails + * A failed attempt is retried up to the configured number of retries, see + * {@see Builder::setMaxRetries()}. The transport is closed between attempts, + * so every retry starts from a clean connection, and a short delay is waited + * out before each one. + * + * @throws ConnectionException If connection or initialization fails on every attempt */ public function connect(TransportInterface $transport): void { $this->transport = $transport; $this->protocol->connect($transport, $this->config); - $transport->connect(); + // A negative retry count would otherwise skip the connection entirely. + $maxAttempts = max(1, $this->config->maxRetries + 1); + + for ($attempt = 1; $attempt <= $maxAttempts; ++$attempt) { + try { + $transport->connect(); - $this->logger->info('Client connected and initialized'); + $this->logger->info('Client connected and initialized', ['attempt' => $attempt]); + + return; + } catch (ConnectionException $e) { + // Release whatever the failed attempt left behind - a spawned + // process, an HTTP session - so the next one starts clean. + $transport->close(); + + if ($attempt === $maxAttempts) { + throw $e; + } + + $this->logger->warning('Connection attempt failed, retrying', [ + 'attempt' => $attempt, + 'max_attempts' => $maxAttempts, + 'exception' => $e, + ]); + + usleep($attempt * self::RETRY_BASE_DELAY_MS * 1000); + } + } } /** diff --git a/src/Client/Builder.php b/src/Client/Builder.php index 35f7f5de..9a69da17 100644 --- a/src/Client/Builder.php +++ b/src/Client/Builder.php @@ -96,7 +96,11 @@ public function setRequestTimeout(int $seconds): self } /** - * Set maximum retry attempts for failed connections. + * Set the number of times a failed connection attempt is retried. + * + * Counts retries, not attempts: 3 means one initial attempt plus up to three + * retries. Pass 0 to fail on the first failure. Only applies to + * {@see Client::connect()}; individual requests are never retried. */ public function setMaxRetries(int $retries): self { diff --git a/src/Client/Protocol.php b/src/Client/Protocol.php index 75648456..3708e9e7 100644 --- a/src/Client/Protocol.php +++ b/src/Client/Protocol.php @@ -145,22 +145,30 @@ public function request(Request $request, int $timeout, bool $withProgress = fal } $this->state->addPendingRequest($requestId, $timeout); - $this->sendRequest($request); - $immediate = $this->state->consumeResponse($requestId); - if (null !== $immediate) { - $this->logger->debug('Received immediate response', ['id' => $requestId]); + try { + $this->sendRequest($request); - return $immediate; - } + $immediate = $this->state->consumeResponse($requestId); + if (null !== $immediate) { + $this->logger->debug('Received immediate response', ['id' => $requestId]); - $this->logger->debug('Suspending fiber for response', ['id' => $requestId]); + return $immediate; + } - return \Fiber::suspend([ - 'type' => 'await_response', - 'request_id' => $requestId, - 'timeout' => $timeout, - ]); + $this->logger->debug('Suspending fiber for response', ['id' => $requestId]); + + return \Fiber::suspend([ + 'type' => 'await_response', + 'request_id' => $requestId, + 'timeout' => $timeout, + ]); + } finally { + // Only the response path clears the pending request; a request that + // timed out or whose send() threw would otherwise stay pending + // forever and immediately fail every later request as timed out. + $this->state->removePendingRequest($requestId); + } } /** diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php new file mode 100644 index 00000000..058243ab --- /dev/null +++ b/tests/Unit/ClientTest.php @@ -0,0 +1,245 @@ +assertInstanceOf(Builder::class, Client::builder()); + } + + #[TestDox('connect() succeeds on the first attempt without retrying')] + public function testConnectSucceedsWithoutRetrying(): void + { + $transport = new FakeTransport([FakeTransport::ACCEPT]); + + $client = Client::builder()->build(); + $client->connect($transport); + + $this->assertSame(1, $transport->connectCalls); + $this->assertSame(0, $transport->closeCalls); + $this->assertTrue($client->isConnected()); + } + + #[TestDox('connect() retries a failed attempt and succeeds on a later one')] + public function testConnectRetriesUntilItSucceeds(): void + { + $transport = new FakeTransport([FakeTransport::REJECT, FakeTransport::REJECT, FakeTransport::ACCEPT]); + + $client = Client::builder()->setMaxRetries(3)->build(); + $client->connect($transport); + + $this->assertSame(3, $transport->connectCalls); + $this->assertTrue($client->isConnected()); + $this->assertSame('Test Server', $client->getServerInfo()?->name); + } + + #[TestDox('connect() closes the transport between two attempts')] + public function testConnectClosesTransportBetweenAttempts(): void + { + $transport = new FakeTransport([FakeTransport::REJECT, FakeTransport::ACCEPT]); + + $client = Client::builder()->setMaxRetries(1)->build(); + $client->connect($transport); + + $this->assertSame(2, $transport->connectCalls); + $this->assertSame(1, $transport->closeCalls, 'the failed attempt must be cleaned up before the retry'); + } + + #[TestDox('connect() rethrows once the retries are exhausted')] + public function testConnectThrowsWhenRetriesAreExhausted(): void + { + $transport = new FakeTransport([FakeTransport::REJECT, FakeTransport::REJECT, FakeTransport::REJECT]); + + $client = Client::builder()->setMaxRetries(2)->build(); + + try { + $client->connect($transport); + $this->fail(\sprintf('Expected a "%s" to be thrown.', ConnectionException::class)); + } catch (ConnectionException) { + // Expected. + } + + $this->assertSame(3, $transport->connectCalls, 'one initial attempt plus two retries'); + $this->assertSame(3, $transport->closeCalls, 'the last attempt must be cleaned up as well'); + $this->assertFalse($client->isConnected()); + } + + #[TestDox('setMaxRetries(0) disables retrying')] + public function testZeroRetriesAttemptsToConnectOnce(): void + { + $transport = new FakeTransport([FakeTransport::REJECT, FakeTransport::ACCEPT]); + + $client = Client::builder()->setMaxRetries(0)->build(); + + $this->expectException(ConnectionException::class); + + try { + $client->connect($transport); + } finally { + $this->assertSame(1, $transport->connectCalls); + } + } + + #[TestDox('a timed out attempt does not leave state behind that fails the retry')] + public function testTimedOutAttemptDoesNotPoisonTheRetry(): void + { + // An init timeout of 0 makes the first attempt time out instead of + // being answered, which is the path that used to leave the request + // pending forever and time out every following attempt right away. + $transport = new FakeTransport([FakeTransport::IGNORE, FakeTransport::ACCEPT]); + + $client = Client::builder()->setInitTimeout(0)->setMaxRetries(1)->build(); + $client->connect($transport); + + $this->assertSame(2, $transport->connectCalls); + $this->assertTrue($client->isConnected()); + } +} + +/** + * Transport whose connection attempts succeed or fail on command. + * + * Requests are answered from the polling loop rather than from send(), the way + * a stdio transport does, with each connect() call answering according to the + * next configured outcome. + * + * @phpstan-import-type McpFiber from TransportInterface + */ +final class FakeTransport extends BaseTransport +{ + /** The initialize request is answered with a result. */ + public const ACCEPT = 'accept'; + + /** The initialize request is answered with a JSON-RPC error. */ + public const REJECT = 'reject'; + + /** The initialize request is not answered at all and has to time out. */ + public const IGNORE = 'ignore'; + + public int $connectCalls = 0; + public int $closeCalls = 0; + + private string $outcome = self::ACCEPT; + + /** @var list Answers not yet delivered to the client */ + private array $outbox = []; + + /** + * @param list $attempts How each successive connect() call behaves + */ + public function __construct(private array $attempts = [self::ACCEPT]) + { + parent::__construct(); + } + + public function connect(): void + { + ++$this->connectCalls; + $this->outcome = array_shift($this->attempts) ?? self::ACCEPT; + + $result = $this->runRequest(new \Fiber(fn () => $this->handleInitialize())); + + if ($result instanceof Error) { + throw new ConnectionException('Initialization failed: '.$result->message); + } + } + + public function send(string $data): void + { + if (self::IGNORE === $this->outcome) { + return; + } + + $message = json_decode($data, true, 512, \JSON_THROW_ON_ERROR); + + if (!isset($message['id'])) { + return; // A notification, nothing to answer. + } + + $answer = self::REJECT === $this->outcome + ? ['error' => ['code' => Error::INTERNAL_ERROR, 'message' => 'Server unavailable']] + : ['result' => [ + 'protocolVersion' => ProtocolVersion::V2025_11_25->value, + 'capabilities' => [], + 'serverInfo' => ['name' => 'Test Server', 'version' => '1.0.0'], + ]]; + + $this->outbox[] = json_encode(['jsonrpc' => '2.0', 'id' => $message['id']] + $answer, \JSON_THROW_ON_ERROR); + } + + public function runRequest(\Fiber $fiber, ?callable $onProgress = null): Response|Error + { + $fiber->start(); + + for ($poll = 0; !$fiber->isTerminated(); ++$poll) { + if ($poll > 1000) { + throw new \LogicException('The fiber never terminated, no pending request became resolvable.'); + } + + foreach ($this->outbox as $answer) { + $this->handleMessage($answer); + } + $this->outbox = []; + + $this->resumeFiber($fiber); + } + + return $fiber->getReturn(); + } + + public function close(): void + { + ++$this->closeCalls; + + $this->handleClose('Transport closed'); + } + + /** + * @param McpFiber $fiber + */ + private function resumeFiber(\Fiber $fiber): void + { + if (!$fiber->isSuspended() || null === $this->state) { + return; + } + + foreach ($this->state->getPendingRequests() as $pending) { + $response = $this->state->consumeResponse($pending['request_id']); + + if (null !== $response) { + $fiber->resume($response); + + return; + } + + if (time() - $pending['timestamp'] >= $pending['timeout']) { + $fiber->resume(Error::forInternalError('Request timed out', $pending['request_id'])); + + return; + } + } + } +} From cfe16009b1f870ef4882e0b0b64310ff05fe2ea1 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 01:08:45 +0200 Subject: [PATCH 2/6] [Client] Reject a negative retry count in Configuration --- src/Client.php | 3 +-- src/Client/Builder.php | 2 ++ src/Client/Configuration.php | 4 ++++ tests/Unit/ClientTest.php | 12 ++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Client.php b/src/Client.php index ea5d7df6..d187f48e 100644 --- a/src/Client.php +++ b/src/Client.php @@ -94,8 +94,7 @@ public function connect(TransportInterface $transport): void $this->transport = $transport; $this->protocol->connect($transport, $this->config); - // A negative retry count would otherwise skip the connection entirely. - $maxAttempts = max(1, $this->config->maxRetries + 1); + $maxAttempts = $this->config->maxRetries + 1; for ($attempt = 1; $attempt <= $maxAttempts; ++$attempt) { try { diff --git a/src/Client/Builder.php b/src/Client/Builder.php index 9a69da17..b97c7dc4 100644 --- a/src/Client/Builder.php +++ b/src/Client/Builder.php @@ -101,6 +101,8 @@ public function setRequestTimeout(int $seconds): self * Counts retries, not attempts: 3 means one initial attempt plus up to three * retries. Pass 0 to fail on the first failure. Only applies to * {@see Client::connect()}; individual requests are never retried. + * + * Negative values are rejected by {@see Configuration} when building. */ public function setMaxRetries(int $retries): self { diff --git a/src/Client/Configuration.php b/src/Client/Configuration.php index 8c7e909b..e22c740f 100644 --- a/src/Client/Configuration.php +++ b/src/Client/Configuration.php @@ -11,6 +11,7 @@ namespace Mcp\Client; +use Mcp\Exception\InvalidArgumentException; use Mcp\Schema\ClientCapabilities; use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Schema\Implementation; @@ -30,5 +31,8 @@ public function __construct( public readonly int $requestTimeout = 120, public readonly int $maxRetries = 3, ) { + if ($maxRetries < 0) { + throw new InvalidArgumentException(\sprintf('The maximum number of retries must be zero or greater, got %d.', $maxRetries)); + } } } diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php index 058243ab..36d69ee8 100644 --- a/tests/Unit/ClientTest.php +++ b/tests/Unit/ClientTest.php @@ -16,6 +16,7 @@ use Mcp\Client\Transport\BaseTransport; use Mcp\Client\Transport\TransportInterface; use Mcp\Exception\ConnectionException; +use Mcp\Exception\InvalidArgumentException; use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Schema\JsonRpc\Error; use Mcp\Schema\JsonRpc\Response; @@ -103,6 +104,17 @@ public function testZeroRetriesAttemptsToConnectOnce(): void } } + #[TestDox('a negative retry count is rejected')] + public function testNegativeRetryCountIsRejected(): void + { + $builder = Client::builder()->setMaxRetries(-1); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The maximum number of retries must be zero or greater, got -1.'); + + $builder->build(); + } + #[TestDox('a timed out attempt does not leave state behind that fails the retry')] public function testTimedOutAttemptDoesNotPoisonTheRetry(): void { From 82fea5982975d6922339c1116f2d067f3414d51a Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 01:13:48 +0200 Subject: [PATCH 3/6] [Client] Use a positive init timeout in the retry test --- tests/Unit/ClientTest.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php index 36d69ee8..e441f634 100644 --- a/tests/Unit/ClientTest.php +++ b/tests/Unit/ClientTest.php @@ -118,12 +118,13 @@ public function testNegativeRetryCountIsRejected(): void #[TestDox('a timed out attempt does not leave state behind that fails the retry')] public function testTimedOutAttemptDoesNotPoisonTheRetry(): void { - // An init timeout of 0 makes the first attempt time out instead of - // being answered, which is the path that used to leave the request - // pending forever and time out every following attempt right away. + // The first attempt is left unanswered so it runs into its init + // timeout, which is the path that used to leave the request pending + // forever and time out every following attempt right away. The timeout + // is the shortest one allowed, so this waits out about a second. $transport = new FakeTransport([FakeTransport::IGNORE, FakeTransport::ACCEPT]); - $client = Client::builder()->setInitTimeout(0)->setMaxRetries(1)->build(); + $client = Client::builder()->setInitTimeout(1)->setMaxRetries(1)->build(); $client->connect($transport); $this->assertSame(2, $transport->connectCalls); @@ -206,8 +207,11 @@ public function runRequest(\Fiber $fiber, ?callable $onProgress = null): Respons { $fiber->start(); + // Polls at the same 1ms interval as the real transports so that a + // request left unanswered runs into its timeout in real time, capped + // well above the longest timeout any test here configures. for ($poll = 0; !$fiber->isTerminated(); ++$poll) { - if ($poll > 1000) { + if ($poll > 5000) { throw new \LogicException('The fiber never terminated, no pending request became resolvable.'); } @@ -217,6 +221,8 @@ public function runRequest(\Fiber $fiber, ?callable $onProgress = null): Respons $this->outbox = []; $this->resumeFiber($fiber); + + usleep(1000); } return $fiber->getReturn(); From e2bedfdfe98ce8e49fd52ff237703134d5ffe17c Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 01:16:16 +0200 Subject: [PATCH 4/6] [Client] Drop redundant docblock note on setMaxRetries --- src/Client/Builder.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Client/Builder.php b/src/Client/Builder.php index b97c7dc4..9a69da17 100644 --- a/src/Client/Builder.php +++ b/src/Client/Builder.php @@ -101,8 +101,6 @@ public function setRequestTimeout(int $seconds): self * Counts retries, not attempts: 3 means one initial attempt plus up to three * retries. Pass 0 to fail on the first failure. Only applies to * {@see Client::connect()}; individual requests are never retried. - * - * Negative values are rejected by {@see Configuration} when building. */ public function setMaxRetries(int $retries): self { From 056dfcc91f14277c2e61773732091ba90347bec7 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 02:05:39 +0200 Subject: [PATCH 5/6] [Client] Clear the initialized flag on a failed connection --- src/Client.php | 5 +++++ tests/Unit/ClientTest.php | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/Client.php b/src/Client.php index d187f48e..d6e88a5a 100644 --- a/src/Client.php +++ b/src/Client.php @@ -104,6 +104,11 @@ public function connect(TransportInterface $transport): void return; } catch (ConnectionException $e) { + // The handshake marks the session initialized before sending the + // initialized notification, so a failure in between would leave + // the client reporting a connection it no longer has. + $this->protocol->getState()->setInitialized(false); + // Release whatever the failed attempt left behind - a spawned // process, an HTTP session - so the next one starts clean. $transport->close(); diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php index e441f634..a577d76f 100644 --- a/tests/Unit/ClientTest.php +++ b/tests/Unit/ClientTest.php @@ -115,6 +115,26 @@ public function testNegativeRetryCountIsRejected(): void $builder->build(); } + #[TestDox('a connection breaking after the handshake result does not leave the client connected')] + public function testFailureAfterHandshakeResultDoesNotLeaveClientConnected(): void + { + // The handshake marks the session initialized before sending the + // initialized notification, so a failure in between must not leave the + // client reporting a connection it no longer has. + $transport = new FakeTransport([FakeTransport::BREAK_AFTER_ACCEPT]); + + $client = Client::builder()->setMaxRetries(0)->build(); + + try { + $client->connect($transport); + $this->fail(\sprintf('Expected a "%s" to be thrown.', ConnectionException::class)); + } catch (ConnectionException) { + // Expected. + } + + $this->assertFalse($client->isConnected()); + } + #[TestDox('a timed out attempt does not leave state behind that fails the retry')] public function testTimedOutAttemptDoesNotPoisonTheRetry(): void { @@ -152,6 +172,9 @@ final class FakeTransport extends BaseTransport /** The initialize request is not answered at all and has to time out. */ public const IGNORE = 'ignore'; + /** The initialize request is answered, but the connection breaks right after. */ + public const BREAK_AFTER_ACCEPT = 'break_after_accept'; + public int $connectCalls = 0; public int $closeCalls = 0; @@ -161,7 +184,7 @@ final class FakeTransport extends BaseTransport private array $outbox = []; /** - * @param list $attempts How each successive connect() call behaves + * @param list $attempts How each successive connect() call behaves */ public function __construct(private array $attempts = [self::ACCEPT]) { @@ -189,7 +212,12 @@ public function send(string $data): void $message = json_decode($data, true, 512, \JSON_THROW_ON_ERROR); if (!isset($message['id'])) { - return; // A notification, nothing to answer. + // The initialized notification, sent once the handshake succeeded. + if (self::BREAK_AFTER_ACCEPT === $this->outcome) { + throw new ConnectionException('Connection lost'); + } + + return; // Nothing to answer. } $answer = self::REJECT === $this->outcome From 7deffba4124fd5d5f2344b22aa8a5874259cefc1 Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 11 Aug 2026 02:07:43 +0200 Subject: [PATCH 6/6] [Client] Trim comments to the non-obvious ones --- src/Client.php | 17 +++-------------- src/Client/Protocol.php | 5 ++--- tests/Unit/ClientTest.php | 16 ++++------------ 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/src/Client.php b/src/Client.php index d6e88a5a..37bd3b44 100644 --- a/src/Client.php +++ b/src/Client.php @@ -55,11 +55,6 @@ */ class Client { - /** - * Base delay between two connection attempts, in milliseconds. - * - * Scaled by the attempt number, so consecutive retries back off linearly. - */ private const RETRY_BASE_DELAY_MS = 100; private ?TransportInterface $transport = null; @@ -82,10 +77,7 @@ public static function builder(): Builder /** * Connect to an MCP server using the provided transport. * - * A failed attempt is retried up to the configured number of retries, see - * {@see Builder::setMaxRetries()}. The transport is closed between attempts, - * so every retry starts from a clean connection, and a short delay is waited - * out before each one. + * A failed attempt is closed and retried, see {@see Builder::setMaxRetries()}. * * @throws ConnectionException If connection or initialization fails on every attempt */ @@ -104,13 +96,10 @@ public function connect(TransportInterface $transport): void return; } catch (ConnectionException $e) { - // The handshake marks the session initialized before sending the - // initialized notification, so a failure in between would leave - // the client reporting a connection it no longer has. + // initialize() flags the session before sending the initialized + // notification, so a failure in between leaves the flag set. $this->protocol->getState()->setInitialized(false); - // Release whatever the failed attempt left behind - a spawned - // process, an HTTP session - so the next one starts clean. $transport->close(); if ($attempt === $maxAttempts) { diff --git a/src/Client/Protocol.php b/src/Client/Protocol.php index 3708e9e7..98c18ae8 100644 --- a/src/Client/Protocol.php +++ b/src/Client/Protocol.php @@ -164,9 +164,8 @@ public function request(Request $request, int $timeout, bool $withProgress = fal 'timeout' => $timeout, ]); } finally { - // Only the response path clears the pending request; a request that - // timed out or whose send() threw would otherwise stay pending - // forever and immediately fail every later request as timed out. + // Only the response path clears it, so a request that timed out or + // whose send() threw would stay pending and fail every later one. $this->state->removePendingRequest($requestId); } } diff --git a/tests/Unit/ClientTest.php b/tests/Unit/ClientTest.php index a577d76f..558de954 100644 --- a/tests/Unit/ClientTest.php +++ b/tests/Unit/ClientTest.php @@ -118,9 +118,6 @@ public function testNegativeRetryCountIsRejected(): void #[TestDox('a connection breaking after the handshake result does not leave the client connected')] public function testFailureAfterHandshakeResultDoesNotLeaveClientConnected(): void { - // The handshake marks the session initialized before sending the - // initialized notification, so a failure in between must not leave the - // client reporting a connection it no longer has. $transport = new FakeTransport([FakeTransport::BREAK_AFTER_ACCEPT]); $client = Client::builder()->setMaxRetries(0)->build(); @@ -138,10 +135,7 @@ public function testFailureAfterHandshakeResultDoesNotLeaveClientConnected(): vo #[TestDox('a timed out attempt does not leave state behind that fails the retry')] public function testTimedOutAttemptDoesNotPoisonTheRetry(): void { - // The first attempt is left unanswered so it runs into its init - // timeout, which is the path that used to leave the request pending - // forever and time out every following attempt right away. The timeout - // is the shortest one allowed, so this waits out about a second. + // The shortest allowed init timeout, so this waits out about a second. $transport = new FakeTransport([FakeTransport::IGNORE, FakeTransport::ACCEPT]); $client = Client::builder()->setInitTimeout(1)->setMaxRetries(1)->build(); @@ -212,12 +206,11 @@ public function send(string $data): void $message = json_decode($data, true, 512, \JSON_THROW_ON_ERROR); if (!isset($message['id'])) { - // The initialized notification, sent once the handshake succeeded. if (self::BREAK_AFTER_ACCEPT === $this->outcome) { throw new ConnectionException('Connection lost'); } - return; // Nothing to answer. + return; // A notification, nothing to answer. } $answer = self::REJECT === $this->outcome @@ -235,9 +228,8 @@ public function runRequest(\Fiber $fiber, ?callable $onProgress = null): Respons { $fiber->start(); - // Polls at the same 1ms interval as the real transports so that a - // request left unanswered runs into its timeout in real time, capped - // well above the longest timeout any test here configures. + // Polls at the same 1ms interval as the real transports, so a request + // left unanswered runs into its timeout in real time. for ($poll = 0; !$fiber->isTerminated(); ++$poll) { if ($poll > 5000) { throw new \LogicException('The fiber never terminated, no pending request became resolvable.');