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

Ensure maxListeners for process.stdout accounts for workers #8689

Conversation

marcins
Copy link
Contributor

@marcins marcins commented Dec 13, 2022

↪️ Pull Request

Fixes the MaxListenersExceededWarning warnings that would occur in environments with > 10 workers.

This occurs because by default Node will pipe worker stdout to process stdout, however it doesn't increase the maxListener property of the process.stdout event emitter.

This behaviour was validated with a simple test script - added 20 workers and process.stdout.getMaxListeners() was still 10, piping to stdout resulted in a warning about 21 listeners.

The "Ora" library used by the CLI Reporter also pipes into process.stdout, and when this pipe is connected this triggers the warnings as worker_count + 1 > 10 (which is the default maxListener value).

As a fix we will set the process.stdout.setMaxListeners() to equal to the worker count + 1. This isn't perfect, especially if something else hooks into process.stdout in future, but due to the limitation of calling process.stdout.getMaxListeners() mentioned above is at least a better outcome for Parcel out of the box.

Related in the context of Parcel: sindresorhus/ora#162

🚨 Test instructions

Before the patch - run Parcel with:

❯ PARCEL_WORKERS=20 node ./node_modules/.bin/parcel build --no-cache packages/examples/ts-example/src/index.ts

console: (node:4012) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 21 unpipe listeners added to [WriteStream]. Use 
emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
[...]

After the patch the same command does not trigger any warnings.

✔️ PR Todo

  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@marcins marcins force-pushed the mszczepanski/prevent-emitter-leak-warning branch from 959dec4 to 1cb0db4 Compare December 13, 2022 22:19
@devongovett devongovett merged commit 593c2bc into parcel-bundler:v2 Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants