-
Notifications
You must be signed in to change notification settings - Fork 29.1k
-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
Add a cleaned up child_processes
API that cleans the API up for child process spawning, like how fs/promises
cleaned up fs
#54799
Comments
What if we make the existing const child = spawn('ls', ['-lh', '/usr']); // reasonable, creates a child process
// Now I want to read its stdout and wait for it to close, this is verbose:
const output = Buffer.concat(await child.stdout.toArray()).toString();
// What if instead we could do:
const output2 = await child.text();
// Or as a one liner
console.log(await spawn('ls', ['-lh', '/usr']).text()); ? |
@benjamingr I also considered that (and forgot to list it) and I even tried that route first. However, I ultimately found it to be untenable:
Will note that the |
Updated the proposal to make a number of revisions and simplifications. It's still similar, but different enough to merit a re-read. One of my goals was to allow export function system(script, args, opts) {
if (!Array.isArray(args)) {
opts ??= args
args = []
}
let {execPath, execArgv} = opts ?? {}
if (process.platform === "win32") {
execPath ??= process.env.COMSPEC || "cmd.exe"
execArgv ??= ["/d", "/s", "/c"]
} else {
execPath ??= "sh"
execArgv ??= ["-c"]
}
return exec(
execPath,
[...execArgv, ...args],
{...opts, pathLookup: true},
)
}
export function fork(script, args, opts) {
if (!Array.isArray(args)) {
opts ??= args
args = []
}
let {execPath, execArgv, env, fds} = opts ?? {}
execPath ??= "node"
execArgv ??= []
const normalized = {...fds}
const ports = []
for (const fd of Object.keys(normalized)) {
if (!/^\d+$/.test(normalized)) continue
const source = normalized[fd]
if (!(source instanceof MessagePort)) continue
if (isTransferred(source)) throw new Error("...")
ports.push({fd, source})
}
let channelFds = ""
for (const {fd, source} of ports) {
// `convertToStream` is of course non-trivial
normalized[fd] = convertToStream(port)
channelFds += "," + fd
}
return exec(
execPath,
[...execArgv, ...args],
{
...opts,
pathLookup: true,
env: channelFds ? {...env, NODE_CHANNEL_FD: channelFds.slice(1)} : env,
}
)
} |
What is the problem this feature will solve?
child_process
is, in my experience, one of the most commonly wrapped APIs by far. It's especially common to wrap it in promise environments.cross-spawn
has over 50 million weekly downloads.It all stems from a few major issues:
What is the feature you are proposing to solve the problem?
I'm thinking of the following API, in
child_process/promises
:result = await promises.exec(command, args?, options?)
to spawn a normal commandresult = await promises.system(command, args?, options?)
to spawn a shell commandresult = await promises.fork(command, args?, options?)
to spawn a child with an IPC channelOptions and arguments:
command
is what to run.exec
: the binary to run, may be afile:
URLsystem
: the shell script to runfork
: the Node script to run, may be afile:
URLargs
is an array of arguments to pass to the script or command and it defaults to the empty array.cross-spawn
's behavior.options
properties still work, with the same defaults:options.detached
options.cwd
options.env
options.argv0
options.uid
options.gid
options.signal
is now an object, where keys are the signal names and values areAbortSignal
s and async iterables that can trigger them.options.ref
determines whether the process starts out ref'd.options.execPath
forsystem
andfork
and represents the path to use. Unlike inchild_process.spawn
, this is not a complete command. Defaults:system
:"sh"
in *nix,process.env.COMSPEC || "cmd.exe"
on Windowsfork
:"node"
options.execArgv
provides the list of arguments to pass before passing the script. Defaults:system
:["-c"]
on *nix,["/d", "/s", "/c"]
on Windowsfork
:[]
options.pathLookup
totrue
(default) to use the system path to locate the target binary,false
to resolve it based on the current working directory. On Unix-like systems,true
also enables interpreters to work.system
andfork
, this is always set totrue
and cannot be configured.%PathExt%
traversal.lookupPath: true
natively withexecve
.options.fds
is an object where each numerical index corresponds to a descriptor to set in the child. This is not necessarily an array, though one could be passed. Default is{0: 0, 1: 1, 2: 2}
to inherit those descriptors. Possible entry values:"close"
: explicitly close FD, cannot be used for FD 0/1/2"null"
: connect to the system null deviceMessagePort
instance (fork
only): open an IPC portMessagePort
on the other side of the channel is also closed in the same way it is for workers where one end closes.fs/promises
file handle,net.Socket
, etc: Pass a given file descriptor directlyreadableStream
: Expose a writable pipe and read from it using the given streamBufferReader
to read from buffers and stringswritableStream
: Expose a readable pipe and write into it using the given streamBufferWriter
to write into buffersoptions.fds.inherit
totrue
to inherit all FDs not specified inoptions.fds
. Default isfalse
, in which FDs 0/1/2 are opened to the null device and all others are closed.The return value is a Promise that settles when the child terminates.
exitCode
andsignalCode
properties if it exited with any other code.pid = await handle.spawned
resolves with the PID on spawn and rejects on spawn error.Additional classes in
stream
:writer = new BufferReader(target | max)
stream.Writable
target
buffer source to fill or amax
byte lengthwriter.bytesWritten
: Get the number of bytes writtenwriter.consume()
: Reset the write state and return the previously written buffer data. If it's not writing to an external target, it's possible to avoid the buffer copy.writer.toString(encoding?)
is sugar forwriter.consume().toString(encoding?)
reader = new BufferWriter(source, encoding?)
stream.Readable
source
string (with optional encoding) or buffer source to read fromreader.bytesRead
: Get the number of bytes readReadable.from(buffer | string)
should return instances of this insteadduplex.reader()
,duplex.writer()
: Return the read or write half of a duplex stream, sharing the same internal stateAnd in
process
:port = process.ipc(n=3)
: Get a (cached)MessagePort
for a given IPC descriptor, throwing if it's not a valid descriptor.result = await process.inspectFD(n)
accepts an FD and returns its type and read/write state.{kind: "file", readable, writable}
readable
andwritable
can be determined viafcntl(F_GETFL, fd)
on *nix (it's been in the POSIX standard for a couple decades)readable
andwritable
can be determined via two calls toReOpenFile
or one call toNtQueryObject
with classObjectBasicInformation
. (They say it can change, but it may be possible to get a stability promise out of them since the page hasn't been modified in over 6 years.){kind: "socket", readable, writable, type: "stream-client" | "stream-server" | "dgram"}
{kind: "tty", readable, writable, rows, columns}
ioctl
syscall on Linux{kind: "ipc", readable, writable}
{kind: "unknown"}
Things I'm intentionally leaving out:
options.serialization
- it's always"advanced"
. This both brings it to close parity with otherMessagePort
-related APIs, and it speeds up message sending since it's already of the correct input format.options.timeout
- just dosignal: {SIGTERM: AbortSignal.timeout(ms)}
.options.windowsHide
- that behavior is just always on as that's what people would generally just expect.options.windowsVerbatimArguments
- just usesystem
and string concatenation."inherit"
constants instdio
- you can just use the descriptor numbers themselves for that."pipe"
- use a passthrough stream for that.options.encoding
- that's a per-descriptor setting now."close"
event - it's better to track that per-stream anyways. Plus, it's one of those incredibly error-prone points.For a summary in the form of TypeScript definitions:
What alternatives have you considered?
I considered:
.ref()
,.unref()
,.pid
,.wait()
, and.raise(signal?)
. The main problem is this, for the common case, requiresawait start(...).then(h => h.wait())
.handle.ipc
as a single port. I don't see why one can't have multiple IPC ports, and it also simplifies the API and the implementation.The text was updated successfully, but these errors were encountered: