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

WebSockets In Bun #2696

Closed
NicoPlyley opened this issue May 17, 2024 · 5 comments
Closed

WebSockets In Bun #2696

NicoPlyley opened this issue May 17, 2024 · 5 comments
Labels

Comments

@NicoPlyley
Copy link
Contributor

What version of Hono are you using?

4.3.7

What runtime/platform is your app running on?

Bun

What steps can reproduce the bug?

If you Bun.serve and return app.fetch() it will not allow Bun to switch protocols, making web sockets unusable. The following is an example of that

Bun.serve({
  fetch(req, server) {
    return app.fetch(req, { ip: server.requestIP(req) }) // This return prevents protocol switching
  },
  websocket,
})

Now that is expected behavior in Bun, so here is how Bun says it should be done instead:

const server = Bun.serve{
  fetch(req, server) {
    const success = server.upgrade(req);
    if (success) {
      // Bun automatically returns a 101 Switching Protocols
      // if the upgrade succeeds
      return undefined;
    }

    // handle HTTP request normally
    return app.fetch(req, { ip: server.requestIP(req) })
  },
  websocket,
  },
});

console.log(`Listening on ${server.hostname}:${server.port}`);

What is the expected behavior?

No response

What do you see instead?

This appears to be because since the server.upgrade was already ran, it is causing issues.
Here is the error that is received:

43 |       if (websocketListeners.onOpen) {
44 |         websocketListeners.onOpen(new Event("open"), createWSContext(ws));
45 |       }
46 |     },
47 |     close(ws, code, reason) {
48 |       const websocketListeners = websocketConns[ws.data.connId];
                                                     ^
TypeError: undefined is not an object (evaluating 'ws.data.connId')
      at close (/hono-test/node_modules/hono/dist/adapter/bun/websocket.js:48:49)
      at fetch (/hono-test/src/index.ts:41:16)
37 |       await next();
38 |     };
39 |   };
40 |   const websocket = {
41 |     open(ws) {
42 |       const websocketListeners = websocketConns[ws.data.connId];
                                                     ^
TypeError: undefined is not an object (evaluating 'ws.data.connId')
      at open (/hono-test/node_modules/hono/dist/adapter/bun/websocket.js:42:49)
      at fetch (/hono-test/src/index.ts:41:16)

22 |   const websocketConns = [];
23 |   const upgradeWebSocket = (createEvents) => {
24 |     return async (c, next) => {
25 |       const server = c.env;
26 |       const connId = websocketConns.push(await createEvents(c)) - 1;
27 |       const upgradeResult = server.upgrade(c.req.raw, {
                                 ^
TypeError: undefined is not an object (evaluating 'server.upgrade')
      at /hono-test/node_modules/hono/dist/adapter/bun/websocket.js:27:29

Additional information

I've had two different people mention this issue, so I decided to investigate further. I think it is important and reasonable, for a user to need to access the IP, or set any env and be able to still use websockets

@NicoPlyley NicoPlyley added the bug label May 17, 2024
@yusukebe
Copy link
Member

@NicoPlyley Thank you for raising the issue.

@nakasyou Could you see this?

@nakasyou
Copy link
Contributor

Hi @NicoPlyley, thank you for sending.

I think it's the same cause as this: #2645.

If WebSocket helper will support { server: server } like #2595 (comment), this problem may be resolved.

@nakasyou
Copy link
Contributor

@NicoPlyley
You can use

return app.fetch(req, { ip: server.requestIP(req), server })

to resolve it.

@NicoPlyley
Copy link
Contributor Author

The new connInfo should solve it too, correct? I'm basing my issue based off what people on discord have had issues with

@nakasyou
Copy link
Contributor

@NicoPlyley
When you use ConnInfo helper, you can get conninfo.
As you said, ConnInfo Helper also supports

return app.fetch(req, { ip: server.requestIP(req), server })

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

No branches or pull requests

3 participants