redis-connections: two connection failures that present as silence, not as an error - #54
Open
alberttwong wants to merge 1 commit into
Open
alberttwong wants to merge 1 commit into
alberttwong wants to merge 1 commit into
Conversation
`timeouts.md` is about making a broken connection fail fast. Two failures get past a correctly configured timeout and produce no useful diagnostic at all, and neither is mentioned anywhere in the skills today — `rediss` and `ECONNRESET` appear in none of them. **1. The wrong scheme against a TLS-only endpoint.** A managed endpoint that requires TLS does not answer a plaintext connection with a TLS error. It resets it, or accepts the socket and stays quiet. Measured against one endpoint seconds apart: `redis://` → ECONNRESET, `rediss://` → PONG. `ECONNRESET` reads as a network fault, so the next moves are firewall rules, DNS and the allowlist, none of which is the problem. This cost us two separate afternoons, the second time because a comment in our own code asserted the endpoint was plaintext — it had been, until it wasn't. **2. A retry policy with no ceiling.** `socket_connect_timeout` bounds one attempt, not how many attempts are made. Where reconnection retries indefinitely — the default for `node-redis`'s pool — the call that establishes the connection never settles and never throws, and there is no timeout to tune because no single operation timed out. In a test suite this stops the run mid-way with no failing assertion and no stack; it is indistinguishable from a slow test and has to be killed. Adds a section to `timeouts.md` with both symptoms, the bounded-retry fix in node-redis and redis-py, and one line in the quick reference, since "set a timeout" is exactly the advice someone will have already followed before hitting either of these. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
skills/redis-connections/references/timeouts.md— one added section. Plus one paragraph inSKILL.md§5, because "set an explicit timeout" is precisely the advice someone will have already followed before hitting either of these.Why
timeouts.mdexists so a broken connection fails. Two failures get past a correctly configured timeout and produce no useful diagnostic at all. Neither is mentioned anywhere in the skills today —redissandECONNRESETappear in none of the eight.1. The wrong URL scheme against a TLS-only endpoint
A managed endpoint that requires TLS does not reject a plaintext connection with a TLS error. It resets it, or accepts the socket and never answers. Measured against the same endpoint, seconds apart:
ECONNRESETreads as a network fault, so the natural next moves are firewall rules, DNS, and the allowlist — none of which is the problem, and nothing in the error mentions TLS.This cost us two separate afternoons. The second one because a comment in our own code asserted the endpoint was plaintext. It had been, until it wasn't — so the section says plainly that an endpoint can become TLS-only without a change on your side, and a comment claiming otherwise has a shelf life.
2. A retry policy with no ceiling turns a dead endpoint into a hang
socket_connect_timeoutbounds one attempt. It does not bound how many attempts are made. Where the reconnect policy retries indefinitely — the default fornode-redis's pool — the call that establishes the connection never settles and never throws. There is no timeout to tune, because no single operation timed out.In a test suite this is especially unpleasant: a
before/setUphook that never settles stops the run mid-way with no failing assertion and no stack, and has to be killed. It is indistinguishable from a slow test.The two also compound. A TLS scheme mismatch is a connection failure, so an unbounded retry policy turns symptom 1 into symptom 2 — which is exactly how we misdiagnosed it twice in one afternoon, first as blocked DNS in a sandbox and then as something else entirely. Both present as the identical silence.
What was added
rediss://for Redis Cloud and any TLS-enabled endpoint, inredis-pyandnode-redis, and check the scheme first when a connection fails with a reset or a hang.reconnectStrategyreturning anErrorat a ceiling (node-redis), and a boundedRetry(ExponentialBackoff(), retries=5)(redis-py).Validation
skills/edited,plugins/redis-development/skills/regenerated withnpm run sync:plugins, nothing hand-edited underplugins/.Evals — not run
Same position as #53, stated rather than glossed: this is a meaningful content change and per CONTRIBUTING it should carry evals. The suite needs
ANTHROPIC_SKILL_CREATOR_PATHand theclaudeCLI, andevals/redis-connections/connections/model-matrix.jsonis 3 models × 2 configurations × 2 repetitions plus aclaude-sonnet-5judge — real spend I was not in a position to authorize on this account.Happy to run it and attach the "Against Baseline" screenshots given a sanctioned budget, or for a maintainer to run it.
Manual validation instead
redis://vsrediss://behaviour is measured, on a Redis Cloud endpoint, in both directions — that is where the two lines above come from rather than from docs.reconnectStrategywith a ceiling was added, after which the same condition fails in ~4s with a message naming the scheme.redis-pysnippets are the documented APIs (Retry,ExponentialBackoff,retry_on_error) but I measured only thenode-redisside; worth a reviewer's eye on the Python one specifically.🤖 Generated with Claude Code
Note
Low Risk
Documentation-only skill content; no application or library code paths change.
Overview
Expands the redis-connections timeout guidance so readers know socket timeouts alone do not guarantee a visible failure.
SKILL.md §5 now warns that plaintext
redis://against TLS-only endpoints can show up asECONNRESET(not a TLS hint), and that unlimited reconnect retries can hang forever with no per-operation timeout to trip.references/timeouts.md adds a section on two failures that present as silence: wrong URL scheme vs TLS-only Redis Cloud (
rediss://in redis-py and node-redis), and unbounded reconnect policies (with boundedreconnectStrategy/Retryexamples). The same edits land inskills/and the vendoredplugins/redis-development/skills/copy.Reviewed by Cursor Bugbot for commit fa68b4c. Bugbot is set up for automated code reviews on this repo. Configure here.