Multiple gateways with socket.io get (disconnect listeners added to [Socket])

[MedianAura]

Bug Report

Current behavior

Input Code

https://github.com/MedianAura/gateway-test

Just use :
npm run start:dev

Go to :
http://127.0.0.1:3000/

Expected behavior

Not to get this warning or at least understand what can cause it and what is the risk

Possible Solution

N/A

Environment

Nest version: 7.6.0

 
For Tooling issues:
- Node version: 14.17.0
- Platform:  Windows 10

Others:
yarn

[jmcdo29]

Your socketio version ol for the public/index.html is v4 which is incompatible with socketio v2 (what nest uses for the server). Please downgrade your client and check again

[MedianAura]

Downgraded to 2.4.0 and still the same issue just a tiny bit less spammy

[jmcdo29]

Cool, wanted to rule that out. It looks like for every gateway class (a class decorated with @WebSocketGateway()), two disconnect handlers get bound to the client instance. One is a function named handler and one is an anonymous function. I'm trying to determine where they are bound from, but you can see in this log how this builds up quickly (I changed all of the @SubscribeMessage()s to have specific letters to correspond with the gateway to make debugging easier)

[]
[
  { message: 'a', callback: [Function: bound ] AsyncFunction },
  { message: 'a2', callback: [Function: bound ] AsyncFunction }
]
[ [Function: handler], [Function (anonymous)] ]
[ { message: 'b', callback: [Function: bound ] AsyncFunction } ]
[
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)]
]
[ { message: 'c', callback: [Function: bound ] AsyncFunction } ]
[
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)]
]
[ { message: 'd', callback: [Function: bound ] AsyncFunction } ]
[
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)]
]
[ { message: 'e', callback: [Function: bound ] AsyncFunction } ]
[
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)],
  [Function: handler],
  [Function (anonymous)]
]
[ { message: 'f', callback: [Function: bound ] AsyncFunction } ]
(node:26549) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 disconnect listeners added to [Socket]. Use emitter.setMaxListeners() to increase limit

This is from adding a console.log in the io-adapter.js file in the node_modules. There should probably be some way to not bind new disconnect handlers, but I'd need more time with the code.

[genadis]

@jmcdo29 Please take a look at #6026 (comment), I believe it's related

[itspers]

So is there any solution?

[FredericoGauz]

I would also appreciate a solution for this. At this moment I am creating many different classes using the @WebSocketGateway() decorator and adding them to their respective modules which are then imported in the general app.module

Is this the right way of doing things or should I create a gateway module that imports the other modules and handle all the messages? I could do that but it seems that this would defeat the purpose of having the code organized by fields instead of functionality.

I saw the option of creating a custom adapter to raise the connection limit, but from what I see I would have to keep increasing the limit the more gateway classes I add.

Thanks in advance! =)

[RDeluxe]

I think this issue should be re-opened, as this is getting spammy really fast

[alterfo]

@FredericoGauz hello, just curious if you're succeed creating a gateways mediator? I gave it a much thought and it seems to be a good API dictionary, alas breaks separation of concerns. Thank you in advance!

[osmankorogluu]

hello, I'm on a very important place in the project. I think the dto is not working properly can you help me?

[FredericoGauz]

Hi Oleg, Unfortunately I didn't, what I did was create a separate module that handles one Gateway and import the other modules, handling their requests. It is far from ideal, but at least is working and it should not be very hard to refractor if we have a chance to do it if something changes in the future. Let me know if I can be of any help. If you are taking generally about nestjs it is very interesting, but there is definitely a trade off between code quality and difficult to use. Sometimes nestjs makes things very easy, but other times I feel like it is a nightmare. Probably because don't understand it as much as I should. Thanks

…

On Tue, Aug 23, 2022, 6:31 PM Oleg Sidorkin ***@***.***> wrote: @FredericoGauz <https://github.com/FredericoGauz> hello, just curious if you're succeed creating a gateways mediator? I gave it a much thought and it seems to be a good API dictionary, but breaks separation of concerns. Thank you in advance! — Reply to this email directly, view it on GitHub <#7249 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEL2SSUQK2GENLA55MRJZT3V2U7JPANCNFSM46M3TJJA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Karman40]

Is there a solution to this error?

[maxgaurav]

I too faced this issue and have found the root cause of the problem. It is in package @nestjs/platform-socket.io

Here is what is happening for every event the client(browser) connects to backed through gateway the function bindMessageHandlers. The function internally creates a listener to the disconnect event of socket which is further used to stop processing events propagated by server to client. Hence if you have 20 such events being subscribed by client the total number of disconnect would be 20 +2. 20 disconnect listeners for each event subscribed and 2 internal system event handling in which 2nd system based event subscription is conditional but still worth mentioning it.

File: io-adapter.ts in package

  public bindMessageHandlers(
    socket: Socket,
    handlers: MessageMappingProperties[],
    transform: (data: any) => Observable<any>,
  ) {
  
  // THE BELOW CODE GETS EXECUTED EVERY TIME. 
    const disconnect$ = fromEvent(socket, DISCONNECT_EVENT).pipe(
      share(),
      first(),
    );

    handlers.forEach(({ message, callback }) => {
      const source$ = fromEvent(socket, message).pipe(
        mergeMap((payload: any) => {
          const { data, ack } = this.mapPayload(payload);
          return transform(callback(data, ack)).pipe(
            filter((response: any) => !isNil(response)),
            map((response: any) => [response, ack]),
          );
        }),
        takeUntil(disconnect$),
      );
      source$.subscribe(([response, ack]) => {
        if (response.event) {
          return socket.emit(response.event, response.data);
        }
        isFunction(ack) && ack(response);
      });
    });
  }

The default max listeners allowed by node process is 10 if you do not override the max listeners. Those of you who are facing this issue can do the following in your main entry file to fix the warning but you would have to calculate based on number of events your client is subscribing. If your events are dynamic in nature then by overtaking the socket and setting the maxlistenrs through the setMaxListeners function on the socket is the way to go.

File: main.ts

// your other imports like nest and etc
import { EventEmitter } from 'events';

/**
 * Setting max listeners to 15
 * It appears every message gateway added, adds a new listener to "disconnect" event. Needs investigation in the nestjs
 * codebase for platform socket.io
 */
(EventEmitter.prototype as any)._maxListeners = 15;

function bootstrap() {
// your main nestjs application creation code 
const app = await NestFactory.create<NestExpressApplication>(AppModule);
...
}

bootstrap();

I will look at the problem in detail in the main code of the package and try to see if I can generate a pull request. The main solution is to move the disconnect observable outside the handler function and keep using that with share action.

This is a very dangerous bug I would say as it is leading to memory leaks especially for cases where events subscribed are dynamic in nature as it exponentially escalates with number of websocket connection made. For those who have limited events like under 20 it is not going to impact as much but still dangerous none the less.

[axotion]

Bump as this issue still exists in the current version of NestJS and those hacks are fine, but I'd love to see a final fix for this 🥹

[luismariotti1]

I am also experiencing this issue. As a solution, I'm considering centralizing the gateway in a single file, which will act as a router and call functions from other modules. I understand this could potentially complicate code maintenance, but it might prevent a decrease in app performance or memory leak.

[lponciolo]

As maxgaurav points out, the problem lies in the io-adapter. I've submitted a PR requesting some changes to solve this issue. If you could validate it, I would appreciate it. PR

[lexarion1]

Hello. Any updates and plans to fix this? Or at least update documentation on the main page how to increase limit and that multiple gateways is not recommended.