When trying to use websockets the connection hangs on trying to connect

[garretcharp]

I was attempting to use WebSockets and whenever using itty-durable the connection hangs on trying to connect. Below is a small little code sample of what I am using to test the WebSockets. When I use it outside the scope of itty-durable (just on a normal route) it works fine.

import { ThrowableRouter } from 'itty-router-extras'
import { createIttyDurable, withDurables } from 'itty-durable'

export class TestWebsocket extends createIttyDurable({ persistOnChange: false }) {
	constructor(state, env) {
		super(state, env)
	}

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

		websocket.accept()

		websocket.send(JSON.stringify({ connected: true }))

		websocket.addEventListener('message', ({ data }) => {
			websocket.send(JSON.stringify({ data }))
		})

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

const router = ThrowableRouter()

router.all('*', withDurables())

// Doesnt work
router.get('/ws', ({ TestWebsocket }) => TestWebsocket.get('test').websocket())

// Works
router.get('/ws2', () => {
	const [client, websocket] = Object.values(new WebSocketPair())

	websocket.accept()

	websocket.send(JSON.stringify({ connected: true }))

	websocket.addEventListener('message', ({ data }) => {
		websocket.send(JSON.stringify({ data }))
	})

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

export default {
	fetch: router.handle
}

[repzip]

Did you get this working?

[TimTinkers]

Dropping by also to see @garretcharp @repzip if either of you had found a solution to this.

[garretcharp]

Hey guys, I did get this working at some point I actually did it without using itty-durable and instead only using itty-router.

Basically in the DO I create an itty router and expose a fetch function that calls the router. Following code probably wont work but a general idea:

import { Router } from 'itty-router'

const router = Router()

router.all('/my/do/route/*', async (req, { MyDo }) => {
  const doId = MyDo.idFromString('<id>')
  return MyDo.get(doId).fetch(req)
})

// DO:
export class MyDo {
  constructor() {
    // do stuff

    this.router = Router()

    this.router.get('/get/ws', request => {
       const [client, websocket] = Object.values(new WebSocketPair())

	websocket.accept()

	websocket.send(JSON.stringify({ connected: true }))

	websocket.addEventListener('message', ({ data }) => {
		websocket.send(JSON.stringify({ data }))
	})

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

  async fetch(request) {
    return this.router.handle(request)
  }
}

[TimTinkers]

Hi @garretcharp, thanks for the tip. Unfortunate that this doesn't work out of the box; makes itty-durable a bit niche for our usage right now. Dropping an itty-router into the durable object was able to remove at least some of the boilerplate.

[kalepail]

Dug into this a bit and it would appear that there's a header issue or maybe just a response issue where the webSocket: client isn't being forwarded when responding. This is either before the pass back or actually in the pass in where the headers.Upgrade: 'websocket' isn't being passed to the method call into the DO.

[kwhitley]

I'm gonna circle back to this... as none of my test scenarios involved websockets. Would love to ensure support for that side of things, just need to do some testing! If anyone has an example repo that I can use as a demo/testbed, that would be a huge help!

In the meantime, the standard (non-socket) v1.x is being wrapped up today, with the final API changes and bug-fixes over the alpha version that's been languishing since launch.

[da7a90-backup]

Would love to contribute to adding web socket support, I feel like this is one of the most powerful and in demand features

[AndrewLester]

Would also like to contribute to make this happen, and before doing any work I'd like to summarize what (I think?) the challenges might be:

• As mentioned above, the Upgrade: websocket header needs to be sent to the DO when making a proxy'd function fetch. In doing this, maybe it makes sense to forward all headers of the current request over to the DO? This is assuming users of itty-durable will write something like DurableObject.connect({ connectionData }), where connect is a user defined DO method that returns a Response with a webSocket property.
• The proxy'd function fetch that you'd get from writing DurableObject.connect({ connectionData }), currently, is only ever a POST request. But, from what I can tell, WebSocket connections MUST be initiated with a GET request. It's weird because on Miniflare (--local) a POST request works just fine, so long as I've got the header. On "non-local" dev mode with wrangler, using a POST request with this setup just crashes wrangler completely. This line in kj might have to do with it? Not sure, maybe I'm just not doing it right. Either way, it seems like it'd be good to use a GET request since that's standard practice.

Going about implementing all this, forwarding all request headers through with the proxy'd function fetch request doesn't seem too difficult IF that's a sensible solution. Maybe forwarding all headers is a bad idea? I'm not sure.

With the POST vs GET issue, it seems like the proxy'd function fetches with POST are pretty ingrained in the system, so breaking out of that for WebSockets could be less pretty. Perhaps the proxyDurable function's get handler could detect the Upgrade: websocket header, and thus perform a GET request instead of a POST? This wouldn't support a request body though, which could instead be turned into URL params and be picked up by the itty-durable DO internal router.

[kwhitley]

I'd love the help on this one, and it's one I'll me short listing myself anyway, so it's perfect timing to get another set of eyes/ideas on tackling this. I too, have run into crash after crash while trying to obtain a socket connection from within the DO.

Personally I had assumed that it didn't even matter that the internal proxied fetches were over POST, as in theory, any function within the DO (even if originally called by POST) could respond to a Worker that originated the request from a client via the proper GET/upgrade request. As in, the Worker would intercept the upgrade request, fire a POST/proxied request to, say a "connect()" function on the DO, which would instantiate a socket pair and respond with the client, which the Worker would then pass along on back to the original client.

• As mentioned above, the Upgrade: websocket header needs to be sent to the DO when making a proxy'd function fetch. In doing this, maybe it makes sense to forward all headers of the current request over to the DO?

I don't see any reason against doing this...

• The proxy'd function fetch that you'd get from writing DurableObject.connect({ connectionData }), currently, is only ever a POST request. But, from what I can tell, WebSocket connections MUST be initiated with a GET request.

Like I described (poorly) above, the Worker can handle the GET/upgrade, and ideally return a socket client from the DO, but i don't think the DO should have to care about any of that... but that's an unsubstantiated guess at this point, as I haven't gotten it working yet. Client<-->Worker socket yes, Client<-->Worker<-->DO not so much.

[AndrewLester]

I was just trying to do some tests on that POST vs. GET thing when sending a WebSocket connection request to a DO and I found some very strange behavior. I have a repro in this GitHub repository here.

The behavior I noticed is that, if the Upgrade: websocket header is present on a request sent to the DO, the request method is automatically set to GET. This only happens on non-local development or prod, that is, if I'm running on Miniflare with --local, a POST request method is still logged. Otherwise, I can only seem to log GET. Again, I may be doing something wrong here. There's also a little bit of other code from what I was testing, so it's not exactly a minimal repro yet...

Maybe this line is actually what I was looking for.

[kwhitley]

Ok, so yesterday I successfully got itty-durable (modified version) to play nicely with, and return a websocket.

A few things to note:

• Confirming: I stand corrected from the previous comment... I set up a simple test, and merely flipping the DO request to a GET (with adjustments to the internal itty-router to receive it and prevent the body from being attached) as opposed to a POST works. TLDR; GET requests to the DO appear to be required to respond with a socket.
• I can also confirm that omitting the upgrade: websocket header from the DO request causes a failure. Original Worker request headers will now be passed along to the DO to remedy.
• As a result, I'm switching up the internal router to only use GET (rather than adding dependent switching logic), attaching the content via a do-content header, rather than the body itself.
• Downstream response/error handling code (catchErrors) within the proxy-durable had to be modified, as a 101 response from within the DO would trigger a !response.ok error.
• The outside Worker code should NOT use the withDurables({ parse: true }) flag, allowing the Response from within the DO to be passed directly back to the original Worker request.

[gishmel]

What's the timeline on this fix landing in this library and potentially itty-router if also relevant? Also follow on question is this equally a problem in itty-router; I had issues with itty-router seemingly not passing through the Upgrade: websocket header between the outside worker or routing worker into the DO worker and thought it is probably related to this issue but wanted to ask to understand better. Thanks for all your work; I love itty-router it's a brilliant library that makes Workers really exciting and terse to accomplish amazing things!

[kwhitley]

@gishmel - I just finished getting it all to work last week, so I just need to backport the itty-durable changes back into the main lib and publish, so hopefully today. No changes necessary in itty-router, which doesn't surprise me, as it's completely return-agnostic (doesn't care if anything returned is a Response or not). The issue was a combination of the following:

• The internal router embedded within the DO was using POST for all method calls as a way to pass arguments/params (serialized and sent across as JSON). Despite being a middleman request, this was a breaking issue, so the internals have been switched to use GET (for all calls), passing the args as a serialized header instead. Effectively no difference at all, but allows webosockets to work.

• The proxyDurable helper (used within the withDurables() middleware was previously building a simple request to the DO from scratch, ignoring all original request headers. In order for the DO to properly pass a websocket back, it needed the upgrade: websocket header as well. To solve this, as well as hopefully enable other future-seeking behaviors, these two helpers now pass the headers from the original Worker request (including the upgrade header) on to the DO through those proxied requests.

The good news?

This should all be 100% transparent in your own itty-durable code, requiring no changes!

[gishmel]

Thanks so much for the extra clarification seems like my problem with itty-router must be self inflicted I thought I was passing back the right headers at the right times but I guess somewhere things aren't propagating correctly. Did you happen to have an example repo where one could see these types of operations working successfully getting an example request from an outside worker and sending it through a Durable Object which returns a websocket client to the outside worker which returns that to the client? I can't help but think maybe I am missing something crucial for websockets specifically since I have no issues with any HTTP related actions or routes? Thanks again for all your work it's been a blast to use itty-* and I look forward to keep using them for a long time to come!

[kwhitley]

I'm trying to work up a simple code example for both a client-Worker socket, and a client-Worker-DO socket. It likely won't be perfect, but i'll publish in advance so you can test the functionality, even if I just need to work out a working example with you here (or on Discord)

[kwhitley]

But it def wasn't your fault - even if you tried to force all the right things with itty-router, the itty-durable layers were preventing any of the right headers from making it through to the DO!

[kwhitley]

Ok, itty-durable@next is released with these changes, so I'd suggest trying it out so I have another set of eyes on it. Unfortunately due to the nature of DO complexity paired with the multi-layer proxy magic of itty-durable, unit tests haven't been tackled like they have in every other lib, making this process a little bit trial-by-fire.

As to the example, I ported some of the example code over to examples/websocket, showing an endpoint that connects a client to a Worker, and other one that lets the DO create the socket instead. These came slightly modified/cleaned up from my working code on OTP Garden, so they hopefully work without too much issue.

router code

import { withDurables } from 'itty-durable'
import { Router } from 'itty-router'
import { error, withParams } from 'itty-router-extras'

// export durable object class, per spec
export * from './Room'

const router = Router()

router
  .all('*', withDurables())

  // GENERAL CLIENT-WORKER SOCKET EXAMPLE
  .get('/connect', (request) => {
    const [client, server] = Object.values(new WebSocketPair())

    server.accept()

    // if a client closes the connection, close it immediately
    server.addEventListener('close', () => {
      server.close()
    })

    // test the connection by sending a delayed message after 2 seconds
    setTimeout(() => {
      server.send('delayed message...')
    }, 2000)

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

  // CLIENT-WORKER-DO SOCKET EXAMPLE
  .get('/room/:id/connect', withParams,
    ({ id, Room }) => Room.get(id).connect()
  )

// CF ES6 module syntax
export default {
  fetch: (request, env, context) => router
                        .handle(request, env, context)
                        .catch((err) => error(err.status || 500, err.message))
}

Durable Object code

import { createDurable } from 'itty-durable'

export class Room extends createDurable({ autoPersist: true }) {
  sockets: any[]

  constructor(state, env) {
    super(state, env)

    this.sockets = []
  }

  // CREATE NEW SOCKET
  connect() {
    const { sockets } = this
    const id = Math.random() // just a random id
    const [client, server] = Object.values(new WebSocketPair())

    server.accept() // immediately accept connection

    // if a client closes the connection, close it immediately
    server.addEventListener('close', () => {
      server.close()
      this.sockets = this.sockets.filter(s => s.id !== id)
    })

    // add socket.server to list of open sockets
    sockets.push({ id, server })

    // send a message to all connected sockets
    this.sendMessage('A new connection has been established!')

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

  sendMessage(message: string) {
    // send a message to all open sockets
    for (const socket of this.sockets) {
      socket.send(message)
    }
  }
}

Hope this gets you a start!

[rahul-sharma-uipath]

@kwhitley I was able to confirm your solution works with itty-durable@next - as expected per this thread, switching from next back to latest stable (1.6.0) causes the socket to hang again.

is there any ETA on when the changes in next will be officially released?