Skip to content

Fix retry logic for POST requests#119

Open
hownowstephen wants to merge 2 commits intomainfrom
MESS-674_fix-retry-post-requests
Open

Fix retry logic for POST requests#119
hownowstephen wants to merge 2 commits intomainfrom
MESS-674_fix-retry-post-requests

Conversation

@hownowstephen
Copy link
Copy Markdown
Contributor

@hownowstephen hownowstephen commented May 7, 2026

Summary

  • Enable retries for POST requests: urllib3's Retry defaults exclude POST from allowed_methods, so all SDK retries were silently no-ops. Sets allowed_methods=None to retry all HTTP methods.
  • Retry on server errors: Adds status_forcelist=[500, 502, 503, 504] so transient 5xx responses trigger retries.
  • Accept all 2xx status codes: Previously only 200 was accepted; 201, 202, 204 etc. raised exceptions.
  • Fix misleading error wrapping: 4xx errors from the API were caught by the generic except Exception and wrapped in "Failed after N retries" — now CustomerIOException is re-raised directly.

Context

This is the most-reported issue in the repo (#76), active since 2021 with users hitting ConnectionResetError in production. The root cause was identified by @skion: retry parameters were decorative for POST.

Closes #76

Test plan

  • test_retry_config_allows_post — verifies allowed_methods=None and status_forcelist on built session
  • test_non_200_raises_without_retry_wrapper — 400 errors raise clean CustomerIOException without "retries" noise
  • test_2xx_status_codes_accepted — 200/201/202/204 all pass through
  • Full test suite passes (make test — 31 tests)
  • Lint passes (make lint)

Note

Medium Risk
Touches core HTTP request/retry behavior; incorrect retry semantics or status handling could change how clients react to transient failures or non-200 successes.

Overview
Fixes ClientBase request handling so all 2xx responses are treated as success (not just 200) and CustomerIOException from non-2xx responses is no longer wrapped in a misleading "failed after N retries" message.

Updates the underlying urllib3 Retry configuration to actually retry POST/other methods (allowed_methods=None) and to retry on selected transient server errors via status_forcelist (500/502/503/504), with new unit tests covering retry config, non-2xx exception behavior, and acceptance of 201/202/204.

Reviewed by Cursor Bugbot for commit 4c97261. Bugbot is set up for automated code reviews on this repo. Configure here.

urllib3's Retry defaults only retry idempotent methods, so POST requests
(used by all SDK methods) were never actually retried despite the
configurable retries parameter. This adds allowed_methods=None to retry
on all HTTP methods, adds status_forcelist for 5xx server errors, and
fixes error handling so 4xx responses aren't wrapped in a misleading
"after N retries" message.

Closes #76
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3c52872. Configure here.

Comment thread customerio/client_base.py Outdated
With raise_on_status=False, exhausted 5xx retries silently return the
response instead of raising MaxRetryError, so the "Failed after N
retries" context message is never shown for server errors. The default
(True) is correct: 4xx codes aren't in status_forcelist so they always
return a response regardless, and 5xx exhaustion properly raises.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed to receive valid reponse after 3 retries

1 participant