Skip to content

fix: Retry flags only on transport errors#708

Draft
marandaneto wants to merge 2 commits into
PostHog:mainfrom
marandaneto:pi-flags-network-retry-20260628
Draft

fix: Retry flags only on transport errors#708
marandaneto wants to merge 2 commits into
PostHog:mainfrom
marandaneto:pi-flags-network-retry-20260628

Conversation

@marandaneto

Copy link
Copy Markdown
Member

💡 Motivation and Context

Align Python /flags/?v=2 retry behavior with the other SDK fixes: retry only bounded network/transport/timeout failures, and do not retry HTTP/API status responses like 408, 429, or 5xx.

This keeps transient network handling while avoiding repeated requests when PostHog has already returned an API response.

💚 How did you test it?

  • uv run --extra test --extra dev ruff check posthog/request.py posthog/client.py posthog/__init__.py posthog/test/test_request.py posthog/test/test_client.py
  • uv run --extra test --extra dev python .github/scripts/check_public_api.py
  • uv run --extra test pytest posthog/test/test_request.py posthog/test/test_client.py

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Implemented with pi. The retry policy was changed from urllib3 status-code retries to explicit /flags transport retries, with default max retries set to 1, 0 disabling retries, and exponential backoff starting at 300ms.

@marandaneto marandaneto self-assigned this Jun 28, 2026
@greptile-apps

greptile-apps Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "fix: retry flags only on transport error..." | Re-trigger Greptile

Comment thread posthog/client.py Outdated
Comment thread posthog/test/test_client.py Outdated
Comment thread posthog/request.py
Comment on lines -44 to -45
# Status codes that indicate transient server errors worth retrying
RETRY_STATUS_FORCELIST = [408, 500, 502, 503, 504]

@marandaneto marandaneto Jun 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

idk if that was intentional and why not all the other sdks do that, so i am assuming we should remove it and just retry for transient errors which is what all sdks should do
if all sdks should retry for those http errors as well, then i'd port this to all the other sdks

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@PostHog/team-feature-flags ?

@marandaneto marandaneto requested a review from a team June 28, 2026 14:46
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.

1 participant