Skip to content

Commit

Permalink
fix(websockets): allow multiple paths when http internal server is used
Browse files Browse the repository at this point in the history
  • Loading branch information
kamilmysliwiec committed Feb 4, 2021
1 parent 29ca9e1 commit cb4a9d6
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 19 deletions.
4 changes: 2 additions & 2 deletions integration/websockets/e2e/ws-gateway.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ describe('WebSocketGateway (WsAdapter)', () => {
// open websockets delay
await new Promise(resolve => setTimeout(resolve, 1000));

ws = new WebSocket('ws://localhost:8082/example');
ws2 = new WebSocket('ws://localhost:8082/ws-path');
ws = new WebSocket('ws://localhost:3000/example');
ws2 = new WebSocket('ws://localhost:3000/ws-path');

await new Promise<void>(resolve =>
ws.on('open', () => {
Expand Down
2 changes: 1 addition & 1 deletion integration/websockets/src/example-path.gateway.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SubscribeMessage, WebSocketGateway } from '@nestjs/websockets';

@WebSocketGateway(8082, {
@WebSocketGateway({
path: '/example',
})
export class ExamplePathGateway {
Expand Down
2 changes: 1 addition & 1 deletion integration/websockets/src/ws-path2.gateway.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SubscribeMessage, WebSocketGateway } from '@nestjs/websockets';

@WebSocketGateway(8082, {
@WebSocketGateway({
path: '/ws-path',
})
export class WsPathGateway2 {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"format": "prettier \"**/*.ts\" --ignore-path ./.prettierignore --write && git status",
"postinstall": "opencollective",
"test": "nyc --require ts-node/register mocha packages/**/*.spec.ts --reporter spec --require 'node_modules/reflect-metadata/Reflect.js' --exit",
"test:integration": "mocha \"integration/*/{,!(node_modules)/**/}/*.spec.ts\" --reporter spec --require ts-node/register --require 'node_modules/reflect-metadata/Reflect.js' --exit",
"test:integration": "mocha \"integration/websockets/{,!(node_modules)/**/}/*.spec.ts\" --reporter spec --require ts-node/register --require 'node_modules/reflect-metadata/Reflect.js' --exit",
"test:docker:up": "docker-compose -f integration/docker-compose.yml up -d",
"test:docker:down": "docker-compose -f integration/docker-compose.yml down",
"lint": "concurrently 'npm run lint:packages' 'npm run lint:integration' 'npm run lint:spec'",
Expand Down
35 changes: 21 additions & 14 deletions packages/platform-ws/adapters/ws-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ export class WsAdapter extends AbstractWsAdapter {

public create(
port: number,
options?: any & { namespace?: string; server?: any },
): any {
options?: Record<string, any> & { namespace?: string; server?: any },
) {
const { server, ...wsOptions } = options;
if (wsOptions?.namespace) {
const error = new Error(
Expand All @@ -55,24 +55,29 @@ export class WsAdapter extends AbstractWsAdapter {
this.logger.error(error);
throw error;
}

if (port === UNDERLYING_HTTP_SERVER_PORT && this.httpServer) {
return this.bindErrorHandler(
this.ensureHttpServerExists(port, this.httpServer);
const wsServer = this.bindErrorHandler(
new wsPackage.Server({
server: this.httpServer,
noServer: true,
...wsOptions,
}),
);

this.addWsServerToRegistry(wsServer, port, options.path || '/');
return wsServer;
}

if (server) {
// When server exists already
return server;
}
if (options.path && port !== UNDERLYING_HTTP_SERVER_PORT) {
// Multiple servers with different paths
// sharing a single HTTP/S server running on different port
// than a regular HTTP application
this.ensureHttpServerExists(port);
const httpServer = this.ensureHttpServerExists(port);
httpServer?.listen(port);

const wsServer = this.bindErrorHandler(
new wsPackage.Server({
Expand Down Expand Up @@ -145,19 +150,22 @@ export class WsAdapter extends AbstractWsAdapter {
}

public async dispose() {
const closeEvents = Array.from(this.httpServersRegistry).map(
([_, server]) => new Promise(resolve => server.close(resolve)),
);
await Promise.all(closeEvents);
const closeEventSignals = Array.from(this.httpServersRegistry)
.filter(([port]) => port !== UNDERLYING_HTTP_SERVER_PORT)
.map(([_, server]) => new Promise(resolve => server.close(resolve)));

await Promise.all(closeEventSignals);
this.httpServersRegistry.clear();
this.wsServersRegistry.clear();
}

protected ensureHttpServerExists(port: number) {
protected ensureHttpServerExists(
port: number,
httpServer = http.createServer(),
) {
if (this.httpServersRegistry.has(port)) {
return;
}
const httpServer = http.createServer();
this.httpServersRegistry.set(port, httpServer);

httpServer.on('upgrade', (request, socket, head) => {
Expand All @@ -179,8 +187,7 @@ export class WsAdapter extends AbstractWsAdapter {
socket.destroy();
}
});

httpServer.listen(port);
return httpServer;
}

protected addWsServerToRegistry<T extends Record<'path', string> = any>(
Expand Down

1 comment on commit cb4a9d6

@wolfuser99
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamilmysliwiec it's broken at current version 8.0.3, could you check please

Please sign in to comment.