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

Exponential Backoff for Failed Pings #16361

Open
4 tasks done
w0pp opened this issue Apr 5, 2024 · 5 comments · May be fixed by #19057
Open
4 tasks done

Exponential Backoff for Failed Pings #16361

w0pp opened this issue Apr 5, 2024 · 5 comments · May be fixed by #19057

Comments

@w0pp
Copy link

w0pp commented Apr 5, 2024

Description

There was an issue (#12693) for this created a while ago, but was closed in favor of skipping pinging when the tab is not shown.

I believe that issue should not have been closed, because skipping pinging when the tab is not shown does not solve the problem entirely - when the tab is visible, it still causes the console to fill up, and the tab to start lagging, or it might even crash the tab/browser.
Here are just some of the use cases where the tab remains visible:

  • "Emulate a focused page" is enabled in devtools
  • tab is open on 2nd monitor
  • tab is open and developer leaves the computer

I think Webpack has solved this best in webpack-dev-server: they use exponential backoff with max 10 retries. Here's their implementation: https://github.com/webpack/webpack-dev-server/blob/master/client-src/socket.js
Of course this solution isn't perfect either, but I think the compromise of having to refresh the page after N failed retries is worth not filling up the console and network tab, and potentially causing the tab/browser to lag or crash.

This issue is essentially a duplicate of #12693 but since that was marked as closed and the conversation in the issue is limited to collaborators I don't see how else to go about this.

Suggested solution

Implement exponential backoff with max retries.

Alternative

No response

Additional context

No response

Validations

@bluwy
Copy link
Member

bluwy commented Apr 9, 2024

When we closed #12693, we implicitly agreed that the visbilitychange check is enough rather than needing to implement exponential backoff. So I don't think it's unintentional that the issue had been closed.

Do you experience the lagginess or crash described here?

when the tab is visible, it still causes the console to fill up, and the tab to start lagging, or it might even crash the tab/browser.

Closing this as wontfix for now.

@bluwy bluwy closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2024
@w0pp
Copy link
Author

w0pp commented Apr 9, 2024

Do you experience the lagginess or crash described here?

Yes, if I leave the tab open for a long time while the dev server is stopped, the browser starts lagging when I switch to the tab.

When we closed #12693, we implicitly agreed that the visbilitychange check is enough rather than needing to implement exponential backoff. So I don't think it's unintentional that the issue had been closed.

I understand your point, but I just don't think the issue should be closed, because that PR only solves the issue when the tab is hidden, not when it's visible. There's a better solution available that would solve both cases.

@bluwy
Copy link
Member

bluwy commented Apr 10, 2024

Do you leave the tab open but still visible? I don't quite understand why the browser would lag after you return to it though. The ping isn't doing anything intensive and doesn't generate a lot of garbage. I have two monitors too and have not experienced the issue.

@w0pp
Copy link
Author

w0pp commented Apr 10, 2024

Do you leave the tab open but still visible?

Yes, I leave the Chrome tab with dev tools open.

I don't quite understand why the browser would lag after you return to it though. The ping isn't doing anything intensive and doesn't generate a lot of garbage. I have two monitors too and have not experienced the issue.

Well, it depends on whether you have dev tools open and how long you leave the tab open for. I would assume the lag is due to dev tools having to store a lot of network requests and console logs - it was probably not optimized for such a large number. I even got an out of memory error code once.
I suggest you open a few tabs of an app running on a Vite dev server, open dev tools, enable "Emulate a focused page", and close the dev server. After a few hours you should start to notice the tabs lagging.

@bluwy
Copy link
Member

bluwy commented Apr 11, 2024

I have not thought about the dev tools case and that makes sense that the memory will rise because of it. I'll re-open this then and we could re-consider the feature.

I'm not so sure about exponential backoff since the page could seemingly not refresh itself if the interval is too long. Maybe having the ping happen for max 2 minutes would be good enough, and if the visibilitychange event gets triggered (becomes visible again) we reset the timer.

@bluwy bluwy reopened this Apr 11, 2024
@sapphi-red sapphi-red linked a pull request Dec 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants