Skip to content

Commit

Permalink
fix: Retry on timeout (#106)
Browse files Browse the repository at this point in the history
* fix: do not set return value for timed out runs

Commands that are killed manually due to timeout rarely returns a
success status code (0). These codes should not be treated as errors
but simply produced because of the timeout.

* fix(windows): use variable to track timeout

Use a variable to track timeout instead of relying on SIGTERM, as
processes on Windows are not killed using signals.
  • Loading branch information
trungnt2910 committed Dec 30, 2022
1 parent 0711ba3 commit 943e742
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
14 changes: 10 additions & 4 deletions dist/index.js
Expand Up @@ -1005,14 +1005,15 @@ function runRetryCmd(inputs) {
function runCmd(attempt, inputs) {
var _a, _b;
return __awaiter(this, void 0, void 0, function () {
var end_time, executable, child;
var end_time, executable, timeout, child;
return __generator(this, function (_c) {
switch (_c.label) {
case 0:
end_time = Date.now() + (0, inputs_1.getTimeout)(inputs);
executable = getExecutable(inputs);
exit = 0;
done = false;
timeout = false;
(0, core_1.debug)("Running command ".concat(inputs.command, " on ").concat(OS, " using shell ").concat(executable));
child = attempt > 1 && inputs.new_command_on_retry
? (0, child_process_1.spawn)(inputs.new_command_on_retry, { shell: executable })
Expand All @@ -1026,13 +1027,17 @@ function runCmd(attempt, inputs) {
child.on('exit', function (code, signal) {
(0, core_1.debug)("Code: ".concat(code));
(0, core_1.debug)("Signal: ".concat(signal));
if (code && code > 0) {
exit = code;
}
// timeouts are killed manually
if (signal === 'SIGTERM') {
return;
}
// On Windows signal is null.
if (timeout) {
return;
}
if (code && code > 0) {
exit = code;
}
done = true;
});
_c.label = 1;
Expand All @@ -1045,6 +1050,7 @@ function runCmd(attempt, inputs) {
_c.label = 4;
case 4:
if (!(!done && child.pid)) return [3 /*break*/, 6];
timeout = true;
(0, tree_kill_1.default)(child.pid);
return [4 /*yield*/, (0, util_1.retryWait)(milliseconds_1.default.seconds(inputs.retry_wait_seconds))];
case 5:
Expand Down
16 changes: 13 additions & 3 deletions src/index.ts
Expand Up @@ -71,6 +71,7 @@ async function runCmd(attempt: number, inputs: Inputs) {

exit = 0;
done = false;
let timeout = false;

debug(`Running command ${inputs.command} on ${OS} using shell ${executable}`);
const child =
Expand All @@ -88,13 +89,21 @@ async function runCmd(attempt: number, inputs: Inputs) {
child.on('exit', (code, signal) => {
debug(`Code: ${code}`);
debug(`Signal: ${signal}`);
if (code && code > 0) {
exit = code;
}

// timeouts are killed manually
if (signal === 'SIGTERM') {
return;
}

// On Windows signal is null.
if (timeout) {
return;
}

if (code && code > 0) {
exit = code;
}

done = true;
});

Expand All @@ -103,6 +112,7 @@ async function runCmd(attempt: number, inputs: Inputs) {
} while (Date.now() < end_time && !done);

if (!done && child.pid) {
timeout = true;
kill(child.pid);
await retryWait(ms.seconds(inputs.retry_wait_seconds));
throw new Error(`Timeout of ${getTimeout(inputs)}ms hit`);
Expand Down

0 comments on commit 943e742

Please sign in to comment.