Avoid unref-ing timer while awaiting status upload

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.
This commit is contained in:
Andrew Eisenberg 2023-02-13 13:26:01 -08:00
parent e187d074ed
commit a2487fb969
6 changed files with 43 additions and 16 deletions

4
lib/upload-lib.js generated
View file

@ -330,7 +330,9 @@ async function waitForProcessing(repositoryNwo, sarifID, logger, options = {
else { else {
util.assertNever(status); util.assertNever(status);
} }
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS); await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS, {
allowProcessExit: false,
});
} }
} }
finally { finally {

File diff suppressed because one or more lines are too long

22
lib/util.js generated
View file

@ -455,10 +455,20 @@ async function bundleDb(config, language, codeql, dbName) {
return databaseBundlePath; return databaseBundlePath;
} }
exports.bundleDb = bundleDb; exports.bundleDb = bundleDb;
async function delay(milliseconds) { /**
// Immediately `unref` the timer such that it only prevents the process from exiting if the * @param milliseconds time to delay
// surrounding promise is being awaited. * @param opts options
return new Promise((resolve) => setTimeout(resolve, milliseconds).unref()); * @param opts.allowProcessExit if true, the timer will not prevent the process from exiting
*/
async function delay(milliseconds, { allowProcessExit }) {
return new Promise((resolve) => {
const timer = setTimeout(resolve, milliseconds);
if (allowProcessExit) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
timer.unref();
}
});
} }
exports.delay = delay; exports.delay = delay;
function isGoodVersion(versionSpec) { function isGoodVersion(versionSpec) {
@ -636,7 +646,7 @@ async function withTimeout(timeoutMs, promise, onTimeout) {
return result; return result;
}; };
const timeoutTask = async () => { const timeoutTask = async () => {
await delay(timeoutMs); await delay(timeoutMs, { allowProcessExit: true });
if (!finished) { if (!finished) {
// Workaround: While the promise racing below will allow the main code // Workaround: While the promise racing below will allow the main code
// to continue, the process won't normally exit until the asynchronous // to continue, the process won't normally exit until the asynchronous
@ -659,7 +669,7 @@ exports.withTimeout = withTimeout;
async function checkForTimeout() { async function checkForTimeout() {
if (hadTimeout === true) { if (hadTimeout === true) {
core.info("A timeout occurred, force exiting the process after 30 seconds to prevent hanging."); core.info("A timeout occurred, force exiting the process after 30 seconds to prevent hanging.");
await delay(30000); await delay(30000, { allowProcessExit: true });
process.exit(); process.exit();
} }
} }

File diff suppressed because one or more lines are too long

View file

@ -463,7 +463,9 @@ export async function waitForProcessing(
util.assertNever(status); util.assertNever(status);
} }
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS); await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS, {
allowProcessExit: false,
});
} }
} finally { } finally {
logger.endGroup(); logger.endGroup();

View file

@ -548,10 +548,23 @@ export async function bundleDb(
return databaseBundlePath; return databaseBundlePath;
} }
export async function delay(milliseconds: number) { /**
// Immediately `unref` the timer such that it only prevents the process from exiting if the * @param milliseconds time to delay
// surrounding promise is being awaited. * @param opts options
return new Promise((resolve) => setTimeout(resolve, milliseconds).unref()); * @param opts.allowProcessExit if true, the timer will not prevent the process from exiting
*/
export async function delay(
milliseconds: number,
{ allowProcessExit }: { allowProcessExit: boolean }
) {
return new Promise((resolve) => {
const timer = setTimeout(resolve, milliseconds);
if (allowProcessExit) {
// 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) { export function isGoodVersion(versionSpec: string) {
@ -748,7 +761,7 @@ export async function withTimeout<T>(
return result; return result;
}; };
const timeoutTask = async () => { const timeoutTask = async () => {
await delay(timeoutMs); await delay(timeoutMs, { allowProcessExit: true });
if (!finished) { if (!finished) {
// Workaround: While the promise racing below will allow the main code // Workaround: While the promise racing below will allow the main code
// to continue, the process won't normally exit until the asynchronous // to continue, the process won't normally exit until the asynchronous
@ -773,7 +786,7 @@ export async function checkForTimeout() {
core.info( core.info(
"A timeout occurred, force exiting the process after 30 seconds to prevent hanging." "A timeout occurred, force exiting the process after 30 seconds to prevent hanging."
); );
await delay(30_000); await delay(30_000, { allowProcessExit: true });
process.exit(); process.exit();
} }
} }