fix(connection): fail fast on unrecoverable connect errors (#4026)

[sean-kim05]

Summary

Addresses the narrow, separable problem I raised in #4026: definitively-unrecoverable connection errors are retried with full backoff. The intentional handshake-phase retry behavior (clarified by @petyaslavova in that thread) is left completely untouched.

Since 7.1.0, Connection.connect() wraps the entire connect + handshake in the retry policy. During socket establishment every OSError is re-wrapped into a generic ConnectionError; because the policy's supported errors include ConnectionError, these are retried up to DEFAULT_RETRY_COUNT (10) times with exponential backoff — even for errors that can never succeed on retry, e.g. a Unix-domain-socket ENOENT (the socket path simply doesn't exist). The wrap also discarded the original errno, so nothing downstream could distinguish a permanent failure from a transient one.

Net effect: Redis(unix_socket_path=..., socket_connect_timeout=2) against a missing socket spends seconds sleeping/retrying, and the common "try UDS, fall back to TCP" pattern becomes very slow.

Change

Uses the existing is_retryable hook in Retry.call_with_retry that connect() simply wasn't passing:

1. Preserve the original error — the OSError → ConnectionError wrap now uses raise ... from e, so the original errno survives for classification.
2. Pass an is_retryable predicate from connect() that returns False for a conservative set of unrecoverable socket errnos (currently just ENOENT). Those fail fast; transient errors such as ECONNREFUSED during server startup keep retrying exactly as before.

Applied to both the sync (redis/connection.py) and async (redis/asyncio/connection.py) stacks.

Tests

• test_connect_without_retry_on_unrecoverable_oserror (sync + async): ENOENT → single attempt, original OSError preserved as __cause__.
• test_connect_retries_on_transient_oserror (sync + async): ECONNREFUSED → still retried (3 attempts with retries=2).

Open questions (carried over from #4026)

Kept deliberately conservative — happy to adjust to your preference:

1. Scope of "unrecoverable": just ENOENT for now — should it also include EACCES / EAFNOSUPPORT / EPROTONOSUPPORT?
2. socket_connect_timeout as a total-time cap: left out of scope here (a broader change carrying a deadline across retries); could be a follow-up.
3. Default vs opt-in: implemented as the default; happy to gate behind a flag if preferred.

Refs #4026.

---

Note

Low Risk
Narrow connect-path retry behavior change with conservative errno list and tests; handshake retry logic is unchanged.

Overview
Connect failures that cannot succeed on retry (e.g. missing Unix socket, ENOENT) now fail immediately instead of burning the default retry count and exponential backoff.

Sync and async Connection.connect() pass is_retryable=self._is_retryable_connect_error into the existing Retry.call_with_retry hook. The predicate treats errors as non-retryable when the raised ConnectionError chains an OSError whose errno is in _UNRETRYABLE_CONNECT_ERRNOS (currently ENOENT only). ECONNREFUSED and other transient errors keep retrying as before.

OSError handling during connect now uses raise ConnectionError(...) from e so the original errno is available on __cause__ for that classification. Tests cover one-shot failure on ENOENT and full retries on ECONNREFUSED.

^{Reviewed by Cursor Bugbot for commit 3459a64. Bugbot is set up for automated code reviews on this repo. Configure here.}

[petyaslavova]

Hi @sean-kim05, thank you for your contribution! I'll review it shortly.

[Sanjays2402]

ENOENT is transient for a Unix socket while the server is starting: the path does not exist until the server binds it. This makes an explicitly configured retry stop after one attempt in that common startup race; should this behavior be opt-in rather than overriding every caller’s Retry policy?

[petyaslavova]

Thanks for the careful work on this, and for keeping the handshake-retry behavior intact.

Before we merge, I'd like to apply some changes on the design toward making this configurable on the Retry object rather than hardcoding a non-retryable errno set as default behavior in the connection layer.

Concretely:

1. Promote the original error up to the retry logic. Keep the raise ConnectionError(...) from e change so the root cause (and its errno) is available, and please make cause preservation consistent across the retryable wrap sites in connection.py (the socket read/write wraps currently build ConnectionError from e.args without from e, so __cause__ is lost there). Any cause-inspecting checker needs the root error to actually be reachable.

2. Add a new optional property on the Retry object — a custom "is this error retryable?" checker. When set, it is applied on top of the existing supported-errors check and composed as an AND: an error is retried only if it is one of the supported error types AND the custom checker returns True. When it is not set, behavior is unchanged.

3. Make it opt-in. We don't want to change the default retry behavior; the ENOENT/unrecoverable-errno logic should be something a user configures via this checker (we can ship it as a reusable helper), not an always-on default. This also means the classification then applies uniformly to every retry site that uses the Retry object, including the lazy reconnect path, instead of only the explicit connect() call.

Please mirror this in both the sync and async Retry/connection stacks, and add regression tests that fail on base and pass on the branch — including a real (non-mock) missing-socket path. One scoping note: this addresses the ENOENT case, not the original #4026 report (an unavailable TCP server / socket_connect_timeout as a total cap), so let's keep #4026 open and retitle this PR accordingly. Once the checker lives on the Retry object and composes as described, this should be ready for another review.