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

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

Open
NuroDev opened this issue Jun 19, 2024 · 3 comments
Labels

Comments

@NuroDev
Copy link
Contributor

NuroDev commented Jun 19, 2024

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?

@NuroDev NuroDev added the bug label Jun 19, 2024
@nakasyou
Copy link
Contributor

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
Copy link
Contributor

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

@NuroDev
Copy link
Contributor Author

NuroDev commented Jun 20, 2024

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.

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

2 participants