Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Provide ability to turn off the default retry-once policy and recommend retry policy #2877

Closed
rymeskar opened this issue Sep 8, 2021 · 3 comments · Fixed by #3608
Assignees
Labels
Feature Request ICM This issue has a corresponding ICM, either for our team or another. R9 Internal 1P
Milestone

Comments

@rymeskar
Copy link

rymeskar commented Sep 8, 2021

By default, MSAL retries http calls once on 5xx with a delay of 1 second.

Scenario
In expert and high traffic scenarios, customers might want to provide own retry strategies, retry periods and resilient http client strategies (circuit breaker).
The behavior within MSAL then intercepts with the logic that the clients might want to inject through the WithHttpClientFactory configuration.

Proposal
Provide a knob to turn off the retry-once behavior of MSAL.

Note: see larger issue tracking this: #3561

Design

While we could add a complex extensiblity point such as the one described here, we recognize that retry libraries like Poly work perfectly fine at HttpClient level and Msal already exposes this layer. So the proposal is to add an overload to WithHttpClientFactory:

WithHttpClientFactory(IMsalHttpClientFactory factory, bool disableDefaultRetryPolicy)

We should provide docs on how to add retry policy for HTTP.

@bgavrilMS
Copy link
Member

CC @trwalke

@bgavrilMS bgavrilMS added this to the 4.46.0 milestone Jul 13, 2022
@bgavrilMS
Copy link
Member

We will remove the default retry policy completely, as there are concenrns about retry storms. Apps should deal with retries.

@pmaytak pmaytak modified the milestones: 4.46.0, 4.47.0 Jul 29, 2022
@bgavrilMS bgavrilMS changed the title [Feature Request] Provide ability to turn off the default retry-once policy [Feature Request] Provide ability to turn off the default retry-once policy and recommend retry policy Aug 5, 2022
@bgavrilMS bgavrilMS added the ICM This issue has a corresponding ICM, either for our team or another. label Aug 5, 2022
@bgavrilMS bgavrilMS mentioned this issue Aug 5, 2022
2 tasks
@trwalke trwalke self-assigned this Aug 10, 2022
@trwalke trwalke linked a pull request Aug 31, 2022 that will close this issue
1 task
@trwalke trwalke mentioned this issue Sep 16, 2022
1 task
@gladjohn
Copy link
Contributor

MSAL 4.47.0 has been released, This issue is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request ICM This issue has a corresponding ICM, either for our team or another. R9 Internal 1P
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants