search

vitejs / vite

Next generation frontend tooling. It's fast!
http://vitejs.dev
MIT License
67.06k stars 6.02k forks source link

Exponential Backoff for Failed Pings #16361

Open w0pp opened 5 months ago

w0pp commented 5 months ago

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:

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 https://github.com/vitejs/vite/issues/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 commented 4 months ago

When we closed https://github.com/vitejs/vite/issues/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.

w0pp commented 4 months ago

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 https://github.com/vitejs/vite/issues/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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.