Don't lose messages when hooks are asynchronous

[airhorns]

New in version 3.0.0, we run the fastify hooks for the request before handing the constructed websocket over to the route handler. Before this change, fastify-websocket processed the UPGRADE request from the client before dispatching the request to the router. This means it created the WebSocket object and the duplex stream wrapping it before invoking all the userland code that might want to do stuff in hooks like authentication, or loading a session, or whatever.

If those hooks that run after the creation of the websocket are asynchronous, that means that the websocket that has already been set up and connected can receive messages during the execution of those async hooks. Generally I think developers would attach onMessage handlers in their route handler functions. If they do that, those message handler functions wouldn't be attached until after all the pre-route handlers have been run, which would mean any messages that arrive during hook processing won't trigger any message handlers and get silently dropped!

Instead, we should buffer any incoming messages until the userland route handler is ready to work with the WebSocket, and give it the chance to synchronously attach event listeners. The net.Socket will buffer any messages, and then the ws library will dispatch them as messages on the next process tick after setting up the WebSocket instance. The handler will run in between there and get a chance to register listeners. Hooks won't have access to the websocket if we do this, but they already didn't, and I think this is a big enough gotcha that that's fine.

To implement this, we stop using the connection event of the ws server, and instead listen to the upgrade event of the http.Server, and still dispatch that request through fastify. Then, in the route handler, we use wss.handleUpgradeRequest to create the WebSocket. The upgrade runs after all the hooks are done, and then synchronously passes in the WebSocket instance to the route handler, allowing it to get the handlers registered in time for any messages coming off the buffer.

This means that the ws server runs without actually attaching to an http.Server, which I think makes sense since we want finer control over when the upgrade is actually handled. Because this requires some special options to be passed to the ws server, I opted to remove support for the noServer option to the ws server. I think this is fine -- it seems kind of strange to use a http sever plugin to register a websocket system that you don't want listening using that http server, but it is a breaking change. We still support the custom server option that gets passed through to ws.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[airhorns]

This tests that messages that arrive while async hooks are processing are still seen in the route handler

[mcollina]

Good work, just a nit

[mcollina]

lgtm