Skip to content

Commit

Permalink
Handle cases where an event is received from tsc while we're still ki…
Browse files Browse the repository at this point in the history
…lling processes after the last event. Fixes gilamran#87.
  • Loading branch information
david-alexander committed Sep 9, 2022
1 parent 8f034b2 commit 49243d7
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 8 deletions.
30 changes: 25 additions & 5 deletions lib/tsc-watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ const run = require('./runner');
const { extractArgs } = require('./args-manager');
const { manipulate, detectState, deleteClear, print } = require('./stdout-manipulator');
const readline = require('readline');
const CancellationToken = require('cancellationtoken').default;

let firstTime = true;
let firstSuccessKiller = null;
let successKiller = null;
let failureKiller = null;
let compilationStartedKiller = null;
let compilationCompleteKiller = null;
let cancelKillProcesses = () => {};

const {
onFirstSuccessCommand,
Expand All @@ -29,14 +31,32 @@ const {
args,
} = extractArgs(process.argv);

function silentlyHandleCancellation(error)
{
if (error instanceof CancellationToken.CancellationError)
{

}
else
{
// Rethrow all other errors.
throw e;
}
}

function killProcesses(killAll) {
return Promise.all([
// Cancel any earlier invocation of this function that is still in progress.
cancelKillProcesses();
const { cancel, token } = CancellationToken.create();
cancelKillProcesses = cancel;

return token.racePromise(Promise.all([
killAll && firstSuccessKiller ? firstSuccessKiller() : null,
successKiller ? successKiller() : null,
failureKiller ? failureKiller() : null,
compilationStartedKiller ? compilationStartedKiller() : null,
compilationCompleteKiller ? compilationCompleteKiller() : null,
]);
]));
}

function runOnCompilationStarted() {
Expand Down Expand Up @@ -118,7 +138,7 @@ rl.on('line', function (input) {
killProcesses(false).then(() => {
runOnCompilationStarted();
Signal.emitStarted();
});
}).catch(silentlyHandleCancellation);
}

if (compilationComplete) {
Expand All @@ -138,7 +158,7 @@ rl.on('line', function (input) {
Signal.emitSuccess();
runOnSuccessCommand();
}
});
}).catch(silentlyHandleCancellation);
}
});

Expand Down Expand Up @@ -193,7 +213,7 @@ const Signal = {

nodeCleanup((_exitCode, signal) => {
tscProcess.kill(signal);
killProcesses(true).then(() => process.exit());
killProcesses(true).catch(silentlyHandleCancellation).finally(() => process.exit());
// don't call cleanup handler again
nodeCleanup.uninstall();
return false;
Expand Down
56 changes: 54 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"printWidth": 100
},
"dependencies": {
"cancellationtoken": "^2.2.0",
"cross-spawn": "^7.0.3",
"node-cleanup": "^2.1.2",
"ps-tree": "^1.2.0",
Expand All @@ -43,6 +44,7 @@
},
"devDependencies": {
"chai": "^4.3.4",
"find-process": "^1.4.7",
"fs-extra": "^10.0.0",
"mocha": "^9.1.3",
"sinon": "^12.0.1",
Expand Down
5 changes: 5 additions & 0 deletions test-commands/unkillable-command.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
process.on('SIGTERM', () => {
console.log('unkillable-command ignored SIGTERM');
});
console.log('unkillable-command started');
process.stdin.resume(); // stop the process from exiting
7 changes: 6 additions & 1 deletion test/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@ class Driver {
return this;
}

startWatch({ failFirst, pretty } = {}) {
startWatch({ failFirst, pretty, command } = {}) {
const params = ['--noClear', '--out', './tmp/output.js', failFirst ? FAIL_FILE_PATH : SUCCESS_FILE_PATH];
if (pretty) {
params.push('--pretty');
}
if (command === 'unkillable-command')
{
params.push('--onSuccess');
params.push(`${process.execPath} ./test-commands/unkillable-command.js`);
}
this.proc = fork('./lib/tsc-watch.js', params, { stdio: 'inherit' });

this.subscriptions.forEach((handler, evName) => this.proc.on('message', event => (evName === event ? handler(event) : noop())));
Expand Down
24 changes: 24 additions & 0 deletions test/tsc-watch.it.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,28 @@ describe('TSC-Watch child process messages', () => {
await driver.wait(3000);
expect(listener.callCount).to.be.equal(1);
});

it('Should not start extra onSuccess commands when previous ones are slow to die (gilamran/tsc-watch#87)', async () => {
const findProcess = require('find-process');

driver.startWatch({ command: 'unkillable-command' });
await driver.wait(3000);

const [ unkillable ] = await findProcess('name', 'unkillable-command.js', true);

// Trigger a new onSuccess command, but don't allow the first one to die just yet.
driver.modifyAndSucceedAfter(0);
await driver.wait(3000);

// Try to trigger *another* new onSuccess command while still waiting for the first one to die.
driver.modifyAndSucceedAfter(0);
await driver.wait(3000);

// Finally let the old first onSuccess command die.
process.kill(unkillable.pid, 'SIGKILL');
await driver.wait(100);

// There should only be one onSuccess command running (the one triggered by the final successful compile).
expect(await findProcess('name', 'unkillable-command.js', true)).to.have.length(1);
});
});

0 comments on commit 49243d7

Please sign in to comment.