diff --git a/CHANGELOG.md b/CHANGELOG.md index f4b0f7de..8fece12a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed - Fixed handling of gap slots in `SlotMap::offsetUnset()` (#1660) - Fixed ZRANGE to include 6.2 arguments (#1662) +- Fixed Sentinel retry to narrow retryable exceptions to CommunicationException (#1665) ## v3.4.2 (2026-03-09) ### Changed diff --git a/src/Connection/Replication/SentinelReplication.php b/src/Connection/Replication/SentinelReplication.php index 1a8bae4a..daf5a168 100644 --- a/src/Connection/Replication/SentinelReplication.php +++ b/src/Connection/Replication/SentinelReplication.php @@ -32,7 +32,6 @@ use Predis\Response\ErrorInterface as ErrorResponseInterface; use Predis\Response\ServerException; use Predis\Retry\Retry; use Predis\Retry\Strategy\ExponentialBackoff; -use Throwable; /** * @author Daniele Alessandri @@ -737,7 +736,7 @@ class SentinelReplication extends AbstractAggregateConnection implements Replica } else { $retry = $parameters->retry; } - $retry->updateCatchableExceptions([Throwable::class]); + $retry->updateCatchableExceptions([CommunicationException::class]); $doCallback = function () use ($method, $command) { $response = $this->getConnectionByCommand($command)->{$method}($command); @@ -749,12 +748,9 @@ class SentinelReplication extends AbstractAggregateConnection implements Replica return $response; }; - $failCallback = function (Throwable $exception) { + $failCallback = function (CommunicationException $exception) { $this->wipeServerList(); - - if ($exception instanceof CommunicationException) { - $exception->getConnection()->disconnect(); - } + $exception->getConnection()->disconnect(); }; return $retry->callWithRetry($doCallback, $failCallback); diff --git a/tests/Predis/Connection/Replication/SentinelReplicationTest.php b/tests/Predis/Connection/Replication/SentinelReplicationTest.php index 818a3028..3b08e362 100644 --- a/tests/Predis/Connection/Replication/SentinelReplicationTest.php +++ b/tests/Predis/Connection/Replication/SentinelReplicationTest.php @@ -12,7 +12,6 @@ namespace Predis\Connection\Replication; -use Exception; use PHPUnit\Framework\MockObject\MockObject; use Predis\Command; use Predis\Connection; @@ -1335,7 +1334,7 @@ class SentinelReplicationTest extends PredisTestCase $this->isRedisCommand('GET', ['key']) ) ->willThrowException( - new Exception('message') + new Connection\ConnectionException($slave1, 'message') ); $slave2 = $this->getMockConnection('tcp://127.0.0.1:6383?role=slave'); @@ -1920,6 +1919,93 @@ class SentinelReplicationTest extends PredisTestCase $this->assertSame($slave2, $replication->getCurrent()); } + /** + * @group disconnected + */ + public function testDoesNotRetryOnNonCommunicationException(): void + { + $master = $this->getMockConnection('tcp://127.0.0.1:6381?role=master'); + $master + ->expects($this->any()) + ->method('isConnected') + ->willReturn(true); + $master + ->expects($this->once()) + ->method('executeCommand') + ->with($this->isRedisCommand('SET', ['key', 'value'])) + ->willThrowException(new RuntimeException('Non-communication error')); + + $sentinel = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?role=sentinel'); + + /** @var Connection\FactoryInterface|MockObject */ + $factory = $this->getMockBuilder('Predis\Connection\FactoryInterface')->getMock(); + + $replication = $this->getReplicationConnection('svc', [$sentinel], $factory); + $replication->add($master); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Non-communication error'); + + $replication->executeCommand(Command\RawCommand::create('set', 'key', 'value')); + } + + /** + * @group disconnected + */ + public function testRetriesOnlyOnCommunicationException(): void + { + $master1 = $this->getMockConnection('tcp://127.0.0.1:6381?role=master'); + $master1 + ->expects($this->any()) + ->method('isConnected') + ->willReturn(true); + $master1 + ->expects($this->once()) + ->method('executeCommand') + ->with($this->isRedisCommand('SET', ['key', 'value'])) + ->willThrowException(new Connection\ConnectionException($master1, 'Connection failed')); + $master1 + ->expects($this->once()) + ->method('disconnect'); + + $master2 = $this->getMockConnection('tcp://127.0.0.1:6382?role=master'); + $master2 + ->expects($this->any()) + ->method('isConnected') + ->willReturn(true); + $master2 + ->expects($this->once()) + ->method('executeCommand') + ->with($this->isRedisCommand('SET', ['key', 'value'])) + ->willReturn('OK'); + + $sentinel = $this->getMockSentinelConnection('tcp://127.0.0.1:5381?role=sentinel'); + $sentinel + ->expects($this->once()) + ->method('executeCommand') + ->with($this->isRedisCommand('SENTINEL', ['get-master-addr-by-name', 'svc'])) + ->willReturn(['127.0.0.1', '6382']); + + /** @var Connection\FactoryInterface|MockObject */ + $factory = $this->getMockBuilder('Predis\Connection\FactoryInterface')->getMock(); + $factory + ->expects($this->once()) + ->method('create') + ->with([ + 'host' => '127.0.0.1', + 'port' => '6382', + 'role' => 'master', + ]) + ->willReturn($master2); + + $replication = $this->getReplicationConnection('svc', [$sentinel], $factory); + $replication->add($master1); + + $this->assertSame('OK', $replication->executeCommand( + Command\RawCommand::create('set', 'key', 'value') + )); + } + /** * @medium * @group disconnected