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

fix(typescript-client): Replace the while(true) in createFetchWithBackoff with a run closure + setTimeout #2366

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

samwillis
Copy link
Contributor

This replaces the while (true) with an explicit call to a run closure. It should fix the issue seeing in #2364 but will probably need testing on an actual iOS device to try and replicate.

iOS Safari tabs goes into a kind of hibernated mode when they are in the background. This throttles setTimeouts, and appears to cancel the while (true) task. By replacing the while (true) with an explicit call to a run closure in a setTimeout I'm hopeful we just see the next tick of the recursive timeout be throttled, paused and then restarted when the tab come back to the foreground.

@KyleAMathews
Copy link
Contributor

@samwillis thanks for picking this up! Perhaps just push up little app somewhere for testing? I can borrow my wife's iphone (or son's tablet haha) and other's can test as well.

@KyleAMathews
Copy link
Contributor

Next step is to reproduce original behavior on IOS & that this PR fixes it.

@balegas balegas requested a review from kevin-dp March 6, 2025 13:22
@kevin-dp
Copy link
Contributor

kevin-dp commented Mar 6, 2025

Next step is to reproduce original behavior on IOS & that this PR fixes it.

I've been able to reproduce the original behavior with Safari on my iPhone.
Sometimes i have to try multiple times for this to happen but when it happens only reloading the page works.

Copy link
Contributor

@kevin-dp kevin-dp left a comment

Choose a reason for hiding this comment

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

The fix looks good to me. I'll try to test this on iOS before we merge this.

}
}
}

return run()
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we don't await anything inside the async function that is returned on L50 (only within the nested run function) we can remove the async keyword on L50.

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