Repository: node
Issue: child_process: sync spawn should not return pid number of a `non-exist-command`
Labels: child_process

<!--
Thank you for suggesting an idea to make Node.js better.

Please fill in as much of the template below as you're able.
-->

**Is your feature request related to a problem? Please describe.**


This strange behavior of sync spawn is found when adding test to https://github.com/nodejs/node/pull/37014#issuecomment-765065762.

Upon adding the test, I found the `pid` value is a bit more complex than I thought:
- an async spawn with invalid cwd will get pid `undefined` (expected)
- an async spawn with invalid commad and `shell: false` will get pid `undefined` (expected)
- an async spawn with invalid commad and `shell: true` will get pid of the shell (expected)
- a sync spawn seems always return pid (strange)

The test code for this:
<details><summary>[test.js]</summary>

```js
[
  () => testSpawn('with invalid command', [ 'non-exist-command' ]),
  () => testSpawn('with invalid command and `shell: true`', [ 'non-exist-command', { shell: true } ]),
  () => testSpawn('with invalid cwd', [ 'node', { cwd: './non/exist/cwd/path/' } ]),
  () => testSpawn('invalid cwd and `shell: true`', [ 'node', { cwd: './non/exist/cwd/path/', shell: true } ]),

  () => testSpawnSync('with invalid command', [ 'non-exist-command' ]),
  () => testSpawnSync('with invalid command and `shell: true`', [ 'non-exist-command', { shell: true } ]),
  () => testSpawnSync('with invalid cwd', [ 'node', { cwd: './non/exist/cwd/path/' } ]),
  () => testSpawnSync('invalid cwd and `shell: true`', [ 'node', { cwd: './non/exist/cwd/path/', shell: true } ])
].forEach((testFunc, index) => setTimeout(testFunc, index * 200))

const formatError = (error) => error && error.message
// const formatError = (error) => error && error.stack // for more detail

const testSpawn = (title = '', spawnArgs = '') => {
  console.log(`\n== test spawn ${title} ==`)
  const { spawn } = require('child_process')
  const subProcess = spawn(...spawnArgs)
  subProcess.on('error', (error) => console.warn('- "error" event:', formatError(error))) // mute Unhandled "error" event or the test will exit
  console.log('- pid:', subProcess.pid, 'exitCode:', subProcess.exitCode)
  process.nextTick(() => console.log('- nextTick pid:', subProcess.pid, 'exitCode:', subProcess.exitCode))
  setTimeout(() => console.log('- after 100ms pid:', subProcess.pid, 'exitCode:', subProcess.exitCode), 100)
}

const testSpawnSync = (title = '', spawnArgs = '') => {
  console.log(`\n== test spawnSync ${title} ==`)
  const { spawnSync } = require('child_process')
  const outcome = spawnSync(...spawnArgs) // NOTE: not a `subProcess`
  console.log('- pid:', outcome.pid, 'status', outcome.status)
  console.log('- error', formatError(outcome.error))
}
```

</details>

And the output:
<details><summary>[v15.6.0 linux x64]</summary>

```
v15.6.0 linux x64

== test spawn with invalid command ==
- pid: undefined exitCode: null
- "error" event: spawn non-exist-command ENOENT
- nextTick pid: undefined exitCode: -2
- after 100ms pid: undefined exitCode: -2

== test spawn with invalid command and `shell: true` ==
- pid: 30493 exitCode: null
- nextTick pid: 30493 exitCode: null
- after 100ms pid: 30493 exitCode: 127

== test spawn with invalid cwd ==
- pid: undefined exitCode: null
- "error" event: spawn node ENOENT
- nextTick pid: undefined exitCode: -2
- after 100ms pid: undefined exitCode: -2

== test spawn invalid cwd and `shell: true` ==
- pid: undefined exitCode: null
- "error" event: spawn /bin/sh ENOENT
- nextTick pid: undefined exitCode: -2
- after 100ms pid: undefined exitCode: -2

== test spawnSync with invalid command ==
- pid: 30496 status null
- error spawnSync non-exist-command ENOENT

== test spawnSync with invalid command and `shell: true` ==
- pid: 30497 status 127
- error undefined

== test spawnSync with invalid cwd ==
- pid: 30498 status null
- error spawnSync node ENOENT

== test spawnSync invalid cwd and `shell: true` ==
- pid: 30499 status null
- error spawnSync /bin/sh ENOENT
```

</details>

<details><summary>[v15.6.0 win32 x64]</summary>

```
v15.6.0 win32 x64

== test spawn with invalid command ==
- pid: undefined exitCode: null
- "error" event: spawn non-exist-command ENOENT
- nextTick pid: undefined exitCode: -4058
- after 100ms pid: undefined exitCode: -4058

== test spawn with invalid command and `shell: true` ==
- pid: 21860 exitCode: null
- nextTick pid: 21860 exitCode: null
- after 100ms pid: 21860 exitCode: 1

== test spawn with invalid cwd ==
- pid: undefined exitCode: null
- "error" event: spawn node ENOENT
- nextTick pid: undefined exitCode: -4058
- after 100ms pid: undefined exitCode: -4058

== test spawn invalid cwd and `shell: true` ==
- pid: undefined exitCode: null
- "error" event: spawn C:\Windows\system32\cmd.exe ENOENT
- nextTick pid: undefined exitCode: -4058
- after 100ms pid: undefined exitCode: -4058

== test spawnSync with invalid command ==
- pid: 0 status null
- error spawnSync non-exist-command ENOENT

== test spawnSync with invalid command and `shell: true` ==
- pid: 8864 status 1
- error undefined

== test spawnSync with invalid cwd ==
- pid: 0 status null
- error spawnSync node ENOENT

== test spawnSync invalid cwd and `shell: true` ==
- pid: 0 status null
- error spawnSync C:\Windows\system32\cmd.exe ENOENT
```

</details>

On win32 some spawnSync tests return pid `0`, which is strange, so I try tracked the source code,
  and the main difference is `spawn_sync` set the pid without checking the `uv_spawn` result.

- The sync spawn's code path:
  * internalBinding: https://github.com/nodejs/node/blob/v15.6.0/lib/internal/child_process.js#L55
  * set pid: https://github.com/nodejs/node/blob/v15.6.0/src/spawn_sync.cc#L720-L721

- The async spawn code path: (different C++ code)
  * internalBinding: https://github.com/nodejs/node/blob/v15.6.0/lib/internal/child_process.js#L39
  * set pid: https://github.com/nodejs/node/blob/v15.6.0/src/process_wrap.cc#L261-L266

**Describe the solution you'd like**

Change the sync spawn to return `pid` similar to the async spawn's `subprocess.pid` behavior.

**Describe alternatives you've considered**

Or do not return the `pid` for sync spawn?
As the process should have exited at the time, and there'll be less use for the `pid`.