Skip to content
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

Pass more node child process tests #16863

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion src/bun.js/api/bun/process.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,7 @@ pub fn spawnProcessPosix(
}

if (options.cwd.len > 0) {
actions.chdir(options.cwd) catch return error.ChangingDirectoryFailed;
try actions.chdir(options.cwd);
}
var spawned = PosixSpawnResult{};
var extra_fds = std.ArrayList(bun.FileDescriptor).init(bun.default_allocator);
Expand Down
11 changes: 4 additions & 7 deletions src/bun.js/api/bun/subprocess.zig
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ pub const Subprocess = struct {
IPClog("Subprocess#doSend", .{});
const ipc_data = &(this.ipc_data orelse {
if (this.hasExited()) {
return global.throw("Subprocess.send() cannot be used after the process has exited.", .{});
return global.ERR_IPC_CHANNEL_CLOSED("Subprocess.send() cannot be used after the process has exited.", .{}).throw();
} else {
return global.throw("Subprocess.send() can only be used if an IPC channel is open.", .{});
}
Expand Down Expand Up @@ -1715,7 +1715,7 @@ pub const Subprocess = struct {

// This is split into a separate function to conserve stack space.
// On Windows, a single path buffer can take 64 KB.
fn getArgv0(globalThis: *JSC.JSGlobalObject, PATH: []const u8, cwd: []const u8, argv0: ?[*:0]const u8, first_cmd: JSValue, allocator: std.mem.Allocator) bun.JSError!struct {
fn getArgv0(globalThis: *JSC.JSGlobalObject, PATH: []const u8, cwd: []const u8, pretend_argv0: ?[*:0]const u8, first_cmd: JSValue, allocator: std.mem.Allocator) bun.JSError!struct {
argv0: [:0]const u8,
arg0: [:0]u8,
} {
Expand All @@ -1727,10 +1727,7 @@ pub const Subprocess = struct {

var actual_argv0: [:0]const u8 = "";

const argv0_to_use: []const u8 = if (argv0) |_argv0|
bun.sliceTo(_argv0, 0)
else
arg0.slice();
const argv0_to_use: []const u8 = arg0.slice();

// This mimicks libuv's behavior, which mimicks execvpe
// Only resolve from $PATH when the command is not an absolute path
Expand All @@ -1757,7 +1754,7 @@ pub const Subprocess = struct {

return .{
.argv0 = actual_argv0,
.arg0 = try allocator.dupeZ(u8, arg0.slice()),
.arg0 = if (pretend_argv0) |p| try allocator.dupeZ(u8, bun.sliceTo(p, 0)) else try allocator.dupeZ(u8, arg0.slice()),
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/javascript.zig
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ pub fn Bun__Process__send_(globalObject: *JSGlobalObject, callFrame: *JSC.CallFr

if (good) {
if (callback.isFunction()) {
Bun__Process__queueNextTick1(zigGlobal, callback, .zero);
Bun__Process__queueNextTick1(zigGlobal, callback, .null);
}
} else {
const ex = globalObject.createTypeErrorInstance("process.send() failed", .{});
Expand Down
115 changes: 62 additions & 53 deletions src/js/node/child_process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ function spawnSync(file, args, options) {
stdio: bunStdio,
windowsVerbatimArguments: options.windowsVerbatimArguments,
windowsHide: options.windowsHide,
argv0: options.argv0,
});
} catch (err) {
error = err;
Expand Down Expand Up @@ -959,11 +960,7 @@ function normalizeSpawnArguments(file, args, options) {
}

// Handle argv0
if (typeof options.argv0 === "string") {
ArrayPrototypeUnshift.$call(args, options.argv0);
} else {
ArrayPrototypeUnshift.$call(args, file);
}
ArrayPrototypeUnshift.$call(args, file);

const env = options.env || process.env;
const envPairs = {};
Expand Down Expand Up @@ -1010,6 +1007,7 @@ function normalizeSpawnArguments(file, args, options) {
file,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!windowsVerbatimArguments,
argv0: options.argv0,
};
}

Expand Down Expand Up @@ -1244,8 +1242,6 @@ class ChildProcess extends EventEmitter {
validateString(options.file, "options.file");
// NOTE: This is confusing... So node allows you to pass a file name
// But also allows you to pass a command in the args and it should execute
// To add another layer of confusion, they also give the option to pass an explicit "argv0"
// which overrides the actual command of the spawned process...
var file;
file = this.spawnfile = options.file;

Expand All @@ -1259,9 +1255,9 @@ class ChildProcess extends EventEmitter {

const stdio = options.stdio || ["pipe", "pipe", "pipe"];
const bunStdio = getBunStdioFromOptions(stdio);
const argv0 = file || options.argv0;
const argv0 = options.argv0 || file;

const has_ipc = $isJSArray(stdio) && stdio[3] === "ipc";
const has_ipc = $isJSArray(stdio) && stdio.includes("ipc");
var env = options.envPairs || process.env;

const detachedOption = options.detached;
Expand All @@ -1270,55 +1266,64 @@ class ChildProcess extends EventEmitter {
const stdioCount = stdio.length;
const hasSocketsToEagerlyLoad = stdioCount >= 3;

this.#handle = Bun.spawn({
cmd: spawnargs,
stdio: bunStdio,
cwd: options.cwd || undefined,
env: env,
detached: typeof detachedOption !== "undefined" ? !!detachedOption : false,
onExit: (handle, exitCode, signalCode, err) => {
this.#handle = handle;
this.pid = this.#handle.pid;
$debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid);

if (hasSocketsToEagerlyLoad) {
process.nextTick(() => {
this.stdio;
$debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid);
});
}
try {
this.#handle = Bun.spawn({
cmd: spawnargs,
stdio: bunStdio,
cwd: options.cwd || undefined,
env: env,
detached: typeof detachedOption !== "undefined" ? !!detachedOption : false,
onExit: (handle, exitCode, signalCode, err) => {
this.#handle = handle;
this.pid = this.#handle.pid;
$debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid);

if (hasSocketsToEagerlyLoad) {
process.nextTick(() => {
this.stdio;
$debug("ChildProcess: onExit", exitCode, signalCode, err, this.pid);
});
}

process.nextTick(
(exitCode, signalCode, err) => this.#handleOnExit(exitCode, signalCode, err),
exitCode,
signalCode,
err,
);
},
lazy: true,
ipc: has_ipc ? this.#emitIpcMessage.bind(this) : undefined,
onDisconnect: has_ipc ? ok => this.#disconnect(ok) : undefined,
serialization,
argv0,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!options.windowsVerbatimArguments,
});
this.pid = this.#handle.pid;
process.nextTick(
(exitCode, signalCode, err) => this.#handleOnExit(exitCode, signalCode, err),
exitCode,
signalCode,
err,
);
},
lazy: true,
ipc: has_ipc ? this.#emitIpcMessage.bind(this) : undefined,
onDisconnect: has_ipc ? ok => this.#disconnect(ok) : undefined,
serialization,
argv0,
windowsHide: !!options.windowsHide,
windowsVerbatimArguments: !!options.windowsVerbatimArguments,
});
this.pid = this.#handle.pid;

$debug("ChildProcess: spawn", this.pid, spawnargs);
$debug("ChildProcess: spawn", this.pid, spawnargs);

onSpawnNT(this);
onSpawnNT(this);

if (has_ipc) {
this.send = this.#send;
this.disconnect = this.#disconnect;
if (options[kFromNode]) this.#closesNeeded += 1;
}
if (has_ipc) {
this.send = this.#send;
this.disconnect = this.#disconnect;
if (options[kFromNode]) this.#closesNeeded += 1;
}

if (hasSocketsToEagerlyLoad) {
for (let item of this.stdio) {
item?.ref?.();
if (hasSocketsToEagerlyLoad) {
for (let item of this.stdio) {
item?.ref?.();
}
}
} catch (ex) {
if ((ex as Error).name !== "SystemError") throw ex;
this.#handle = null;
process.nextTick(() => {
this.emit("error", ex);
this.emit("close", (ex as SystemError).errno ?? -1);
});
}
}

Expand Down Expand Up @@ -1352,7 +1357,7 @@ class ChildProcess extends EventEmitter {
// Bun does not handle handles yet
try {
this.#handle.send(message);
if (callback) process.nextTick(callback);
if (callback) process.nextTick(callback, null);
return true;
} catch (error) {
if (callback) {
Expand Down Expand Up @@ -1599,6 +1604,10 @@ class ShimmedStdioOutStream extends EventEmitter {
destroy() {
return this;
}

setEncoding() {
return this;
}
}

//------------------------------------------------------------------------------
Expand Down
23 changes: 23 additions & 0 deletions test/js/node/test/common/net.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const net = require('net');

const options = { port: 0, reusePort: true };

function checkSupportReusePort() {
return new Promise((resolve, reject) => {
const server = net.createServer().listen(options);
server.on('listening', () => {
server.close(resolve);
});
server.on('error', (err) => {
console.log('The `reusePort` option is not supported:', err.message);
server.close();
reject(err);
});
});
}

module.exports = {
checkSupportReusePort,
options,
};
102 changes: 102 additions & 0 deletions test/js/node/test/parallel/test-child-process-cwd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright Joyent, Inc. and other Node contributors.
//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to permit
// persons to whom the Software is furnished to do so, subject to the
// following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';

const common = require('../common');
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

const assert = require('assert');
const { spawn } = require('child_process');

// Spawns 'pwd' with given options, then test
// - whether the child pid is undefined or number,
// - whether the exit code equals expectCode,
// - optionally whether the trimmed stdout result matches expectData
function testCwd(options, expectPidType, expectCode = 0, expectData) {
const child = spawn(...common.pwdCommand, options);

assert.strictEqual(typeof child.pid, expectPidType);

child.stdout.setEncoding('utf8');

// No need to assert callback since `data` is asserted.
let data = '';
child.stdout.on('data', function(chunk) {
data += chunk;
});

// Can't assert callback, as stayed in to API:
// _The 'exit' event may or may not fire after an error has occurred._
child.on('exit', function(code, signal) {
assert.strictEqual(code, expectCode);
});

child.on('close', common.mustCall(function() {
if (expectData) {
// In Windows, compare without considering case
if (common.isWindows) {
assert.strictEqual(data.trim().toLowerCase(), expectData.toLowerCase());
} else {
assert.strictEqual(data.trim(), expectData);
}
}
}));

return child;
}


// Assume does-not-exist doesn't exist, expect exitCode=-1 and errno=ENOENT
{
testCwd({ cwd: 'does-not-exist' }, 'undefined', -1)
.on('error', common.mustCall(function(e) {
assert.strictEqual(e.code, 'ENOENT');
}));
}

{
assert.throws(() => {
testCwd({
cwd: new URL('http://example.com/'),
}, 'number', 0, tmpdir.path);
}, /The URL must be of scheme file/);

if (process.platform !== 'win32') {
assert.throws(() => {
testCwd({
cwd: new URL('file://host/dev/null'),
}, 'number', 0, tmpdir.path);
}, /File URL host must be "localhost" or empty on/);
}
}

// Assume these exist, and 'pwd' gives us the right directory back
testCwd({ cwd: tmpdir.path }, 'number', 0, tmpdir.path);
const shouldExistDir = common.isWindows ? process.env.windir : '/dev';
testCwd({ cwd: shouldExistDir }, 'number', 0, shouldExistDir);
testCwd({ cwd: tmpdir.fileURL() }, 'number', 0, tmpdir.path);

// Spawn() shouldn't try to chdir() to invalid arg, so this should just work
testCwd({ cwd: '' }, 'number');
testCwd({ cwd: undefined }, 'number');
testCwd({ cwd: null }, 'number');
Loading