Skip to content

Conversation

@yaythomas
Copy link
Member

Description of changes:

Fix retry strategy to only apply default error patterns when neither retryable_errors nor retryable_error_types is specified, matching the behavior of the JS/TS SDK. Previously, specifying only one filter would still apply defaults, causing unintended retry behavior.

Also fix critical jitter calculation bug where jitter was being added to base delay instead of replacing it, effectively doubling delays.

  • Change RetryStrategyConfig fields to None defaults
  • Apply default patterns only when both filters are None
  • Rename compute_jitter to apply_jitter for clarity
  • Fix apply_jitter to return final delay not additive amount
  • Update test expectations to match corrected jitter behavior
  • Add missing math import to waits.py

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Fix retry strategy to only apply default error patterns when neither
retryable_errors nor retryable_error_types is specified, matching the
behavior of the JS/TS SDK. Previously, specifying only one filter would
still apply defaults, causing unintended retry behavior.

Also fix critical jitter calculation bug where jitter was being added
to base delay instead of replacing it, effectively doubling delays.

- Change RetryStrategyConfig fields to None defaults
- Apply default patterns only when both filters are None
- Rename compute_jitter to apply_jitter for clarity
- Fix apply_jitter to return final delay not additive amount
- Update test expectations to match corrected jitter behavior
- Add missing math import to waits.py
Prevent ruff fmt errors since experimental is on
@yaythomas yaythomas requested a review from wangyb-A as a code owner November 27, 2025 19:28
@yaythomas yaythomas changed the title Retries optional not set fix: retries optional not set Nov 27, 2025
@yaythomas yaythomas closed this Nov 27, 2025
@yaythomas yaythomas reopened this Nov 27, 2025
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Just one small comment, but lgtm.

ParidelPooya
ParidelPooya previously approved these changes Nov 27, 2025
wangyb-A
wangyb-A previously approved these changes Nov 27, 2025
@yaythomas yaythomas dismissed stale reviews from wangyb-A and ParidelPooya via 588bb44 November 27, 2025 19:42
@yaythomas yaythomas merged commit 4e28a5e into main Nov 27, 2025
14 of 15 checks passed
@yaythomas yaythomas deleted the retries-optional-not-set branch November 27, 2025 19:47
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.

4 participants