Skip to content

Add policy to handle Retry-After response headers#1097

Merged
ansasaki merged 6 commits intokeylime:masterfrom
sarroutbi:202509011440-add-retry-after-policy
Sep 3, 2025
Merged

Add policy to handle Retry-After response headers#1097
ansasaki merged 6 commits intokeylime:masterfrom
sarroutbi:202509011440-add-retry-after-policy

Conversation

@sarroutbi
Copy link
Copy Markdown
Contributor

@sarroutbi sarroutbi commented Sep 2, 2025

This pull request enhances the ResilientClient by adding support for the standard Retry-After HTTP response header.

Currently, our client relies solely on an exponential backoff strategy for all retryable errors. While effective, this approach is less precise when a server explicitly indicates how long we should wait before trying again (e.g., for rate limiting or temporary maintenance).

This change introduces a dedicated middleware that respects the Retry-After header, making our client a better HTTP citizen and improving its ability to recover from transient server-side issues efficiently.

Co-Authored-By: Gemini noreply@google.com
Co-Authored-By: Claude noreply@anthropic.com
Signed-off-by: Sergio Arroutbi sarroutb@redhat.com

@sarroutbi sarroutbi force-pushed the 202509011440-add-retry-after-policy branch from 4315b9f to 7b6bacb Compare September 2, 2025 13:03
@sarroutbi sarroutbi marked this pull request as draft September 2, 2025 13:04
@sarroutbi sarroutbi marked this pull request as ready for review September 2, 2025 14:20
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 2, 2025

Codecov Report

❌ Patch coverage is 65.45455% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.12%. Comparing base (07463b8) to head (e5a51e7).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
keylime/src/resilient_client.rs 65.45% 19 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 58.12% <65.45%> (+0.08%) ⬆️
upstream-unit-tests 58.12% <65.45%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
keylime/src/resilient_client.rs 58.00% <65.45%> (+5.47%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread Cargo.toml Outdated
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
- Added a maximum retry attempt limit (MAX_RETRY_AFTER_ATTEMPTS = 5)
to prevent infinite retries

Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
This change refactors the retry mechanism to eliminate the separate,
hardcoded retry limit for Retry-After responses. Previously, the client
used two different values for maximum retries: a constant for the
RetryAfterMiddleware and a parameter for the exponential backoff policy.
This could lead to confusing and inconsistent behavior.

Now, the RetryAfterMiddleware is initialized with the same max_retries
value provided to the ResilientClient constructor, ensuring that both
retry strategies (Retry-After headers and exponential backoff) respect a
single, unified configuration.

Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
@sarroutbi sarroutbi force-pushed the 202509011440-add-retry-after-policy branch from b666abc to e5a51e7 Compare September 2, 2025 17:19
@sarroutbi sarroutbi requested a review from ansasaki September 2, 2025 17:19
Copy link
Copy Markdown
Contributor

@ansasaki ansasaki left a comment

Choose a reason for hiding this comment

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

LGTM!

@ansasaki ansasaki merged commit fa7c59a into keylime:master Sep 3, 2025
14 checks passed
@sarroutbi sarroutbi deleted the 202509011440-add-retry-after-policy branch September 3, 2025 11:27
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.

3 participants