WsAdapter creates two sockets on same server and causes nest to die on ws connection

[lmexo]

Bug Report

Current behavior

Creating a new nest instance with ws websocket adapter, nest creates two sockets on the same server. Using nest's default server booted in main.ts is common getting a websocket connection through this server by upgrading the connection. As I understood the docs, this should be accomplished by just not specifying a port number with the @WebSocketGateway() decorator.

Expected behavior

Allow me to open my socket without crashing nest :)

Possible Solution

The ws-Adapter runs new websocket.Server(...) twice on the same httpServer in line 37 below:

nest/packages/platform-ws/adapters/ws-adapter.ts

Lines 30 to 51 in 2bdcdb5

	public create(
	port: number,
	options?: any & { namespace?: string; server?: any },
	): any {
	const { server, ...wsOptions } = options;
	if (port === 0 && this.httpServer) {
	return this.bindErrorHandler(
	new wsPackage.Server({
	server: this.httpServer,
	...wsOptions,
	}),
	);
	}
	return server
	? server
	: this.bindErrorHandler(
	new wsPackage.Server({
	port,
	...wsOptions,
	}),
	);
	}

The create-method is called twice in packages/websockets/socket-server-provider.ts

• createSocketServer call Adapter.create
  

  nest/packages/websockets/socket-server-provider.ts

  Line 34 in 2bdcdb5

  const ioServer = adapter.create(port, partialOptions);

• and then calls createWithNamespace -> getServerOfNamespace -> calls Adapter.create again
  

  nest/packages/websockets/socket-server-provider.ts

  Lines 66 to 70 in 2bdcdb5

  	return adapter.create(port, {
  	...options,
  	namespace: this.validateNamespace(options.namespace || ''),
  	server,
  	});

The adpter.create()-method is aware that no server should actually be created but the app's server should be used. In new ws.Server() the socket-server registers for the upgrade-event on the underlaying httpServer. Since two Socket's are created on this server, both registered upgrade-handlers are fired from the same server-instance and ws throws

Error: server.handleUpgrade() was called more than once with the same socket, possibly due to a misconfiguration
    at WebSocketServer.completeUpgrade (node_modules\ws\lib\websocket-server.js:267:13)
    at WebSocketServer.handleUpgrade (node_modules\ws\lib\websocket-server.js:245:10)
    at Server.upgrade (node_modules\ws\lib\websocket-server.js:89:16)
    at Server.emit (events.js:201:15)
    at onParserExecuteCommon (_http_server.js:580:14)
    at onParserExecute (_http_server.js:520:3)

Edit: The upgrade-handlers are called when a WebSocket trys to connect, not on application startup - and then nest crashes.

Although the error comes from ws it is due to wrong usage by nest.

Environment

Nest version: 7.4.1

[kamilmysliwiec]

Can you provide a minimum reproduction repository? This would help us to fix this issue quicker.

Also, if you're interested in creating a PR for this issue, go ahead! PRs are more than welcome :)

[lmexo]

Heyhey, I am not that sure about the structure and idea of the code (esp. why there is the server is encapsulated multiple times), otherwise I would have made a PR.

But I created a reproduction repo: https://github.com/lmexo/nest-4968

[kamilmysliwiec]

Thanks @ImeXo! I've cloned your repo and everything works properly on my machine:

Opening socket at port 3030 - this should work
Socket connection opened on port 3030

Upgrading a connection at port 3000 - this should crash the server
Socket connection opened on port 3000 <-- added this logo

No crashes

[lmexo]

Okay, that is weird... The problem only occurs when using namespaces 🤔 I updated the reproduction repository accordingly

[kamilmysliwiec]

Would you like to create a PR for this issue?

[cloakedch]

I stumbled upon this issue after googling for the error message.

Here's my two cents to this:

• Since socket.io-client (The client counterpart to socket.io) is likely going to be deprecated soon, i am trying to migrate from socket.io to ws
• The error seems to only happen when using the platform 'ws' (instead of 'socket.io').
• The error happens only when trying to use namespaces
• ws doesn't seem to support the concept of namespaces/channels/rooms (?)
• My frontend was still expecting a socket.io server counterpart. After I changed it to rxjs websockets, it seems to work
• ws is still less mature than socket.io, but we take what we have...

[dzcpy]

I wonder if it's possible to use both socket.io and websocket as a gateway in different modules?

[TrejGun]

sorry for offtopic

@cloakedch why do you think that socket.io-client is going to be deprecated?

[cloakedch]

There were numerous hints from Damien Arrachequesne in the issue tracker that plain ws is the way to go. Today, it may look different again.

[kamilmysliwiec]

In the next major release (v8), this will simply crash during the application bootstrap process, warning you that "namespaces" are no supported by the "ws" package (it's an exclusive socket.io feature).