WebSocket usage with worktop

[universse]

I'm using worktop@0.8.0-next.8 with miniflare@2.0.0-rc.. I got this error when trying to connect to ws route. Could you take a look? I understand that I'm using unstable releases.

GET /ws 200 OK (1.74ms)
[mf:err] TypeError: Web Socket request did not return status 101 Switching Protocols response with Web Socket

This is index.js.

import { Router } from 'worktop'
import { listen } from 'worktop/ws'
import { reply } from 'worktop/module'

const API = new Router()

API.add(
  `GET`,
  `/ws`,
  listen((req, ctx, socket) => {
    console.log('connected')
  })
)

export default reply(API.run)

Same error when not using worktop/ws.

import { Router } from 'worktop'
import { reply } from 'worktop/module'

const API = new Router()

API.add(`GET`, `/ws`, async (req, ctx) => {
  const upgradeHeader = req.headers.get(`Upgrade`)

  if (upgradeHeader !== `websocket`) {
    return new Response(`Expected websocket`, { status: 400 })
  }

  const [client, server] = Object.values(new WebSocketPair())

  server.accept()

  return new Response(null, {
    status: 101,
    webSocket: client,
  })
})

export default reply(API.run)

The minimal ws example works fine.

async function handleRequest(request) {
  try {
    const url = new URL(request.url)
    switch (url.pathname) {
      case `/`:
        return new Response(`hello`)

      case `/ws`:
        const upgradeHeader = request.headers.get(`Upgrade`)

        if (upgradeHeader !== `websocket`) {
          return new Response(`Expected websocket`, { status: 400 })
        }

        const [client, server] = Object.values(new WebSocketPair())
        server.accept()

        return new Response(null, {
          status: 101,
          webSocket: client,
        })

      default:
        return new Response(`Not found`, { status: 404 })
    }
  } catch (err) {
    return new Response(err.toString())
  }
}

export default {
  async fetch(request: Request, env) {
    try {
      return await handleRequest(request, env)
    } catch (e) {
      return new Response(`${e}`)
    }
  },
}

I'm also using esbuild with below config, if that is useful.

import { build } from 'esbuild'

build({
  entryPoints: [`src/index.js`],
  outfile: `dist/index.mjs`,

  bundle: true,
  charset: `utf8`,
  format: `esm`,
  mainFields: [`browser`, `module`, `main`],
  platform: `neutral`,
  target: `es2020`,

  sourcemap: true,
  minify: true,
}).catch((err) => {
  console.error(err.stack)
  process.exitCode = 1
})

[lukeed]

This is the core of worktop's request handler:

let res;
try {
  res = await handler(request, context);
} catch (err) {
  // ...
} finally {
  res = new Response(res ? res.body : 'OK', res);
  return finalize(res, req.method === 'HEAD');
}

In the finally block, it always creates a new Response instance.

My guess (without having looked at source) is that miniflare's Response class is not looking for & using an existing Response.webSocket property when using a Response as the ResponseInit value.

[lukeed]

Yeah, miniflare's Response class has a private #webSocket property – it's a public readonly in the runtime.

And here's the part where our Response is only looking for a .webSocket property on a ResponseInit value: https://github.com/cloudflare/miniflare/blob/v2/packages/core/src/standards/http.ts#L377-L396

[mrbbot]

Hey! 👋 miniflare@2.0.0-rc.3 has just been released, including the fix for this. You can find the changelog here.

[universse]

Hi. Thanks for the update. Now I'm getting this error instead.

[mf:err] GET /api/ws/123: TypeError: Cannot clone a response to a WebSocket 
handshake.
    at Response.clone (file:///.../node_modules/.pnpm/@miniflare+core@2.0.0-rc.3_@miniflare+watcher@2.0.0-rc.3/node_modules/@miniflare/core/src/standards/http.ts:516:13)
    at a (C:\Users\...\dist\index.mjs:156:214)
    at fetch (C:\Users\...\dist\index.mjs:168:61)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)   
    at EventTarget.[kDispatchFetch] (file:///.../node_modules/.pnpm/@miniflare+core@2.0.0-rc.3_@miniflare+watcher@2.0.0-rc.3/node_modules/@miniflare/core/src/standards/event.ts:356:13)
    at file:///.../node_modules/.pnpm/@miniflare+http-server@2.0.0-rc.3_@miniflare+watcher@2.0.0-rc.3/node_modules/@miniflare/http-server/src/index.ts:177:20
    at Server.<anonymous> (file:///.../node_modules/.pnpm/@miniflare+http-server@2.0.0-rc.3_@miniflare+watcher@2.0.0-rc.3/node_modules/@miniflare/http-server/src/index.ts:368:24)
GET /api/ws/123 500 Internal Server Error (136.61ms)
[mf:err] TypeError: Web Socket request did not return status 101 Switching Protocols response with Web Socket     
    at Server.<anonymous> (file:///.../node_modules/.pnpm/@miniflare+http-server@2.0.0-rc.3_@miniflare+watcher@2.0.0-rc.3/node_modules/@miniflare/http-server/src/index.ts:376:11)

[mrbbot]

Hmmm ok 😕, your very first code snippet doesn't throw for me now.

That TypeError (which is also thrown by the real workers runtime) means .clone() is being called on a Response with a webSocket set. Interestingly, workers let you construct a new Response() from an existing Response with a webSocket:

const [webSocket1, webSocket2] = Object.values(new WebSocketPair());

const res1 = new Response(null, {
  status: 101,
  webSocket: webSocket1,
});
const res2 = new Response(null, res1); // ✅

const res3 = new Response(null, {
  status: 101,
  webSocket: webSocket2,
});
const res4 = res3.clone(); // ❌ TypeError: Cannot clone a response to a WebSocket handshake.

Are you able to share your worker script or a minimal reproduction?

[universse]

Can confirm the original script is working. What I changed was importing reply from worktop/cache instead of worktop/module.

import { Router } from 'worktop'
import { reply } from 'worktop/cache'
import { listen } from 'worktop/ws'

const API = new Router()

API.add(
  `GET`,
  `/ws`,
  listen((req, ctx, socket) => {
    console.log(`connected`)
  })
)

export default reply(API.run)

Looks like the response is cloned here https://github.com/lukeed/worktop/blob/next/packages/worktop/src/cache.ts#L24.

Thanks for taking a look. I will file an issue there.