Skip to content

Commit cb1a588

Browse files
Merge pull request #3937 from github/robertbrignull/waitForProcessing_backoff
Change waitForProcessing to use exponential backoff
2 parents ba47406 + 6047ac7 commit cb1a588

2 files changed

Lines changed: 44 additions & 32 deletions

File tree

lib/entry-points.js

Lines changed: 17 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/upload-lib.ts

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,10 @@ function dumpSarifFile(
829829
fs.writeFileSync(outputFile, sarifPayload);
830830
}
831831

832-
const STATUS_CHECK_FREQUENCY_MILLISECONDS = 5 * 1000;
833-
const STATUS_CHECK_TIMEOUT_MILLISECONDS = 2 * 60 * 1000;
832+
// Should lead to status checks after 5s, 15s, 35s, 75s, and 155s.
833+
const STATUS_CHECK_INITIAL_BACKOFF_MILLISECONDS = 5 * 1000;
834+
const STATUS_CHECK_BACKOFF_MULTIPLIER = 2;
835+
const STATUS_CHECK_MAX_TRIES = 5;
834836

835837
type ProcessingStatus = "pending" | "complete" | "failed";
836838

@@ -854,20 +856,17 @@ export async function waitForProcessing(
854856
try {
855857
const client = api.getApiClient();
856858

857-
const statusCheckingStarted = Date.now();
858-
while (true) {
859-
if (
860-
Date.now() >
861-
statusCheckingStarted + STATUS_CHECK_TIMEOUT_MILLISECONDS
862-
) {
863-
// If the analysis hasn't finished processing in the allotted time, we continue anyway rather than failing.
864-
// It's possible the analysis will eventually finish processing, but it's not worth spending more
865-
// Actions time waiting.
866-
logger.warning(
867-
"Timed out waiting for analysis to finish processing. Continuing.",
868-
);
869-
break;
870-
}
859+
// Do an initial wait because processing will always take a minimum of 2-3 seconds
860+
let statusCheckBackoff = STATUS_CHECK_INITIAL_BACKOFF_MILLISECONDS;
861+
if (process.env["NODE_ENV"] !== "test") {
862+
await util.delay(statusCheckBackoff, { allowProcessExit: false });
863+
}
864+
865+
for (
866+
let statusCheckCount = 1;
867+
statusCheckCount <= STATUS_CHECK_MAX_TRIES;
868+
statusCheckCount++
869+
) {
871870
let response: OctokitResponse<any> | undefined = undefined;
872871
try {
873872
response = await client.request(
@@ -912,9 +911,18 @@ export async function waitForProcessing(
912911
util.assertNever(status);
913912
}
914913

915-
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS, {
916-
allowProcessExit: false,
917-
});
914+
if (statusCheckCount === STATUS_CHECK_MAX_TRIES) {
915+
// If the analysis hasn't finished processing in the allotted time, we continue anyway rather than failing.
916+
// It's possible the analysis will eventually finish processing, but it's not worth spending more
917+
// Actions time waiting.
918+
logger.warning(
919+
"Timed out waiting for analysis to finish processing. Continuing.",
920+
);
921+
break;
922+
} else {
923+
statusCheckBackoff *= STATUS_CHECK_BACKOFF_MULTIPLIER;
924+
await util.delay(statusCheckBackoff, { allowProcessExit: false });
925+
}
918926
}
919927
} finally {
920928
logger.endGroup();

0 commit comments

Comments
 (0)