Skip to content

Commit

Permalink
Avoid unref-ing timer while awaiting status upload
Browse files Browse the repository at this point in the history
We had a problem where `waitForProcessing` was not completing before
the node process ends. This is because using `unref` would allow the
node process to end without having the `delay` function complete.
  • Loading branch information
aeisenberg committed Feb 13, 2023
1 parent e187d07 commit 5eb309d
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 16 deletions.
2 changes: 1 addition & 1 deletion lib/upload-lib.js

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

2 changes: 1 addition & 1 deletion lib/upload-lib.js.map

Large diffs are not rendered by default.

21 changes: 15 additions & 6 deletions lib/util.js

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

2 changes: 1 addition & 1 deletion lib/util.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/upload-lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ export async function waitForProcessing(
util.assertNever(status);
}

await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS);
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS, { unref: false });
}
} finally {
logger.endGroup();
Expand Down
21 changes: 15 additions & 6 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,19 @@ export async function bundleDb(
return databaseBundlePath;
}

export async function delay(milliseconds: number) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
return new Promise((resolve) => setTimeout(resolve, milliseconds).unref());
/**
* @param milliseconds time to delay
* @param opts.unref if true, the timer will not prevent the process from exiting
*/
export async function delay(milliseconds: number, opts: { unref: boolean }) {
return new Promise((resolve) => {
const timer = setTimeout(resolve, milliseconds);
if (opts.unref) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
timer.unref();
}
});
}

export function isGoodVersion(versionSpec: string) {
Expand Down Expand Up @@ -748,7 +757,7 @@ export async function withTimeout<T>(
return result;
};
const timeoutTask = async () => {
await delay(timeoutMs);
await delay(timeoutMs, { unref: true });
if (!finished) {
// Workaround: While the promise racing below will allow the main code
// to continue, the process won't normally exit until the asynchronous
Expand All @@ -773,7 +782,7 @@ export async function checkForTimeout() {
core.info(
"A timeout occurred, force exiting the process after 30 seconds to prevent hanging."
);
await delay(30_000);
await delay(30_000, { unref: true });
process.exit();
}
}
Expand Down

0 comments on commit 5eb309d

Please sign in to comment.