GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — doc: child_process: clarify `subprocess.pid` can be `undefined` when `ENOENT` by dr-js · Pull Request #37014 · nodejs/node Conversation
Contributor
From the code And a short test snippet: const { spawn } = require('child_process') const subProcess = spawn('non-exist-command') subProcess.on('error', (error) => console.warn('mute Unhandled "error" event:', error)) console.log('- pid:', subProcess.pid) process.nextTick(() => console.log('- after error emit')) console.log('== end of test ==')
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
|
@aduh95 Upon adding the test, I found the
Should I add more detailed behavior description to the doc and update test for async and sync spawn, The test code for this: [test.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)) } And the output: [v15.6.0 linux x64][v15.6.0 win32 x64] |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hum that's weird indeed, that might be a bug with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@aduh95 Sure, so in this PR I'll add docs & test checks for And I'll open another Issue with above test to discuss sync spawn pid behavior, and maybe a PR to add test to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
9933250 to
fd5748b
Compare
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
LGTM with doc updates. @nodejs/documentation @nodejs/child_process |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
ec48ec2 to
61f86f3
Compare
Sorry, something went wrong.
There was an error while loading. Please reload this page.
From the code `nodejs@8` and up should behave the same:
github.com/nodejs/node/blame/v8.17.0/lib/internal/child_process.js#L290
And a short test snippet:
```js
const { spawn } = require('child_process')
const subProcess = spawn('non-exist-command')
subProcess.on('error', (error) =>
console.warn('mute Unhandled "error" event:', error))
console.log('- pid:', subProcess.pid)
process.nextTick(() => console.log('- after error emit'))
console.log('== end of test ==')
```
Note: the sync spawn result `pid` currently do not follow this pattern.
Co-authored-by: Rich Trott <[email protected]>
PR-URL: nodejs#37014
Reviewed-By: Antoine du Hamel <[email protected]>
Note: this only add checks for async spawn, as the sync spawn do not return a `subProcess`. PR-URL: nodejs#37014 Reviewed-By: Antoine du Hamel <[email protected]>
61f86f3 to
1e0cfa3
Compare
|
Landed in 574590d...1e0cfa3 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
From the code `nodejs@8` and up should behave the same:
github.com/nodejs/node/blame/v8.17.0/lib/internal/child_process.js#L290
And a short test snippet:
```js
const { spawn } = require('child_process')
const subProcess = spawn('non-exist-command')
subProcess.on('error', (error) =>
console.warn('mute Unhandled "error" event:', error))
console.log('- pid:', subProcess.pid)
process.nextTick(() => console.log('- after error emit'))
console.log('== end of test ==')
```
Note: the sync spawn result `pid` currently do not follow this pattern.
Co-authored-by: Rich Trott <[email protected]>
PR-URL: #37014
Reviewed-By: Antoine du Hamel <[email protected]>
Note: this only add checks for async spawn, as the sync spawn do not return a `subProcess`. PR-URL: #37014 Reviewed-By: Antoine du Hamel <[email protected]>
From the code `nodejs@8` and up should behave the same:
github.com/nodejs/node/blame/v8.17.0/lib/internal/child_process.js#L290
And a short test snippet:
```js
const { spawn } = require('child_process')
const subProcess = spawn('non-exist-command')
subProcess.on('error', (error) =>
console.warn('mute Unhandled "error" event:', error))
console.log('- pid:', subProcess.pid)
process.nextTick(() => console.log('- after error emit'))
console.log('== end of test ==')
```
Note: the sync spawn result `pid` currently do not follow this pattern.
Co-authored-by: Rich Trott <[email protected]>
PR-URL: #37014
Reviewed-By: Antoine du Hamel <[email protected]>
Note: this only add checks for async spawn, as the sync spawn do not return a `subProcess`. PR-URL: #37014 Reviewed-By: Antoine du Hamel <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.