From b395b0e35b9b7361321f1ff5eba3257f06f3dfea Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Sun, 2 Aug 2026 11:49:57 +1200 Subject: [PATCH 1/2] fix(client): code the pre-send failures so callers can replay them Connection validation runs ahead of the send, so both failures it raises leave the command unsent and nothing applied - the one class of failure a caller may safely replay. Both carried code 0, making that legible only in the message text, and a caller that classifies on text turns a permanent failure into an advertised-retryable one the moment a message quotes a caller-chosen value. A cloud connection pool consequently could not tell a replayable refusal from a post-send timeout of unknown outcome, and surfaced the former to clients as a 500 during a backing resize. Both now carry MongoDB's own HostUnreachable, with isUnsentError() to read it. The messages are unchanged, so the client's own internal reconnect check that matches on them still behaves identically. Co-Authored-By: Claude Opus 5 --- src/Client.php | 7 +++++-- src/Exception.php | 22 +++++++++++++++++++++ tests/UnsentErrorTest.php | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 tests/UnsentErrorTest.php diff --git a/src/Client.php b/src/Client.php index 1718719..2ad5494 100644 --- a/src/Client.php +++ b/src/Client.php @@ -2178,13 +2178,16 @@ private function validateConnection(): void } } + // Both throws are raised ahead of the send, so the command never + // reached the server and a caller may safely replay it. They carry + // HostUnreachable to say so in a code rather than in text. if (!$this->isConnected) { - throw new Exception('Client is not connected to MongoDB'); + throw new Exception('Client is not connected to MongoDB', Exception::HOST_UNREACHABLE); } if (!$this->client->isConnected()) { $this->isConnected = false; - throw new Exception('Connection to MongoDB has been lost'); + throw new Exception('Connection to MongoDB has been lost', Exception::HOST_UNREACHABLE); } } diff --git a/src/Exception.php b/src/Exception.php index 7c47d6a..184d1ba 100644 --- a/src/Exception.php +++ b/src/Exception.php @@ -9,6 +9,19 @@ */ class Exception extends \Exception { + /** + * The connection was gone before the command left the client. + * + * MongoDB's own HostUnreachable. Carried by every throw raised out of + * connection validation, which runs ahead of the send: the command was + * never on the wire, so nothing was applied and a caller may safely + * replay it. Distinguishing this from a post-send failure used to be + * possible only by matching the message text, which callers cannot do + * safely - a message quotes caller-chosen values, a code cannot be + * spelled by a caller. + */ + public const int HOST_UNREACHABLE = 6; + protected array $errorLabels = []; protected ?array $writeErrors = null; protected ?array $writeConcernErrors = null; @@ -105,6 +118,15 @@ public function isWriteConcernError(): bool * * @return bool */ + /** + * Whether the command was still unsent when this failure was raised, so + * nothing was applied and the caller may replay it. + */ + public function isUnsentError(): bool + { + return $this->code === self::HOST_UNREACHABLE; + } + public function isTimeoutError(): bool { $timeoutCodes = [ diff --git a/tests/UnsentErrorTest.php b/tests/UnsentErrorTest.php new file mode 100644 index 0000000..743f5ad --- /dev/null +++ b/tests/UnsentErrorTest.php @@ -0,0 +1,40 @@ +assertTrue($unsent->isUnsentError()); + $this->assertFalse($unsent->isTimeoutError(), 'An unsent failure must not be confused with a post-send timeout'); + } + + public function testAPostSendTimeoutIsNotReportedAsUnsent(): void + { + $timeout = new Exception('Receive timeout: no data received within reasonable time', 11601); + + $this->assertTrue($timeout->isTimeoutError()); + $this->assertFalse( + $timeout->isUnsentError(), + 'The command was already on the wire, so its outcome is unknown and it must never be replayed blindly', + ); + } + + public function testAnUncodedFailureIsNotAssumedUnsent(): void + { + $this->assertFalse(new Exception('something went wrong')->isUnsentError()); + } +} From 232d355fe8a5a85b8b61d288180ed283fa6a20c3 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Sun, 2 Aug 2026 12:03:48 +1200 Subject: [PATCH 2/2] fix(client): carry the pre-send failures in a type callers can trust Review found the first attempt unsound. Connection validation and the dial before it run ahead of every send, so a failure raised there leaves the command unsent and nothing applied - but a numeric code cannot say so. A post-send error response is parsed into the same Exception holding the SERVER's code, HostUnreachable included, so any caller keying on the code would replay an operation that was already transmitted and could apply it twice. The typed class constant also broke the PHP 8.0-8.2 this package still supports. The signal is now a type: UnsentException, raised only by the client and only before it has sent anything, answering isUnsentError() true where the base answers false. It extends Exception, so every existing catch behaves identically, and the messages are unchanged so the client's own internal reconnect check still matches. The failed reconnect dial throws it too - review caught that it escapes validateConnection while the command is just as unsent. Covered: an unsent failure is distinguishable by type and still caught as the package exception, a post-send timeout is never unsent, and a server response reporting HostUnreachable is never unsent. Co-Authored-By: Claude Opus 5 --- src/Client.php | 9 ++++----- src/Exception.php | 20 ++++++-------------- src/UnsentException.php | 25 +++++++++++++++++++++++++ tests/UnsentErrorTest.php | 35 +++++++++++++++++++++++++---------- 4 files changed, 60 insertions(+), 29 deletions(-) create mode 100644 src/UnsentException.php diff --git a/src/Client.php b/src/Client.php index 2ad5494..8c4c153 100644 --- a/src/Client.php +++ b/src/Client.php @@ -303,7 +303,7 @@ public function connect(): self try { if (!$this->client->connect($this->host, $this->port, $budget)) { $this->invalidate(); - throw new Exception("Failed to connect to MongoDB at {$this->host}:{$this->port}"); + throw new UnsentException("Failed to connect to MongoDB at {$this->host}:{$this->port}"); } $this->isConnected = true; @@ -2179,15 +2179,14 @@ private function validateConnection(): void } // Both throws are raised ahead of the send, so the command never - // reached the server and a caller may safely replay it. They carry - // HostUnreachable to say so in a code rather than in text. + // reached the server and a caller may safely replay it. if (!$this->isConnected) { - throw new Exception('Client is not connected to MongoDB', Exception::HOST_UNREACHABLE); + throw new UnsentException('Client is not connected to MongoDB'); } if (!$this->client->isConnected()) { $this->isConnected = false; - throw new Exception('Connection to MongoDB has been lost', Exception::HOST_UNREACHABLE); + throw new UnsentException('Connection to MongoDB has been lost'); } } diff --git a/src/Exception.php b/src/Exception.php index 184d1ba..61953c2 100644 --- a/src/Exception.php +++ b/src/Exception.php @@ -9,19 +9,6 @@ */ class Exception extends \Exception { - /** - * The connection was gone before the command left the client. - * - * MongoDB's own HostUnreachable. Carried by every throw raised out of - * connection validation, which runs ahead of the send: the command was - * never on the wire, so nothing was applied and a caller may safely - * replay it. Distinguishing this from a post-send failure used to be - * possible only by matching the message text, which callers cannot do - * safely - a message quotes caller-chosen values, a code cannot be - * spelled by a caller. - */ - public const int HOST_UNREACHABLE = 6; - protected array $errorLabels = []; protected ?array $writeErrors = null; protected ?array $writeConcernErrors = null; @@ -121,10 +108,15 @@ public function isWriteConcernError(): bool /** * Whether the command was still unsent when this failure was raised, so * nothing was applied and the caller may replay it. + * + * False here on purpose: this type is what a server's own error response + * is parsed into, and that response carries the server's code. Only + * {@see UnsentException} — raised solely by the client, solely before it + * has sent anything — answers true. */ public function isUnsentError(): bool { - return $this->code === self::HOST_UNREACHABLE; + return false; } public function isTimeoutError(): bool diff --git a/src/UnsentException.php b/src/UnsentException.php new file mode 100644 index 0000000..686a6e8 --- /dev/null +++ b/src/UnsentException.php @@ -0,0 +1,25 @@ +assertTrue($unsent->isUnsentError()); - $this->assertFalse($unsent->isTimeoutError(), 'An unsent failure must not be confused with a post-send timeout'); + $this->assertInstanceOf(Exception::class, $unsent, 'Callers that catch the package exception must still catch it'); } public function testAPostSendTimeoutIsNotReportedAsUnsent(): void @@ -33,8 +38,18 @@ public function testAPostSendTimeoutIsNotReportedAsUnsent(): void ); } - public function testAnUncodedFailureIsNotAssumedUnsent(): void + /** + * The reason a code cannot carry this: the server picks it. A response + * that happens to report HostUnreachable is still a post-send answer, and + * replaying the operation it answered could apply it twice. + */ + public function testAServerErrorResponseIsNeverReportedAsUnsent(): void { - $this->assertFalse(new Exception('something went wrong')->isUnsentError()); + $response = new \stdClass(); + $response->code = 6; + $response->codeName = 'HostUnreachable'; + $response->errmsg = 'host unreachable'; + + $this->assertFalse(Exception::fromResponse($response)->isUnsentError()); } }