honojs/hono

Middlewares cause `TypeError: Headers are immutable.` if not added to the api after an endpoint using upgradeWebSocket.

Closed this issue · 7 comments

Discussed in https://github.com/orgs/honojs/discussions/2534

Originally posted by jonasfroeller April 20, 2024
I protected my route group with the jwt middleware and I am using upgradeWebSocket.

If i send a request to the websocket endpoint with a Authorization: Bearer header I get this error and my api crashes:

TypeError: Headers are immutable.
    at Headers.delete (ext:deno_fetch/20_headers.js:336:13)
    at Context.set res (https://deno.land/x/hono@v4.2.5/context.ts:189:31)
    at dispatch (https://deno.land/x/hono@v4.2.5/compose.ts:60:20)
    at eventLoopTick (ext:core/01_core.js:168:7)
    at async https://deno.land/x/hono@v4.2.5/hono-base.ts:356:25
    at async ext:deno_http/00_serve.js:455:18
Upgrade response was not returned from callback

I found out, that the jwtMiddleware doesn't seem to work with cookies and the cors middleware, the logger and poweredBy as well as .route crashed the websocket endpoint with the TypeError: Headers are immutable. error. WHY?

I put the middlewares and the route group under the websocket endpoint, and it works now:

app.get('/chat/:monitor_id', upgradeWebSocket((c) => { // HAS TO BE ABOVE, causes `TypeError: Headers are immutable.` if not
  const monitor_id = c.req.param('monitor_id')

  return {
    onMessage(event, ws) {
      ws.send(event.data.toString() + " in " + monitor_id)
    },
    onClose: () => {
      console.log('Connection closed')
    },
  }
}))

app.use('*', logger(), poweredBy(), cors())
app.route("", home)

minimal repro

import { Hono } from "https://deno.land/x/hono@v4.2.5/mod.ts";
import { upgradeWebSocket } from "https://deno.land/x/hono@v4.2.5/helper.ts";
import { cors } from "https://deno.land/x/hono@v4.2.5/middleware.ts";

const app = new Hono();

app.use(cors());
app.get("/", upgradeWebSocket(c => ({})));

Deno.serve(app.fetch);

Send a WebSocket request to localhost:8000

related: #1102

NOTE:
Since the response has already been given headers by the cors middleware, it seems that if you try to give headers such as sec-websocket-accept by Deno.updradeWebSocket afterwards, an error is thrown in the middle of the handler.
As a result, I think it is hung because the response is not returned even though the upgrade should have been done.

Hi.

I didn't investigate but only saw the code though. We may have to clone the request object here:

const { response, socket } = Deno.upgradeWebSocket(c.req.raw)

Hi, I tried this, but the same error occurred.

--- a/src/adapter/deno/websocket.ts
+++ b/src/adapter/deno/websocket.ts
@@ -5,6 +5,7 @@ export const upgradeWebSocket: UpgradeWebSocket = (createEvents) => async (c, ne
     return await next()
   }
   const { response, socket } = Deno.upgradeWebSocket(c.req.raw)
+  const editableResponse = response.clone()
 
   const events = await createEvents(c)
 
@@ -26,5 +27,5 @@ export const upgradeWebSocket: UpgradeWebSocket = (createEvents) => async (c, ne
   socket.onclose = (evt) => events.onClose && events.onClose(evt, wsContext)
   socket.onerror = (evt) => events.onError && events.onError(evt, wsContext)
 
-  return response
+  return editableResponse
 }

Hmm. I've struggled, but this is a difficult issue. It will take some time to fix it.

It is currently impossible to set a value in the header of a response from a WebSocket endpoint. So, we cannot use middleware such as CORS. However, as noted in #1102, CORS is not required.

Hmm. I've struggled, but this is a difficult issue. It will take some time to fix it.

Documenting it in the docs would be good, so that others know, if they do not know a lot about how WebSockets work, as I did.
I think it does not have to be fixed, because it is not really a bug, just something you have to think about, when structuring the routes. If you just put the WebSocket routes above all other routes, and middlewares, everything works.