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

retry get mode until success #191

Merged
merged 2 commits into from
Nov 27, 2023

Conversation

ReiHashimoto
Copy link

I've added retry calling api to get mode.
Please give me your opinion on any potential problems or better ways to implement this.

Comment on lines 36 to 38
new Promise((resolve) =>
setTimeout(resolve, 1000 * 2 ** attempts),
).then(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ReiHashimoto I think if we do it like this, when the server fails, it will take a very long time to make api calls

Copy link
Author

Choose a reason for hiding this comment

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

@sanglevinh Thank you for your review.
I replaced it to use constant waiting time.
Does it look fine?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ReiHashimoto Yes, I feel like this is quite reasonable

Copy link
Author

Choose a reason for hiding this comment

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

@sanglevinh Thank you.
Can I merge this PR to your feature/mode_standalone branch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ReiHashimoto yes, I removed the
if (res.data === undefined) {
return getModeStandaloneApi()
}
you can merge it

@ReiHashimoto ReiHashimoto merged commit 9e4ed8f into feature/mode_standalone Nov 27, 2023
1 check passed
@ReiHashimoto ReiHashimoto deleted the feature/mode_standalone_retry branch November 27, 2023 09:18
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.

2 participants