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

WebSocket race condition causing Miniflare not to send messages #88

Closed
TimTinkers opened this issue Nov 8, 2021 · 3 comments
Closed
Labels
bug Something isn't working fixed-in-next Fixed in next release

Comments

@TimTinkers
Copy link

I have code for a WebSocket client that looks like this:

let connection = new WebSocket(EVENT_LISTENER_URL, [ 'protocol' ]);
connection.addEventListener('open', async (event) => {
  await sleep(180); // <- Miniflare does not actually send the message without this line
  connection.send(JSON.stringify({ type: 'REQUEST_ID' })); // this doesn't actually get sent
}

From logging data coming into the Durable Object, I was able to figure out that Miniflare didn't send some of my messages in response to a WebSocket open event without a forced delay of at least 180ms.

@mrbbot
Copy link
Contributor

mrbbot commented Nov 11, 2021

Hey! 👋 This is interesting. Miniflare queues messages if you haven't called accept yet, so should dispatch all messages it receives. Any chance you're calling accept() after addEventListener("message", (e) => { ... }) in your worker?

@TimTinkers
Copy link
Author

Any chance you're calling accept() after addEventListener...

Nope.

      // Create a new WebSocket connection to the caller.
      const [ client, server ] = Object.values(new WebSocketPair());
      server.accept();
      // ...
      server.addEventListener('message', async (event) => { 
        // ...
      });

Definitely calling accept first.

@mrbbot mrbbot added bug Something isn't working fixed-in-next Fixed in next release labels Nov 13, 2021
@mrbbot
Copy link
Contributor

mrbbot commented Nov 13, 2021

Hey! 👋 miniflare@2.0.0-rc.1 has just been released, including a fix for this. You can find the changelog here. Thanks for reporting this, and please let me know if you have any other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed-in-next Fixed in next release
Projects
None yet
Development

No branches or pull requests

2 participants