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

feat(watch): log reason for rerun & improved exit handling #412

Merged
merged 8 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 67 additions & 52 deletions src/watch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { constants as osConstants } from 'os';
import path from 'path';
import { command } from 'cleye';
import { watch } from 'chokidar';
import { lightMagenta, lightGreen, yellow } from 'kolorist';
import { run } from '../run.js';
import {
removeArgvFlags,
Expand All @@ -15,18 +16,6 @@ import {
log,
} from './utils.js';

const killProcess = async (
childProcess: ChildProcess,
) => {
const waitForExit = new Promise((resolve) => {
childProcess.on('exit', resolve);
});

childProcess.kill();

await waitForExit;
};

const flags = {
noCache: {
type: Boolean,
Expand Down Expand Up @@ -74,8 +63,13 @@ export const watchCommand = command({
};

let runProcess: ChildProcess | undefined;
let exiting = false;

const spawnProcess = () => {
if (exiting) {
return;
}

const childProcess = run(rawArgvs, options);

childProcess.on('message', (data) => {
Expand All @@ -95,7 +89,6 @@ export const watchCommand = command({
);

if (path.isAbsolute(dependencyPath)) {
// console.log('adding', dependencyPath);
watcher.add(dependencyPath);
}
}
Expand All @@ -104,10 +97,40 @@ export const watchCommand = command({
return childProcess;
};

let waitingExits = false;
const reRun = debounce(async () => {
if (waitingExits) {
log('forcing restart');
let waitingChildExit = false;

const killProcess = async (
childProcess: ChildProcess,
signal: NodeJS.Signals = 'SIGTERM',
forceKillOnTimeout = 5000,
) => {
let exited = false;
const waitForExit = new Promise<number | null>((resolve) => {
childProcess.on('exit', (exitCode) => {
exited = true;
waitingChildExit = false;
resolve(exitCode);
});
});

waitingChildExit = true;
childProcess.kill(signal);

setTimeout(() => {
if (!exited) {
log(yellow(`Process didn't exit in ${Math.floor(forceKillOnTimeout / 1000)}s. Force killing...`));
childProcess.kill('SIGKILL');
}
}, forceKillOnTimeout);

return await waitForExit;
};

const reRun = debounce(async (event?: string, filePath?: string) => {
const reason = event ? `${event ? lightMagenta(event) : ''}${filePath ? ` in ${lightGreen(`./${filePath}`)}` : ''}` : '';

if (waitingChildExit) {
log(reason, yellow('Process hasn\'t exited. Killing process...'));
runProcess!.kill('SIGKILL');
return;
}
Expand All @@ -116,12 +139,10 @@ export const watchCommand = command({
if (runProcess) {
// If process still running
if (runProcess.exitCode === null) {
log('restarting');
waitingExits = true;
log(reason, yellow('Restarting...'));
await killProcess(runProcess);
waitingExits = false;
} else {
log('rerunning');
log(reason, yellow('Rerunning...'));
}

if (options.clearScreen) {
Expand All @@ -134,39 +155,33 @@ export const watchCommand = command({

reRun();

function exit(signal: NodeJS.Signals) {
/**
* In CLI mode where there is only one run, we can inherit the child's exit code.
* But in watch mode, the exit code should reflect the kill signal.
*/

process.exit(
/**
* https://nodejs.org/api/process.html#exit-codes
* >128 Signal Exits: If Node.js receives a fatal signal such as SIGKILL or SIGHUP,
* then its exit code will be 128 plus the value of the signal code. This is a
* standard POSIX practice, since exit codes are defined to be 7-bit integers, and
* signal exits set the high-order bit, and then contain the value of the signal
* code. For example, signal SIGABRT has value 6, so the expected exit code will be
* 128 + 6, or 134.
*/
128 + osConstants.signals[signal],
);
}

function relaySignal(signal: NodeJS.Signals) {
// Child is still running
if (runProcess && runProcess.exitCode === null) {
// Wait for child to exit
runProcess.on('close', () => exit(signal));
runProcess.kill(signal);
const relaySignal = (signal: NodeJS.Signals) => {
// Disable further spawns
exiting = true;

// Child is still running, kill it
if (runProcess?.exitCode === null) {
if (waitingChildExit) {
log(yellow('Previous process hasn\'t exited yet. Force killing...'));
}

killProcess(
runProcess,
// Second Ctrl+C force kills
waitingChildExit ? 'SIGKILL' : signal,
).then(
(exitCode) => {
process.exit(exitCode ?? 0);
},
() => {},
);
} else {
exit(signal);
process.exit(osConstants.signals[signal]);
}
}
};

process.once('SIGINT', relaySignal);
process.once('SIGTERM', relaySignal);
process.on('SIGINT', relaySignal);
process.on('SIGTERM', relaySignal);

/**
* Ideally, we can get a list of files loaded from the run above
Expand Down Expand Up @@ -198,5 +213,5 @@ export const watchCommand = command({
).on('all', reRun);

// On "Return" key
process.stdin.on('data', reRun);
process.stdin.on('data', () => reRun('Return key'));
});
15 changes: 8 additions & 7 deletions src/watch/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,21 @@ export const log = (...messages: unknown[]) => console.log(
// https://github.com/sindresorhus/ansi-escapes/blob/2b3b59c56ff77a/index.js#L80
export const clearScreen = '\u001Bc';

export function debounce(
originalFunction: () => void,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const debounce = <T extends (this: unknown, ...args: any[]) => void>(
originalFunction: T,
duration: number,
) {
): T => {
let timeout: NodeJS.Timeout | undefined;

return () => {
return function () {
if (timeout) {
clearTimeout(timeout);
}

timeout = setTimeout(
() => originalFunction(),
() => Reflect.apply(originalFunction, this, arguments),
duration,
);
};
}
} as T;
};
2 changes: 2 additions & 0 deletions tests/specs/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import path from 'path';
import { setTimeout } from 'timers/promises';
import { testSuite, expect } from 'manten';
import { createFixture } from 'fs-fixture';
import stripAnsi from 'strip-ansi';
import { tsx } from '../utils/tsx.js';
import { processInteract } from '../utils/process-interact.js';

Expand Down Expand Up @@ -62,6 +63,7 @@ export default testSuite(async ({ describe }) => {
return true;
}
},
data => stripAnsi(data).includes('[tsx] change in ./value.js Rerunning...\n'),
data => data.includes('goodbye world\n'),
],
5000,
Expand Down
Loading