Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gateway ignores non-first connection when using dynamic namespace #5993

Closed
jusfeel opened this issue Dec 21, 2020 · 7 comments
Closed

Gateway ignores non-first connection when using dynamic namespace #5993

jusfeel opened this issue Dec 21, 2020 · 7 comments
Labels
needs triage This issue has not been looked into

Comments

@jusfeel
Copy link

jusfeel commented Dec 21, 2020

Bug Report

Current behavior

Using dynamic namespace, say user ID for example, after restart nest server, the first client's works as excepted, whoever that may be, but next user, he cannot trigger the handle connection handler on the gateway any more. Even close all the socket, and reconnect, same. The first user ID, if it was the first connect, that namespace is the only namespace nest is able to handle Connect for that specific namespace. Seems the connection is static for the first connected client.

So behavior is that the first connection can receive and send event no problem but the next client cannot send event to the server to the namespace it is tended (server won't get it) but his namespace and socket somehow is able to receive event from the server sent to this namespace - feels like somehow namespace is able to receive event since it is namespace of a socket which the server can talk to but to send event, the client send to the namespace of that socket, that socket is completely deaf to that, mainly because the connection is never established for that socket on that namespace at all.

Input Code

@WebSocketGateway({ namespace: /^.*$/ })
export class SpoolGateway implements OnGatewayConnection, OnGatewayDisconnect {
    @WebSocketServer()
    server: Server;
    async handleConnection(@ConnectedSocket() client: Socket) {
        console.log('CONNECT');
    }

    async handleDisconnect(@ConnectedSocket() client: Socket) {
          console.log('DISCONNECT')
    }
 
    @SubscribeMessage('message')
    async handleMessage(@ConnectedSocket() client: Socket, @MessageBody() data: any) {
        console.log('message received', data);
    }
}

Expected behavior

Current gateway for nest, if used for dynamic namespace,
should treat namespace dynamically when client has a namespace defined as it needs to
namespace itself for each client should handle connect and disconnect
You can test this from this code using socket.io library

// server
const httpServer = require("http").createServer();
const io = require("socket.io")(httpServer, {
  // ...
});

io.on("connection", (socket) => {
  //console.log('server connection..');
});

const workspaces = io.of(/^\/\w+$/);

workspaces.on('connection', socket => {
  const workspace = socket.nsp;
  console.log('connected to ', workspace.name);
  workspace.emit('hello');
});

httpServer.listen(3000);
// package.json
{
  "name": "socketio-server",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "socket.io": "^2.3.0"
  }
}

client - copy this, change namespace 'def' to anything else, open another browser, keep refresh both browser

<html>
<head>
    <script src="socket.io.js"></script>
    <script>
        const socket = io('http://localhost:3000/def');
        socket.on('connect', function () {
            console.log('I am Connected');
        });
        socket.on('disconnect', function () {
            console.log('Disconnected');
        });
    </script>
</head>
<body></body>
</html>

Possible Solution

Environment


    "@nestjs/common": "^7.6.1",
    "@nestjs/config": "^0.5.0",
    "@nestjs/core": "^7.6.1",
    "@nestjs/jwt": "^7.2.0",
    "@nestjs/microservices": "^7.6.1",
    "@nestjs/passport": "^7.1.5",
    "@nestjs/platform-express": "^7.6.1",
    "@nestjs/platform-socket.io": "^7.6.1",
    "@nestjs/swagger": "^4.7.5",
    "@nestjs/typeorm": "^7.1.5",
    "@nestjs/websockets": "^7.6.1",
@jusfeel jusfeel added the needs triage This issue has not been looked into label Dec 21, 2020
@kamilmysliwiec
Copy link
Member

@jusfeel this issue should be reported in the socket.io repository. Nest simply passes down all the options you specify in the @Gateway() decorator down to the socket.io constructor.

@ChrisKatsaras
Copy link

@jusfeel did you end up finding a solution to your issue?

@jusfeel
Copy link
Author

jusfeel commented Jun 15, 2021

@jusfeel did you end up finding a solution to your issue?

Gave it up. Finally just @WebSocketGateway({ namespace: '/' }) and handle nsp myself

async handleConnection(@ConnectedSocket() client: Socket) {
    const userId = client.handshake.query.userId;
    const nsp = client.server.of(`U-${userId}`);
    nsp.once('connection', async socket => {
    ...
    });
   ...

@ChrisKatsaras
Copy link

So from what I see above, you create a base namespace / and then whenever a socket connection occurs, you create another namespace for the user e.g U-user1 . However, I'm confused how you get that connection into that namespace. I tested what you implemented above and while a new namespace is created, the socket connection doesn't get added to that namespace. Am I missing something?
Thanks for the quick reply @jusfeel :)

@jusfeel
Copy link
Author

jusfeel commented Jun 22, 2021

The way I see namespace is that each connection have that namespace created on it. That's why I use the initial root namespace as some sort of "whatever" as long as it allows me to give the connection that namespace so all the connection having that namespace can be reached.

Also it seems that socket.io 2 got the 'root' namespace created no matter what disregarding if you intend to a specific namespace of your own. I am waiting for nest has new version socket.io 4 release and rethink about my design.

You might also want to think about using rooms instead of namespaces. For me, it's bit late.

@FA-QuangLT
Copy link

@jusfeel @ChrisKatsaras
I'm also experiencing this intermittent issue when using a regex expression for dynamic namespace.

But, In case:

Running a single socket.io node.
Or only one namespace
Or without namespace
=> it works normally.
Note: I have only one the code snippet for namespace instance in my app. This issue occurs when running on multiple node deployed in Kubernetes with many pods.

Is there any solution for this issue ? My application has many namespaces so I needs using dynamic namespace instead of hard code.
detail issue

@ChrisKatsaras
Copy link

Given my teams requirements, we ended up going with a static set of namespaces instead of the dynamic approach. Sorry, @FA-QuangLT , no solution for you on my end 😢

@nestjs nestjs locked and limited conversation to collaborators Jul 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

4 participants