honojs / hono

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

Example at https://hono.dev/helpers/websocket#websocket-helper doesn't work with deno. #2423

Closed unworthyEnzyme closed 3 months ago

unworthyEnzyme commented 3 months ago

What version of Hono are you using?

2.0.2

What runtime/platform is your app running on?

Deno

What steps can reproduce the bug?

Just run the example code:

import { Hono } from "https://deno.land/x/hono@v2.0.2/mod.ts";
import { upgradeWebSocket } from "https://deno.land/x/hono@v4.1.4/helper.ts";

const app = new Hono();

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

Deno.serve(app.fetch);

And try to connect it in a browser console. const socket = new WebSocket('ws://localhost:8000/')

What is the expected behavior?

I expect the client to connect to the server.

What do you see instead?

I get a ts error like this:

No overload matches this call.
  The last overload gave the following error.
    Argument of type 'string' is not assignable to parameter of type 'Handler<string, Env>'.

and when I try to connect to it I get a runtime error like this.

TypeError: Cannot read properties of undefined (reading 'Symbol(request)')
    at toInnerRequest (ext:deno_fetch/23_request.js:560:17)
    at Object.upgradeWebSocket (ext:deno_http/01_http.js:410:17)
    at https://deno.land/x/hono@v4.1.4/adapter/deno/websocket.ts:7:37
    at dispatch (https://deno.land/x/hono@v2.0.2/compose.ts:28:30)
    at https://deno.land/x/hono@v2.0.2/compose.ts:12:12
    at Hono.dispatch (https://deno.land/x/hono@v2.0.2/hono.ts:182:23)
    at fetch (https://deno.land/x/hono@v2.0.2/hono.ts:203:17)
    at ext:deno_http/00_serve.js:455:24
    at ext:deno_http/00_serve.js:689:29
    at eventLoopTick (ext:core/01_core.js:169:7)

Additional information

OS: Windows 11

Deno version:

yusukebe commented 3 months ago

Hi @unworthyEnzyme

@nakasyou, Could you take a look at this issue?

nakasyou commented 3 months ago

Hi @unworthyEnzyme, thank you for writing Issue. I couldn't reenactment this errors, I got an onerror on Browser. But I have an idea to resolve.

You tried this code:

const socket = new WebSocket('ws://localhost:8000/')

I think you may resolve it if you write code like this:

const socket = new WebSocket('ws://localhost:8000/ws')

Because route of WebSocket is /ws. onerror error is removed if I change my code.

nakasyou commented 3 months ago

Oh, I found the cause that seems to be. You use Hono v2.0.2. WebSocket helper on Deno uses c.req.raw API, but v2.0.2 doesn't have c.req.raw API because c.req.raw has implemented since v3.

If we would like to resolve it, we should support v2 or write "WebSocket helper doesn't support Hono before v3" to docs.

Using different version same package is difficult when we use most package managers such as Bun and npm. But when we use Deno, it is easy.

unworthyEnzyme commented 3 months ago

Hi @nakasyou. Thank you for your answer.

Oh wow, I didn't realize v4.1.4 was out. When I click the vscode Update specifier to it's redirected specifier., it rewrites the version as 2.0.2. It doesn't do the same with the upgradeWebSocket import which is weird. I didn't even noticed the versions of the two imports was different. I wonder why this is the case.

For the solution I think adding a notice to the docs is sufficient since it is quite an old version.

nakasyou commented 3 months ago

Hi @unworthyEnzyme, thank you for your opinion.

Now, the docs includes warnings! https://hono.dev/guides/middleware#built-in-middleware