honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
18.32k stars 515 forks source link

RPC client does not pass extra arguments, like headers, when using a websocket #2996

Open NuroDev opened 2 months ago

NuroDev commented 2 months ago

What version of Hono are you using?

4.4.3

What runtime/platform is your app running on?

Bun

What steps can reproduce the bug?

import { Hono } from 'hono';
import { createBunWebSocket } from 'hono/bun';
import { hc } from 'hono/client';

const { upgradeWebSocket, websocket } = createBunWebSocket();

const app = new Hono()
  .use('*', async (c, next) => {
    console.log({
      Authorization: c.req.header('Authorization'), // undefined
    });

    await next();
  })
  .get(
    '/ws',
    upgradeWebSocket(() => ({
      onMessage(event, ws) {
        console.log(`Message from client: ${event.data}`);
        ws.send('Hello from server!');
      },
      onClose: () => {
        console.log('Connection closed');
      },
    })),
  );

const server = Bun.serve({
  fetch: app.fetch,
  websocket,
});

const client = hc<typeof app>(server.url.href, {
  headers: {
    Authorization: 'Bearer 123',
  },
});

const socket = client.ws.$ws();

await new Promise((resolve) => setTimeout(resolve, 500));

socket.close();
server.stop();

What is the expected behavior?

Based on the example above: c.req.header('Authorization') should return 'Bearer 123'.

What do you see instead?

Based on the example above: c.req.header('Authorization') instead returns undefined.

Additional information

I of course did a quick look into this and this line seems to be the cause. We just need to pass args as a 2nd argument when creating the new websocket instance.

At a basic level the const args = ... could just be moved up & used for both req.fetch(...) and new WebSocket(...) but I don't know if there is any issue with exposing all arguments when creating the new websocket instance, or if it should be limited to just headers?

nakasyou commented 2 months ago

Hello @NuroDev

Currently, WebSocket API in JavaScript doesn't support sending headers. WebSocket constructor supports only URL and protocol. RPC Client uses WebSocket API, so I think we can't resolve it. However writing about it to docs is good idea.

Reference:

nakasyou commented 2 months ago

Oh, sorry, I missed a fact. WebSocket in Bun accepts headers.

NuroDev commented 2 months ago

Interesting, good point @nakasyou. After a bit more digging I found this issue created 7 years asking for this to become a standard. However Bun seems to have gone ahead and added support for it anyway.