skewten-incubator / worse

websocket implementation
0 stars 0 forks source link

re-add side-by-side upgrade listening #8

Open SEAPUNK opened 8 years ago

SEAPUNK commented 8 years ago

the http 'upgrade' event sucks. it doesn't provide an easy way to maintain socket acceptance

example:

server.on('upgrade', (req, socket) => {
  if (req.headers.upgrade === 'websocket') {
    if (await isValidPath(req.url)) {
      acceptSocket(socket)
    }
  }
})

server.on('upgrade', async (req, socket) => {
  if (req.headers.upgrade === 'websocket') {
    if (await isValidPath(req.url)) {
      acceptSocket(socket)
    }
  }
})

inversely, if we aren't accepting a socket, we're left with either just doing nothing (where the socket has the potential to be idle for a while until finally closing), or doing some hacky "fix" in the off-chance that no other websocket server is listening to the upgrade event

i'm removing path checking, disallowing many:one relations with websocket servers and http servers (respectively), and adding to the docs that only this server is allowed to handle upgrade requests, and that adding more upgrade listeners can mess everything up

i'll add some of these things back whenever node changes how upgrade handling is done

i'll file an issue in nodejs about this sometime later

SEAPUNK commented 8 years ago

well actually no, i'm going to do a few things different

give me a bit to figure this out