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

Client pings Server every 1s in case of connection-lost #1989

Closed
quriosapien opened this issue Feb 12, 2021 · 9 comments
Closed

Client pings Server every 1s in case of connection-lost #1989

quriosapien opened this issue Feb 12, 2021 · 9 comments

Comments

@quriosapien
Copy link

quriosapien commented Feb 12, 2021

Is your feature request related to a problem? Please describe.
Problem message - https://discord.com/channels/804011606160703521/804011606160703524/809370571040882709
The client pings the server every 1000ms (1s) in case of a connection lost.
It leads to a situation like this -
Screenshot 2021-02-11 at 3 54 53 PM

Describe the solution you'd like
We can delay the ping made to the server in case of a web-socket connection lost.
With every failed ping, we can increase the time it waits for the next ping.

Describe alternatives you've considered
We can show a error message in browser itself and let user click on a button to reload. Thus we can avoid pinging server every 1s. This would be a very manual effort though.

Additional context
I guess, this is where we need to modify our code.
vitejs-code

@sod
Copy link
Contributor

sod commented Feb 12, 2021

could also use https://developer.mozilla.org/de/docs/Web/API/Page_Visibility_API to dial back the aggressiveness of checking when in the background, but try to pick up faster if in the foreground.

@yyx990803
Copy link
Member

What is really wrong with this? I mean, it logs an error, but is there really any problem this causes?

@quriosapien
Copy link
Author

What is really wrong with this? I mean, it logs an error, but is there really any problem this causes?

I won't say it's wrong. The code on the client tries to reconnect to the Server, which is good.

But then, as in my case, I closed the Server but the tab in the browser was still open. When I saw the tab again later (in the evening), it was still trying to connect to the server and was ping(ing) backend every 1 second to check for a successful web-socket connection. It was unnecessary code running every second.

I think we can improve this. Instead of checking it every 1 second forever, we can track the no of failed attempts and increase the interval of retry. A similar example is how GMAIL tries to connect back to the server. @sod suggested another technique that would even enhance the delay in retry in the case when the page is in the background.

@airhorns
Copy link
Contributor

I think exponential backoff will give a poor developer experience where the page won't reload in a timely manner when the server does come back. I think that's the point of this reload code -- to have the browser respond to stuff you might be doing in a terminal so you don't have to manually refresh it all the time. If there's a big interval between checks, then the developer will probably beat vite back to the page and the whole feature rendered useless. For this reason I wouldn't call it unnecessary code.

It's also worth noting this only happens in development, it doesn't generate any load in actual production.

@yyx990803
Copy link
Member

The point of this retry is so that when you kill and restart the vite server you don't have to manually reload the page. If it doesn't do it promptly then there's not much point in doing it in the first place.

I don't really see a strong argument against this since the page won't be in working state when Vite is disconnected anyway, and errors are going once the page reloads. It is also pinging a dedicated endpoint that shouldn't cause dev server load.

@jcwatson11
Copy link

When vite is hosted behind a proxy (apache reverse proxy, for example), the ping process causes the page to entirely reload every second. This is more problematic. While under normal circumstances, I would say this isn't a problem, the reverse proxy situation is common and valid as a use case.

@Shinigami92
Copy link
Member

Proposal: What about providing an option for this that takes for example a function / callback as a strategy to wait X seconds for the next attempt. The default is the current behavior.

@braebo
Copy link
Contributor

braebo commented Jun 17, 2021

When using VSCode-Liveshare, this causes a page-refresh every second or so, making it unusable. Is there a way to disable / override this?

@github-actions
Copy link

This issue has been locked since it has been closed for more than 14 days.

If you have found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest Vite version. If you have any other comments you should join the chat at Vite Land or create a new discussion.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants