Node is throwing a MaxListenersExceededWarning

ROSeaboyer commented 2 years ago

Are you certain it's a bug?

[X] Yes, it looks like a bug

Are you using the latest version?

[X] Yes, I'm using the latest version

Is there an existing issue for this?

[X] I have searched existing issues, it hasn't been reported yet

Issue description

When running a deployment with the below configuration, I get a MaxListenersExceededWarning when serviceD and serviceE start their deployment. This occurs regardless of which service of serviceB, serviceC, and serviceF finish deploying last.

Not the biggest deal, just something I thought I'd bring up.

node: 16.14.2 @serverless/compose: 1.0.2

Service configuration (serverless-compose.yml) content

services:
  serviceA:
    path: serviceA
  serviceB:
    path: serviceB
    dependsOn:
      - serviceA
  serviceC:
    path: serviceC
    dependsOn:
      - serviceA
  serviceD:
    path: serviceD
    dependsOn:
      - serviceA
      - serviceB
  serviceE:
    path: serviceE
    dependsOn:
      - serviceA
      - serviceC
  serviceF:
    path: serviceF
    dependsOn:
      - serviceA

Command name and used flags

serverless deploy --stage ryan

Command output

(node:41560) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added to [process]. Use emitter.setMaxListeners() to increase limit

mnapoli commented 2 years ago

This isn't indeed a harmless warning but kind of annoying, it could be good to get rid of it.

I'm not 100% sure personally if we should increase the number of max listeners, or slightly refactor the code to avoid the warning. @pgrzesik any opinion on this?

pgrzesik commented 2 years ago

Thanks a lot for reporting @ROSeaboyer - I'll look into it 💯

mnapoli commented 2 years ago

I think one thing we do wrong here is that we never remove the event listener once the child process has terminated. That might contribute to the "ever-increasing" number of event listeners.

pgrzesik commented 2 years ago

I've looked into it and managed to reproduce the problem. @mnapoli you're right - we should be removing the listener there, however, the warning might still pop up e.g. in situation where there's a lot of services and they're are deployed in parallel - I think we might need to set the max listener number to at least the number of all services in Compose configuration.

mnapoli commented 2 years ago

ohh right that makes a lot of sense 👍

serverless / compose