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

setTimeout to emit drain in websocket.js:175 causes following message delayed 1s when tab in background #649

Closed
zyf0330 opened this issue Dec 17, 2020 · 6 comments
Labels
question Further information is requested

Comments

@zyf0330
Copy link

zyf0330 commented Dec 17, 2020

You want to:

  • [] report a bug
  • [x ] request a feature

Current behaviour

When using Chrome or Safari and putting tab into background, timer is delayed to ensure at least 1s interval. And it causes that
later message is delayed 1s.

Steps to reproduce (if the current behaviour is a bug)

  1. write a simple client which sends two messages to server every 1s and run in browser
  2. put browser tab into background
  3. put it back into foreground

Expected behaviour

sent message is not delayed

Setup

  • OS: Ubuntu
  • browser: Chrome
  • engine.io-client version: 4.0.5

Other information (e.g. stacktraces, related issues, suggestions how to fix)

I try to remove this setTimeout but connection is disconnected. So I want to know if setTimeout can be removed?

@darrachequesne
Copy link
Member

Hi! Here's what I have been able to reproduce:

// client-side
const socket = io();

let count = 0;

setInterval(() => {
  socket.emit("test", ++count, new Date());
}, 1000);

// server-side
io.on("connection", (socket) => {
  socket.on("test", (count, date) => {
    console.log(count, date.substring(11), new Date().toISOString().substring(11));
  })
});
7 21:33:19.556Z 21:33:19.558Z
8 21:33:20.556Z 21:33:20.558Z
# put browser tab to background
9 21:33:22.385Z 21:33:22.386Z
10 21:33:23.385Z 21:33:23.386Z
11 21:33:24.385Z 21:33:24.386Z

Happens on Chrome (87.0.4280.88) and Firefox (84.0) / Ubuntu.

Is this the behavior you are describing? In that case, since the timestamp is the same on the client and on the server, I'd say the issue comes from the setInterval in the test, which is delayed when the tab is put to background.

@darrachequesne darrachequesne added the question Further information is requested label Jan 13, 2021
@zyf0330
Copy link
Author

zyf0330 commented Jan 14, 2021

Yes, this is the case.
Delay is caused by setTimeout here

setTimeout(function() {
.
I want to know if it can be optimized.

@hyperlink
Copy link

Hello!

We've ran into a similar time throttling issue where the server or the client will get ping timeout because of a missed heartbeat. We're still on socket.io v2 but looking at the code I think the newer version could suffer from the same issue.

From the client perspective, after the tab has been backgrounded for around 5 minutes the setTimeout of zero in the write method does not get fired until many, many seconds later resulting in the ping packet not being sent to the server in time before the client times out (the timeout timer is started before the packet is sent not when the packet is flushed). The ping packet gets stuck in the buffer because the writable was not set to true. I suspect the new version will have the same issue but instead of ping, the response will not be sent back to the server in time.

Our workaround is to change the setTimeout to a Promise.resolve().then() which will still block writes until the next tick without Chrome throttling the timer to death. We have good results so far.

We've also had a hard time reproducing this issue because every time we tried to create a small test the timer does not seem to throttle. I suspect Chrome only aggressively throttles on tabs that are consuming higher resources.

@andrewaustin
Copy link

@darrachequesne With Chrome now throttling setTimeout calls and other browsers soon following suit, can the setTimeout be removed? Why is it useful here?

@darrachequesne
Copy link
Member

The setTimeout() call seems to be useful though, because the CI fails for 32702ee.

But I don't mind removing it if you find an elegant way to do so 👍

For reference, it was added in f3710ff.

darrachequesne pushed a commit that referenced this issue Jun 22, 2021
An immediate setTimeout was used to unblock the WebSocket write.
Unfortunately, this setTimeout can be throttled by browsers when the
tab is backgrounded.

This can lead to missed pong responses to server pings, which
eventually leads to disconnection.

Related: #649
@darrachequesne
Copy link
Member

This should be fixed by f30a10b, included in engine.io-client@5.1.2.

Big thanks to @hyperlink for his PR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants