nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.73k stars 7.63k forks source link

Multiple connection events for single client websocket connection #3858

Closed Sieabah closed 4 years ago

Sieabah commented 4 years ago

Bug Report

Current behavior

When a client connects to the server all WebSocketGateways get a connection (of the same client) times the total number of WebSocketGateways.

Input Code

Setup 2 or more WebSocketGateways that are separate but share the same namespace

Expected behavior

Connection event should fire once per connection per gateway

Possible Solution

Have each gateway be responsible for handling its own connection event and not pollute all gateways with the connection event which results in duplication.

kamilmysliwiec commented 4 years ago

Setup 2 or more WebSocketGateways that are separate but share the same namespace

If they share the same namespace, the connection event has to fire for all of them. This wouldn't make sense if we just randomly picked one gateway and fired event for it.

Sieabah commented 4 years ago

@kamilmysliwiec No you misunderstood the problem. Yes it should emit the event once per gateway, not emit the event the by the number of gateways you have in total.

If I register a client connected event in my auth gateway every other gateway I make in the same namespace will publish an additional event to the auth gateway. So in the end, for a single connection event the auth gateway gets a total of 3 (n) events.

@WebSocketGateway()
export class AuthWsController implements OnGatewayConnection {
   handleConnection(client: any): any { console.log('AuthWs'); }
}

@WebSocketGateway()
export class UserWsController implements OnGatewayConnection {
   handleConnection(client: any): any { console.log('UserWs'); }
}

@WebSocketGateway()
export class TestWsController implements OnGatewayConnection {
   handleConnection(client: any): any { console.log('TestWs'); }
}

If a user connect to the gateway AuthWsController.handleConnection() gets called 3 times, UserWsController.handleConnection() and TestWsController.handleConnection() also get called the same number of times for a single connection. I think the desired behavior is they each get the event once. As it's a single connection event.

This can't honestly be desired behavior, how am I meant to dedupe this connection event? The client only knows that it connected once, because it did only once. I currently have it just dedupe in a 5 second cache if the same socket id connects but it's silly that this is default behavior.

kamilmysliwiec commented 4 years ago

Are you sure that you have the latest version of all @nestjs packages (@nestjs/websockets in this case)? I'm pretty sure this issue was reported a while ago and we fixed it already.

If you have all packages up to date, please, share a small repository which reproduces your issue.

Sieabah commented 4 years ago

I'll recheck to see if I have some weird issue with a bad lockfile keeping it around for some reason

Sieabah commented 4 years ago

It was indeed some issue with my lock file keeping a version of 6.9.x, this issue does not persist in 6.10.14

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.