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

Awaiting in the upgradeWebSocket callback prevents the websocket events from being registered correctly #954

Open
Rick-Phoenix opened this issue Feb 1, 2025 · 6 comments · May be fixed by #959
Labels

Comments

@Rick-Phoenix
Copy link

What version of Hono are you using?

4.6.15

What runtime/platform is your app running on? (with version if possible)

Node 22

What steps can reproduce the bug?

  • Use the upgradeWebSocket helper on a route
  • Await anything inside of it
    (Even this will cause the same result)
await new Promise((resolve) => {
          setTimeout(() => {
            resolve(`Delay`);
          }, 0);
        });

What is the expected behavior?

  • The promises inside the callback should resolve and the websocket events should be registered normally

What do you see instead?

  • The connection is completed but events don't get registered (more details below)

Additional information

Awaiting in the callback seems to mess with the order of operations.
When not awaiting anything (whether the callback is async or not doesn't matter), the order of operation as seen in the debugger is this:
(the functions referenced here come from index.mjs in the @hono/node-ws package)

  1. createEvents (the callback given to the upgradeWebSocket helper) is awaited
  2. The callback returns
  3. nodeUpgradeWebSocket is awaited
  4. wss.handleUpgrade runs (so wss.emit 'connection' runs)
  5. Events are created (starting with events.onOpen?.(new Event("open"), ctx))

While awaiting, the order changes and looks like this

  1. createEvents (the callback given to the upgradeWebSocket helper) is awaited
  2. The promise inside the callback is awaited (so the callback hasn't returned the events yet)
  3. wss.handleUpgrade with 'emit connection' gets called right after, even if no events have been returned yet
  4. Now the callback returns the events
  5. nodeUpgradeWebSocket is awaited
  6. Events do not get created and attached to the webSocket, so the webSocket connection does complete, but with no events attached.

Moving the awaiting inside one of the events (onOpen, for example) or even earlier with a preceding middleware works fine.

@Rick-Phoenix
Copy link
Author

Just realized I opened this in the Hono repository and not the middleware repo. Sorry about that. If you can move it to there somehow let me know, otherwise I'll close it and reopen it there.

@yusukebe
Copy link
Member

yusukebe commented Feb 2, 2025

Hi @Rick-Phoenix

Does that mean this is @hono/node-ws's problem?

@Rick-Phoenix
Copy link
Author

Indeed @yusukebe

@yusukebe yusukebe transferred this issue from honojs/hono Feb 3, 2025
@yusukebe
Copy link
Member

yusukebe commented Feb 3, 2025

@Rick-Phoenix I transferred the issue to honojs/middleware.

@yusukebe
Copy link
Member

yusukebe commented Feb 3, 2025

Hi @nakasyou

Do you have time to look at it?

@nakasyou nakasyou linked a pull request Feb 5, 2025 that will close this issue
4 tasks
@nakasyou
Copy link
Contributor

nakasyou commented Feb 5, 2025

@yusukebe, the problem fixed in #959. Could you check this?

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

Successfully merging a pull request may close this issue.

3 participants