Skip to content

Fix sentinel discovery methods not catching StreamInitException#1650

Merged
vladvildanov merged 3 commits intopredis:mainfrom
wkerschbaumer:fix/sentinel-discovery-catch-throwable
Mar 5, 2026
Merged

Fix sentinel discovery methods not catching StreamInitException#1650
vladvildanov merged 3 commits intopredis:mainfrom
wkerschbaumer:fix/sentinel-discovery-catch-throwable

Conversation

@wkerschbaumer
Copy link
Copy Markdown
Contributor

Summary

  • getMaster(), getSlaves(), and updateSentinels() only catch ConnectionException when connecting to Sentinel nodes. StreamInitException (thrown by stream_socket_client() failures in StreamFactory) extends PredisException directly — not ConnectionException — so it propagates uncaught, preventing fallback to the next Sentinel node.
  • This causes complete application failure when any single Sentinel is unreachable, even if other Sentinels are healthy. Every PHP worker gets stuck in the retryCommandOnFailure() loop retrying the same dead Sentinel.
  • Adds StreamInitException to the catch clause via union type in all three methods. This is more precise than catching PredisException or Throwable, which would also swallow ServerException (e.g. ERR No such master with that name) and mask configuration errors.

Related: #1577 applied a similar fix to retryCommandOnFailure() but missed these three sentinel discovery methods.

Exception hierarchy context

PredisException
  ├── CommunicationException
  │     └── ConnectionException      ← caught before this fix
  ├── StreamInitException            ← NOT caught (thrown by stream_socket_client failure)
  └── ServerException                ← should NOT be caught (config/protocol errors)

How to reproduce

  1. Configure Predis with 3 Sentinel nodes
  2. Make one Sentinel unreachable at the network level (e.g. blackholed IP, NIC disconnect — not a clean process shutdown)
  3. stream_socket_client() to the dead Sentinel throws StreamInitException
  4. getMaster() / getSlaves() / updateSentinels() don't catch it → no fallback to the next Sentinel
  5. retryCommandOnFailure() catches it, but retries hit the same dead Sentinel every time

Test plan

  • Existing Sentinel test suite passes
  • Verify StreamInitException during sentinel discovery triggers fallback to next sentinel
  • Verify ServerException (e.g. wrong service name) still propagates without retry

The getMaster(), getSlaves(), and updateSentinels() methods only catch
ConnectionException when connecting to Sentinel nodes. StreamInitException
(thrown by stream_socket_client() failures) extends PredisException directly,
not ConnectionException, so it propagates uncaught — preventing fallback to
the next Sentinel node.

This causes complete application failure when any single Sentinel is
unreachable, even if other Sentinels are healthy.

Add StreamInitException to the catch clause using a union type in all three
methods. This is more precise than catching PredisException, which would
also swallow ServerException (e.g. "ERR No such master with that name")
and mask configuration errors.

See also predis#1577 which applied a similar fix to retryCommandOnFailure().
@wkerschbaumer wkerschbaumer requested a review from a team as a code owner February 27, 2026 12:30
- Remove spaces around | in union catch (php-cs-fixer)
- Add CHANGELOG.md entry for predis#1650
@vladvildanov
Copy link
Copy Markdown
Contributor

@wkerschbaumer Hey! We're good to go with this PR as soon as you'll fix coding standards issue. Most likely it would be solved by just calling composer style:fix

@vladvildanov vladvildanov merged commit d0f6671 into predis:main Mar 5, 2026
36 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.

3 participants