Repository navigation
fix(flags): do not retry a refused flags connection - #1180
Conversation
A refused connection is not transient, so flags() now raises it on the first attempt instead of retrying it on the backoff schedule. Timeouts, resets, DNS and TLS failures are still retried.
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review.
A timeout raised while the application handles its own refused connection was treated as a refusal and not retried.
|
thanks @luantaraschi |
SDK performance: posthog-pythonBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
💡 Motivation and Context
flags()retries on anyrequests.exceptions.ConnectionError.requestsraises that same class for a refused connection (ConnectionRefusedErrorwrapped in urllib3'sNewConnectionErrorandMaxRetryError), so when nothing is listening on the host, the request is sent again after 300 ms (and again after 600 ms withmax_retries=2) before the error reaches the caller.The HTTP client spec in PostHog/sdk-specs treats a refused connection as non-transient: the SDK should not retry it and should surface the flag error right away (
acceptance/private/http-client.feature, "exactly 1 feature flag request should be sent").compliance/posthog-python.mdlists this as gap n18.This change looks for a
ConnectionRefusedErrorbehind the transport error before retrying, without following unrelated exceptions the application was handling, and re-raises when it finds one. Timeouts, resets, DNS and TLS failures and 502/504 responses are still retried.💚 How did you test it?
test_flags_does_not_retry_refused_connectionbinds and closes a local socket, then callsflags()against that port withmax_retries=2. On main it sends 3 requests; with the fix it sends 1 and never sleeps. The existing retry tests still pass.ruff format --checkandruff check(0.12.2) are clean.python:3.12-slimcontainer: 6052 passed, no failures.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code with Claude Opus 5.5 found this while checking posthog-python against the HTTP client requirements in PostHog/sdk-specs, reproduced it against a closed local port, and wrote the fix and the test. Luan directed the work.