ENOTCONN when using add-comments in flow-dev-tools when many files have errors

[jimmydief]

Flow version: 0.132.0 - 0.147.0 (latest) Node version: Tried with both 12.16.1 and v14.16.0 (LTS)

Expected behavior

Upon upgrading from 0.131.0 to 0.132.0, I can use the add-comments command of flow-dev-tools to suppress new errors and ideally to add codes to my existing suppressions and remove warnings.

Actual behavior

After upgrading to either 0.132.0 or 0.147.0 (latest) from 0.131.0, the add-comments command of flow-dev-tools fails after running for a while with the following exception:

../flow/tool add-comments --all --bin ./node_modules/flow-bin/cli.js --check check .
uncaught exception Error: read ENOTCONN
    at tryReadStart (net.js:567:20)
    at Socket._read (net.js:578:5)
    at Socket.Readable.read (_stream_readable.js:478:10)
    at Socket.read (net.js:618:39)
    at new Socket (net.js:372:12)
    at Object.Socket (net.js:263:41)
    at createSocket (internal/child_process.js:314:14)
    at ChildProcess.spawn (internal/child_process.js:437:23)
    at spawn (child_process.js:548:9)
    at Object.execFile (child_process.js:232:17) {
  errno: 'ENOTCONN',
  code: 'ENOTCONN',
  syscall: 'read'
} Error: read ENOTCONN
    at tryReadStart (net.js:567:20)
    at Socket._read (net.js:578:5)
    at Socket.Readable.read (_stream_readable.js:478:10)
    at Socket.read (net.js:618:39)
    at new Socket (net.js:372:12)
    at Object.Socket (net.js:263:41)
    at createSocket (internal/child_process.js:314:14)
    at ChildProcess.spawn (internal/child_process.js:437:23)
    at spawn (child_process.js:548:9)
    at Object.execFile (child_process.js:232:17)
Cleaning up..

The remove-comments command works fine.

Please let me know if there's any additional logging I can add to help narrow down the issue.

[jimmydief]

Looked into this a bit more and the error seems to be caused by the number of spawned processes exceeding my system's limit (2048). Looking briefly into the addComments script, it looks like it's spawning a child process per file containing errors which is causing problems when the number of files with errors exceeds the process limit.

Unfortunately, passing a specific file path doesn't seem to work as a workaround here but I was able to manually work around this by temporarily deleting parts of my project and running the script separately on each individual chunk.

Seems like the right fix is likely capping the number of child processes that are used concurrently to avoid tripping the system limit.

[christianvuerings]

@jimmydief another workaround is to change the following code:

https://github.com/facebook/flow/blob/cda6e1b82ace2c1a4caa5f0b01c9528f90d975a1/packages/flow-dev-tools/src/comment/add-commentsRunner.js#L77

to

for (const error of errors.slice(0, 500)) { 

We do have to run flow/tool add-comments multiple times but at least we don't get the read ENOTCONN error. /cc @jackhsu978

[jackhsu978]

@christianvuerings this workaround works great! thank you so much!