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

Messages are dispatched while microtask queue is not empty #2216

Closed
1 task done
OrKoN opened this issue Apr 10, 2024 · 6 comments · Fixed by #2218
Closed
1 task done

Messages are dispatched while microtask queue is not empty #2216

OrKoN opened this issue Apr 10, 2024 · 6 comments · Fixed by #2218

Comments

@OrKoN
Copy link

OrKoN commented Apr 10, 2024

Is there an existing issue for this?

  • I've searched for any related issues and avoided creating a duplicate issue.

Description

I have previously reported #2159 which was fixed but it looks like the solution might not have been complete.

With the latest version of ws the microtasks scheduled in the event callback are handled first but the microtasks created by those microtasks are not. This is different from the WebSocket client behavior in the browser.

Server code:

import { WebSocketServer } from 'ws';

const wss = new WebSocketServer({ port: 8080 });

wss.on('connection', function connection(ws) {
  ws.send('something1');
  ws.send('something2');
});

Client code:

import { WebSocket } from 'ws';

const ws = new WebSocket('ws:https://127.0.0.1:8080');

ws.addEventListener('message', function message(event) {
    console.log('received: %s', event.data);
    new Promise(resolve => {
      resolve()
    }).then(() => {
      console.log('microtask', event.data.toString())
      return Promise.resolve().then(() => {
        console.log('microtask-nested', event.data.toString());
      });
    })
});

ws version

8.16.0

Node.js Version

v20.10.0

System

System:
OS: macOS 14.4.1
CPU: (10) arm64 Apple M1 Max
Memory: 12.88 GB / 64.00 GB
Shell: 5.9 - /bin/zsh

Expected result

received: something1
microtask something1
microtask-nested something1
received: something2
microtask something2
microtask-nested something2

Actual result

received: something1
microtask something1
received: something2
microtask-nested something1
microtask something2
microtask-nested something2

Attachments

No response

@lpinca
Copy link
Member

lpinca commented Apr 10, 2024

That's because this is what happens under the hood.

queueMicrotask(function () {
  console.log('received: something1');
  Promise.resolve().then(() => {
    console.log('microtask something1');
    return Promise.resolve().then(() => {
      console.log('microtask-nested something1');
    });
  });
  queueMicrotask(function () {
    console.log('received: something2');
    Promise.resolve().then(() => {
      console.log('microtask something2');
      return Promise.resolve().then(() => {
        console.log('microtask-nested something2');
      });
    });
  });
});

We can get the expected behavior by resuming the parser when the microtask queue drains, for example by using setImmediate() instead of queueMicrotask(), but in that case we don't have one event per microtask but one event per loop iteration.

@OrKoN
Copy link
Author

OrKoN commented Apr 10, 2024

The WebSocket spec defines one event to be dispatched per task and not per microtask so setImmediate sounds appropriate. I am not sure if that degrades the performance though.

@lpinca
Copy link
Member

lpinca commented Apr 10, 2024

Isn't a task in that context/spec a microtask?

I am not sure if that degrades the performance though.

It almost certainly does but there is the allowSynchronousEvents option now. Anyway setImmediate() might also have unintended side effects. I have to think about it.

@OrKoN
Copy link
Author

OrKoN commented Apr 10, 2024

It is a task but it is on a different queue, a microtask queue, in the html spec. The spec also says that a microtask queue is not a task queue so I am not sure. I think people also refer to tasks/global tasks as macro tasks as opposed to micro tasks.

@OrKoN
Copy link
Author

OrKoN commented Apr 10, 2024

https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-task definitely puts a task on a task queue and not a microtask queue though.

@lpinca
Copy link
Member

lpinca commented Apr 10, 2024

It looks like browsers do not dispatch one event per microtask so I guess I misunderstood the spec in #2160.

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 a pull request may close this issue.

2 participants