Skip to content

Conversation

@erdimaden
Copy link
Contributor

@erdimaden erdimaden commented Jul 22, 2024

What changed? Why?

Adding auto-retry functionality for failed GET requests

Qualified Impact

retryCondition: (error: AxiosError) => {
return (
error.config?.method?.toUpperCase() === "GET" &&
(error.response?.status || 0) in [500, 501, 502, 503, 504]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can add more if we need

Copy link
Contributor

@alex-stone alex-stone Aug 1, 2024

Choose a reason for hiding this comment

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

501 not implemented seems like not worth retrying?

500 feels borderline, since that would indicate that our service is explicitly throwing that error. However it's likely that there are a number of retryable errors that bubble up to a 500 that we need to squash first.

I think it's fine to leave 500 for now, but I would probably drop 501 from this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alex-stone sounds good, I updated.

@erdimaden erdimaden changed the title Adding auto-retry for failed HTTP calls Adding auto-retry for failed HTTP GET calls Jul 22, 2024
@erdimaden erdimaden changed the title Adding auto-retry for failed HTTP GET calls Adding auto-retry for failed GET requests Jul 22, 2024
@erdimaden erdimaden changed the title Adding auto-retry for failed GET requests Adding retry for failed GET requests Jul 22, 2024
Base automatically changed from v0.0.10 to master July 23, 2024 22:06
@erdimaden erdimaden force-pushed the feat/network-retry branch from d7e3f1b to f5c01ed Compare August 1, 2024 16:45
@cb-heimdall
Copy link

cb-heimdall commented Aug 1, 2024

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@erdimaden erdimaden merged commit dc2191c into master Aug 5, 2024
@erdimaden erdimaden deleted the feat/network-retry branch August 5, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants