Skip to content

Comments

GH-249: adding RetryAfterError FeignException type to map 429 http response from Adobe APIs#250

Closed
francoisledroff wants to merge 1 commit intomainfrom
GH-249
Closed

GH-249: adding RetryAfterError FeignException type to map 429 http response from Adobe APIs#250
francoisledroff wants to merge 1 commit intomainfrom
GH-249

Conversation

@francoisledroff
Copy link
Collaborator

@francoisledroff francoisledroff commented Sep 17, 2025 •
edited
Loading

Description

This PR adds support for throwing a RetryAfterError FeignException when we get 429 from Adobe APIs allowing clients to handle the exception and retry accordingly

Related Issue

Motivation and Context

How Has This Been Tested?

Locally

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@shikhartanwar
GH-249 : Throw RetryAfterError exception when we get 429 from upstr…
c79d2d3 
…eam service allowing clients to handle the exception and retry accordingly (#248)

This PR adds support for throwing a `RetryAfterError` `FeignException` when we get `429` from Adobe APIs allowing clients to handle the exception and retry accordingly

## Related Issue

* #249
* somewhat related #150
francoisledroff
francoisledroff commented Sep 17, 2025
View reviewed changes
core/src/main/java/com/adobe/aio/exception/feign/RetryAfterError.java
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class RetryAfterError extends FeignException {
Copy link
Collaborator Author

@francoisledroff francoisledroff Sep 17, 2025

Choose a reason for hiding this comment

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

actually @shikhartanwar we should use the RetryableException instead here
to leverage the Feign retryer see #151

Copy link
Collaborator Author

@francoisledroff francoisledroff Sep 17, 2025

Choose a reason for hiding this comment

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

The default implementation of ErrorDecoder only creates a RetryableExeception instance when the response contains the “Retry-After” header

Copy link
Member

@shikhartanwar shikhartanwar Sep 17, 2025

Choose a reason for hiding this comment

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

But when I checked in the E2E logs it was always an upstream error crafted by us

Copy link
Collaborator Author

@francoisledroff francoisledroff Sep 17, 2025

Choose a reason for hiding this comment

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

yes we had a bug in our ErrorDecoder I worked on a fix here : #251

@francoisledroff
Copy link
Collaborator Author

francoisledroff commented Sep 17, 2025

closing it in favor of #251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shikhartanwar shikhartanwar shikhartanwar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@francoisledroff @shikhartanwar