Skip to content

Conversation

@ridgey-dev
Copy link
Contributor

@ridgey-dev ridgey-dev commented Jul 16, 2025

In v2.x, failures from stream_socket_client() would result in a CommunicationException, which is caught and retried by SentinelReplication.

In v3.x, such failures now throw a StreamInitException, which is not a subclass of CommunicationException, and thus bypasses the existing retry logic.

Change
Replaces the catch (CommunicationException $e) block with catch (Throwable $e)

Maintains existing retry behavior, but broadens the scope to handle all exceptions thrown during connection attempts, including StreamInitException and any future unexpected errors.

Justification
Brings retry handling in SentinelReplication in line with the logic already adopted in RedisCluster (#788)

@ridgey-dev ridgey-dev requested a review from a team as a code owner July 16, 2025 15:03
@ridgey-dev ridgey-dev changed the title Retry SentinelReplication::retryCommandOnFailure on all classes with Throwable interface Retry all exceptions from Sentinel replicas Jul 16, 2025
@coveralls
Copy link

coveralls commented Jul 16, 2025

Coverage Status

coverage: 93.089% (+0.03%) from 93.064%
when pulling bb57396 on ridgey-dev:sentinel-replication-stream-init-exception
into c60003a on predis:main.

@vladvildanov
Copy link
Contributor

@ridgey-dev Oh, thanks for your contribution! LGTM

@vladvildanov vladvildanov merged commit 8e7afbe into predis:main Jul 18, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants