Skip to content

Commit

Permalink
Fix ESM node processes being unable to fork into other scripts
Browse files Browse the repository at this point in the history
Currently, Node processes instantiated through the `--esm` flag result
in a child process being created so that the ESM loader can be
registered. This works fine and is reasonable.

The child process approach to register ESM hooks currently prevents
the NodeJS `fork` method from being used because the `execArgv`
propagated into forked processes causes `ts-node` (which is also
propagated as child exec script -- this is good because it allows nested
type resolution to work) to always execute the original entry-point,
causing potential infinite loops because the designated fork module
script is not executed as expected.

This commit fixes this by not encoding the entry-point information into
the state that is captured as part of the `execArgv`. Instead the
entry-point information is always retrieved from the parsed rest command
line arguments in the final stage (`phase4`).

Fixes #1812.
  • Loading branch information
devversion committed Jun 24, 2022
1 parent bf13086 commit 61e47c5
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 33 deletions.
122 changes: 90 additions & 32 deletions src/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function main(
const state: BootstrapState = {
shouldUseChildProcess: false,
isInChildProcess: false,
entrypoint: __filename,
tsNodeScript: __filename,
parseArgvResult: args,
};
return bootstrap(state);
Expand All @@ -62,7 +62,7 @@ export function main(
export interface BootstrapState {
isInChildProcess: boolean;
shouldUseChildProcess: boolean;
entrypoint: string;
tsNodeScript: string;
parseArgvResult: ReturnType<typeof parseArgv>;
phase2Result?: ReturnType<typeof phase2>;
phase3Result?: ReturnType<typeof phase3>;
Expand Down Expand Up @@ -319,28 +319,16 @@ Options:
process.exit(0);
}

// Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint
// This is complicated because node's behavior is complicated
// `node -e code -i ./script.js` ignores -e
const executeEval = code != null && !(interactive && restArgs.length);
const executeEntrypoint = !executeEval && restArgs.length > 0;
const executeRepl =
!executeEntrypoint &&
(interactive || (process.stdin.isTTY && !executeEval));
const executeStdin = !executeEval && !executeRepl && !executeEntrypoint;

const cwd = cwdArg || process.cwd();
/** Unresolved. May point to a symlink, not realpath. May be missing file extension */
const scriptPath = executeEntrypoint ? resolve(cwd, restArgs[0]) : undefined;

if (esm) payload.shouldUseChildProcess = true;
// If ESM is explicitly enabled through the flag, stage3 should be run in a child process
// with the ESM loaders configured.
if (esm) {
payload.shouldUseChildProcess = true;
}

return {
executeEval,
executeEntrypoint,
executeRepl,
executeStdin,
cwd,
scriptPath,
};
}

Expand Down Expand Up @@ -372,7 +360,18 @@ function phase3(payload: BootstrapState) {
esm,
experimentalSpecifierResolution,
} = payload.parseArgvResult;
const { cwd, scriptPath } = payload.phase2Result!;
const { cwd } = payload.phase2Result!;

// NOTE: When we transition to a child process for ESM, the entry-point script determined
// here might not be the one used later in `phase4`. This can happen when we execute the
// original entry-point but then the process forks itself using e.g. `child_process.fork`.
// We will always use the original TS project in forked processes anyway, so it is
// expected and acceptable to retrieve the entry-point information here in `phase2`.
// See: https://github.com/TypeStrong/ts-node/issues/1812.
const { entryPointPath } = getEntryPointInfo(
payload.parseArgvResult!,
payload.phase2Result!
);

const preloadedConfig = findAndReadConfig({
cwd,
Expand All @@ -387,7 +386,12 @@ function phase3(payload: BootstrapState) {
compilerHost,
ignore,
logError,
projectSearchDir: getProjectSearchDir(cwd, scriptMode, cwdMode, scriptPath),
projectSearchDir: getProjectSearchDir(
cwd,
scriptMode,
cwdMode,
entryPointPath
),
project,
skipProject,
skipIgnore,
Expand All @@ -403,23 +407,76 @@ function phase3(payload: BootstrapState) {
experimentalSpecifierResolution as ExperimentalSpecifierResolution,
});

if (preloadedConfig.options.esm) payload.shouldUseChildProcess = true;
// If ESM is enabled through the parsed tsconfig, stage4 should be run in a child
// process with the ESM loaders configured.
if (preloadedConfig.options.esm) {
payload.shouldUseChildProcess = true;
}

return { preloadedConfig };
}

/**
* Determines the entry-point information from the argv and phase2 result. This
* method will be invoked in two places:
*
* 1. In phase 3 to be able to find a project from the potential entry-point script.
* 2. In phase 4 to determine the actual entry-point script.
*
* Note that we need to explicitly re-resolve the entry-point information in the final
* stage because the previous stage information could be modified when the bootstrap
* invocation transitioned into a child process for ESM.
*
* Stages before (phase 4) can and will be cached by the child process through the Brotli
* configuration and entry-point information is only reliable in the final phase. More
* details can be found in here: https://github.com/TypeStrong/ts-node/issues/1812.
*/
function getEntryPointInfo(
argvResult: NonNullable<BootstrapState['parseArgvResult']>,
phase2Result: NonNullable<BootstrapState['phase2Result']>
) {
const { code, interactive, restArgs } = argvResult;
const { cwd } = phase2Result;
// Figure out which we are executing: piped stdin, --eval, REPL, and/or entrypoint
// This is complicated because node's behavior is complicated
// `node -e code -i ./script.js` ignores -e
const executeEval = code != null && !(interactive && restArgs.length);
const executeEntrypoint = !executeEval && restArgs.length > 0;
const executeRepl =
!executeEntrypoint &&
(interactive || (process.stdin.isTTY && !executeEval));
const executeStdin = !executeEval && !executeRepl && !executeEntrypoint;

/** Unresolved. May point to a symlink, not realpath. May be missing file extension */
const entryPointPath = executeEntrypoint
? resolve(cwd, restArgs[0])
: undefined;

return {
executeEval,
executeEntrypoint,
executeRepl,
executeStdin,
entryPointPath,
};
}

function phase4(payload: BootstrapState) {
const { isInChildProcess, entrypoint } = payload;
const { isInChildProcess, tsNodeScript } = payload;
const { version, showConfig, restArgs, code, print, argv } =
payload.parseArgvResult;
const { cwd } = payload.phase2Result!;
const { preloadedConfig } = payload.phase3Result!;

const {
entryPointPath,
executeEntrypoint,
executeEval,
cwd,
executeStdin,
executeRepl,
executeEntrypoint,
scriptPath,
} = payload.phase2Result!;
const { preloadedConfig } = payload.phase3Result!;
executeStdin,
} = getEntryPointInfo(payload.parseArgvResult!, payload.phase2Result!);

/**
* <repl>, [stdin], and [eval] are all essentially virtual files that do not exist on disc and are backed by a REPL
* service to handle eval-ing of code.
Expand Down Expand Up @@ -566,12 +623,13 @@ function phase4(payload: BootstrapState) {

// Prepend `ts-node` arguments to CLI for child processes.
process.execArgv.push(
entrypoint,
tsNodeScript,
...argv.slice(2, argv.length - restArgs.length)
);

// TODO this comes from BoostrapState
process.argv = [process.argv[1]]
.concat(executeEntrypoint ? ([scriptPath] as string[]) : [])
.concat(executeEntrypoint ? ([entryPointPath] as string[]) : [])
.concat(restArgs.slice(executeEntrypoint ? 1 : 0));

// Execute the main contents (either eval, script or piped).
Expand Down
3 changes: 2 additions & 1 deletion src/child/child-entrypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ const base64Payload = base64ConfigArg.slice(argPrefix.length);
const payload = JSON.parse(
brotliDecompressSync(Buffer.from(base64Payload, 'base64')).toString()
) as BootstrapState;

payload.isInChildProcess = true;
payload.entrypoint = __filename;
payload.tsNodeScript = __filename;
payload.parseArgvResult.argv = process.argv;
payload.parseArgvResult.restArgs = process.argv.slice(3);

Expand Down
22 changes: 22 additions & 0 deletions src/test/esm-loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,28 @@ test.suite('esm', (test) => {
});
}

test.suite('esm child process and forking', (test) => {
test('should be able to fork vanilla NodeJS script', async () => {
const { err, stdout, stderr } = await exec(
`${BIN_PATH} --esm ./esm-child-process/process-forking/index.mts`
);

expect(err).toBe(null);
expect(stdout.trim()).toBe('Passing: from main');
expect(stderr).toBe('');
});

test('should be able to fork into a nested TypeScript ESM script', async () => {
const { err, stdout, stderr } = await exec(
`${BIN_PATH} --esm ./esm-child-process/process-forking-nested-esm/index.mts`
);

expect(err).toBe(null);
expect(stdout.trim()).toBe('Passing: from main');
expect(stderr).toBe('');
});
});

test.suite('parent passes signals to child', (test) => {
test.runSerially();

Expand Down
23 changes: 23 additions & 0 deletions tests/esm-child-process/process-forking-nested-esm/index.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { fork } from 'child_process';
import { dirname, join } from 'path';
import { fileURLToPath } from 'url';

// Initially set the exit code to non-zero. We only set it to `0` when the
// worker process finishes properly with the expected stdout message.
process.exitCode = 1;

const projectDir = dirname(fileURLToPath(import.meta.url));
const workerProcess = fork(join(projectDir, 'worker.mts'), [], {
stdio: 'pipe',
});

let stdout = '';

workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8')));
workerProcess.on('error', () => (process.exitCode = 1));
workerProcess.on('close', (status, signal) => {
if (status === 0 && signal === null && stdout.trim() === 'Works') {
console.log('Passing: from main');
process.exitCode = 0;
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"module": "Node16"
}
}
3 changes: 3 additions & 0 deletions tests/esm-child-process/process-forking-nested-esm/worker.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const message: string = 'Works';

console.log(message);
23 changes: 23 additions & 0 deletions tests/esm-child-process/process-forking/index.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { fork } from 'child_process';
import { dirname, join } from 'path';
import { fileURLToPath } from 'url';

// Initially set the exit code to non-zero. We only set it to `0` when the
// worker process finishes properly with the expected stdout message.
process.exitCode = 1;

const projectDir = dirname(fileURLToPath(import.meta.url));
const workerProcess = fork(join(projectDir, 'worker.mjs'), [], {
stdio: 'pipe',
});

let stdout = '';

workerProcess.stdout.on('data', (chunk) => (stdout += chunk.toString('utf8')));
workerProcess.on('error', () => (process.exitCode = 1));
workerProcess.on('close', (status, signal) => {
if (status === 0 && signal === null && stdout.trim() === 'Works') {
console.log('Passing: from main');
process.exitCode = 0;
}
});
5 changes: 5 additions & 0 deletions tests/esm-child-process/process-forking/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"module": "Node16"
}
}
1 change: 1 addition & 0 deletions tests/esm-child-process/process-forking/worker.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('Works');

0 comments on commit 61e47c5

Please sign in to comment.