Skip to content

Fix retry logic for POST requests#119

Merged
hownowstephen merged 3 commits intomainfrom
MESS-674_fix-retry-post-requests
May 7, 2026
Merged

Fix retry logic for POST requests#119
hownowstephen merged 3 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
Changes HTTP retry behavior and success-status handling for all client requests, which can affect error handling and timing in production; risk is mitigated by targeted unit tests.

Overview
Fixes ClientBase request handling to treat any 2xx response as success (instead of only 200) and to re-raise CustomerIOException directly so API 4xx/validation errors aren’t wrapped as retry failures.

Updates the underlying Retry configuration to actually retry POST (and all methods) and to retry on transient 5xx responses via status_forcelist, with new unit tests validating the retry config and the updated status/exception behavior.

Reviewed by Cursor Bugbot for commit 4ea47ee. 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.
@hownowstephen hownowstephen merged commit ca084bc into main May 7, 2026
10 checks passed
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

2 participants