From 343a812a2b1b39f4ea0f18b3dfcdec895befa83e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=22DualFroz=22=20Fox?= Date: Thu, 8 Oct 2026 18:34:40 +0200 Subject: [PATCH] Fix ConnectionErrorProof response count on single node read errors (#1739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Till Krüss --- CHANGELOG.md | 11 ++-- src/Pipeline/ConnectionErrorProof.php | 2 +- .../Pipeline/ConnectionErrorProofTest.php | 56 +++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab362c8..6a7669d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,13 +9,14 @@ ### Fixed - Fixed client iteration with master/slave and Sentinel replication (#923) - Fixed `*StreamConnection` looping forever when closed in the middle of a bulk reply (#1731) -- Fixed cluster CAS transactions returning transaction objects for reads and sending `MULTI` twice -- Fixed `MultiExec::multi()` being ignored as the first call of a CAS transaction -- Fixed cleanup of empty transactions and transaction callbacks that throw PHP errors -- Fixed `spl_object_hash()` deprecation on PHP 8.6 in `ConnectionErrorProof` pipeline +- Fixed cluster CAS transactions returning transaction objects for reads and sending `MULTI` twice (#1735) +- Fixed `MultiExec::multi()` being ignored as the first call of a CAS transaction (#1735) +- Fixed cleanup of empty transactions and transaction callbacks that throw PHP errors (#1735) +- Fixed `spl_object_hash()` deprecation on PHP 8.6 in `ConnectionErrorProof` pipeline (#1738) +- Fixed `ConnectionErrorProof` pipeline returning too few responses or throwing `ValueError` (#1739) ### Security -- Fixed aborted cluster transactions leaving node connections in `MULTI` +- Fixed aborted cluster transactions leaving node connections in `MULTI` (#1735) ## v3.6.1 (2026-09-17) ### Fixed diff --git a/src/Pipeline/ConnectionErrorProof.php b/src/Pipeline/ConnectionErrorProof.php index 3b873c4..5a5b8e0 100644 --- a/src/Pipeline/ConnectionErrorProof.php +++ b/src/Pipeline/ConnectionErrorProof.php @@ -73,7 +73,7 @@ class ConnectionErrorProof extends Pipeline try { $responses[$i] = $connection->readResponse($command); } catch (CommunicationException $exception) { - $add = count($commands) - count($responses); + $add = $sizeOfPipe - count($responses); $responses = array_merge($responses, array_fill(0, $add, $exception)); break; diff --git a/tests/Predis/Pipeline/ConnectionErrorProofTest.php b/tests/Predis/Pipeline/ConnectionErrorProofTest.php index 52bc4c1..01e448b 100644 --- a/tests/Predis/Pipeline/ConnectionErrorProofTest.php +++ b/tests/Predis/Pipeline/ConnectionErrorProofTest.php @@ -138,6 +138,62 @@ class ConnectionErrorProofTest extends PredisTestCase $pipeline->execute(); } + /** + * @dataProvider singleConnectionReadFailureProvider + * @group disconnected + */ + public function testReturnsExceptionForFailedAndRemainingCommandsOnReadFailureWithSingleConnection( + int $sizeOfPipe, + int $failingIndex + ): void { + $connection = $this->getMockConnection('tcp://127.0.0.1:7001'); + $exception = new ConnectionException($connection, 'Connection lost'); + + $readResponses = []; + $expectedResponses = []; + + for ($i = 0; $i < $sizeOfPipe; ++$i) { + if ($i < $failingIndex) { + $readResponses[] = "value$i"; + $expectedResponses[] = "value$i"; + } else { + $expectedResponses[] = $exception; + } + } + + $readResponses[] = $this->throwException($exception); + + $connection + ->expects($this->once()) + ->method('write'); + $connection + ->expects($this->exactly($failingIndex + 1)) + ->method('readResponse') + ->willReturnOnConsecutiveCalls(...$readResponses); + + $pipeline = new ConnectionErrorProof(new Client($connection)); + + for ($i = 0; $i < $sizeOfPipe; ++$i) { + $pipeline->echo("value$i"); + } + + $responses = $pipeline->execute(); + + $this->assertCount($sizeOfPipe, $responses); + $this->assertSame($expectedResponses, $responses); + } + + public function singleConnectionReadFailureProvider(): array + { + return [ + 'single command' => [1, 0], + 'first of three' => [3, 0], + 'second of three' => [3, 1], + 'last of three' => [3, 2], + 'fourth of five' => [5, 3], + ]; + } + /** * Returns a mocked cluster connection routing commands by their first key. *