Bailing on failure when running concurrently should also work with vscode's WSL extension

[s-h-a-d-o-w]

Description

This is related to my previous work in #1117.

Currently, this doesn't work because of an old flaw in vscode. It fails pretty gracefully though - it just can't interrupt other running tasks and prints the error caused by WSL. A lot like the previous behavior.

I've submitted an issue to the vscode team but... let's see whether they are interested in fixing this.

I would argue to hold on to hope for a week or two before considering other options (I do have some backup plans 😅), if that's alright with you.

Steps to reproduce

• Use WSL with this extension: https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-wsl
• Try to commit something with a linting problem (and another, long-running task)
• You should see something like the following:

[STARTED] Reverting to original state because of errors...
[SUCCESS] Reverting to original state because of errors...
[STARTED] Cleaning up temporary files...
[SUCCESS] Cleaning up temporary files...

✖ eslint --cache:

/home/andy/temp/chrome-extension/src/auth.ts
  5:7  error  'something' is assigned a value but never used  @typescript-eslint/no-unused-vars

✖ 1 problem (1 error, 0 warnings)

(node:13531) UnhandledPromiseRejectionWarning: Error: your 131072x1 screen size is bogus. expect trouble

    at ChildProcess.<anonymous> (/home/andy/temp/chrome-extension/node_modules/lint-staged/node_modules/pidtree/lib/bin.js:42:19)
    at ChildProcess.emit (events.js:400:28)
    at maybeClose (internal/child_process.js:1058:16)
    at Socket.<anonymous> (internal/child_process.js:443:11)
    at Socket.emit (events.js:400:28)
    at Pipe.<anonymous> (net.js:686:12)
(Use `node --trace-warnings ...` to show where the warning was created)

Environment

• OS: Windows 11 with Ubuntu WSL
• Node.js: v14.19.0
• lint-staged: 12.3.6

[juliomrc]

After the latest minor update in pidtree, we could bump the required version of pidtree to ^0.6.0 in lint-staged to fix this issue.

[iiroj]

@juliomrc thanks! Will be updated/fixed in #1173

[s-h-a-d-o-w]

@iiroj I would like to note that ever since I realized that my initial implementation was flawed (I'm deeply sorry about that!), I tried to improve it in ways that @lildeadprince mentioned here (using events instead of polling): #1138

I found a simpler, more reliable solution relatively quickly and you can see it here: https://github.com/s-h-a-d-o-w/lint-staged/blob/chore/improve-task-killing-reliability/lib/resolveTaskFn.js

I haven't been able to submit a PR though because as you can see on that branch, odd problems with some tests where multiple tests seem to share state appear. (In runAll.spec.js, things like: https://github.com/okonet/lint-staged/blob/master/test/runAll.spec.js#L195 )

But I thought I should share it here and now regardless, since the changes in resolveTaskFn.js seem solid to me, as the additional tests that I added also demonstrate.

[iiroj]

@s-h-a-d-o-w thanks, I will take a look at the EventEmitter approach.

[iiroj]

@s-h-a-d-o-w I added a commit to #1173 that replaces setInterval with EventsEmitter!

[s-h-a-d-o-w]

Awesome, thanks for incorporating those improvements!