From 1da651c2196959a0403d7f49689ac6aabac909a9 Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 10:39:23 +0100 Subject: [PATCH 1/6] Add retries to the upload --- lib/upload-lib.js | 56 ++++++++++++++++++++++++++--------------- src/upload-lib.ts | 63 +++++++++++++++++++++++++++++++++-------------- 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index f28f085a4..62d49a065 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -54,6 +54,40 @@ function combineSarifFiles(sarifFiles) { return JSON.stringify(combinedSarif); } exports.combineSarifFiles = combineSarifFiles; +// Upload the given payload. +// If the request fails then this will be retry a small number of times. +async function uploadPayload(payload) { + core.info('Uploading results'); + const githubToken = core.getInput('token'); + const ph = new auth.BearerCredentialHandler(githubToken); + const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); + const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; + // Make up to 4 attempts to upload, and sleep for these + // number of seconds between each attempt. + // We don't want to backoff too much to avoid wasting action + // minutes, but just waiting a little bit could maybe help. + const backoffPeriods = [1, 5, 15]; + for (let attempt = 0; attempt <= backoffPeriods.length; attempt++) { + const res = await client.put(url, payload); + core.debug('response status: ' + res.message.statusCode); + if (res.message.statusCode === 202) { + core.info("Successfully uploaded results"); + return; + } + const requestID = res.message.headers["x-github-request-id"]; + if (attempt < backoffPeriods.length) { + // Log the failure as a warning but don't mark the action as failed yet + core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + + await res.readBody()); + // Sleep for the backoff period + await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + } + else { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + } + } +} // Uploads a single sarif file or a directory of sarif files // depending on what the path happens to refer to. async function upload(input) { @@ -112,26 +146,8 @@ async function uploadFiles(sarifFiles) { "started_at": startedAt, "tool_names": toolNames, }); - core.info('Uploading results'); - const githubToken = core.getInput('token'); - const ph = new auth.BearerCredentialHandler(githubToken); - const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); - const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; - const res = await client.put(url, payload); - const requestID = res.message.headers["x-github-request-id"]; - core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. - // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - } - else if (res.message.statusCode !== 202) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); - } - else { - core.info("Successfully uploaded results"); - } + // Make the upload + await uploadPayload(payload); // Mark that we have made an upload fs.writeFileSync(sentinelFile, ''); } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 8c6a31e4e..c9b69dfcb 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -47,6 +47,48 @@ export function combineSarifFiles(sarifFiles: string[]): string { return JSON.stringify(combinedSarif); } +// Upload the given payload. +// If the request fails then this will be retry a small number of times. +async function uploadPayload(payload) { + core.info('Uploading results'); + + const githubToken = core.getInput('token'); + const ph: auth.BearerCredentialHandler = new auth.BearerCredentialHandler(githubToken); + const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); + const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; + + // Make up to 4 attempts to upload, and sleep for these + // number of seconds between each attempt. + // We don't want to backoff too much to avoid wasting action + // minutes, but just waiting a little bit could maybe help. + const backoffPeriods = [1, 5, 15]; + + for (let attempt = 0; attempt <= backoffPeriods.length; attempt++) { + + const res: http.HttpClientResponse = await client.put(url, payload); + core.debug('response status: ' + res.message.statusCode); + + if (res.message.statusCode === 202) { + core.info("Successfully uploaded results"); + return; + } + + const requestID = res.message.headers["x-github-request-id"]; + + if (attempt < backoffPeriods.length) { + // Log the failure as a warning but don't mark the action as failed yet + core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + + await res.readBody()); + // Sleep for the backoff period + await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + + } else { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + } + } +} + // Uploads a single sarif file or a directory of sarif files // depending on what the path happens to refer to. export async function upload(input: string) { @@ -112,25 +154,8 @@ async function uploadFiles(sarifFiles: string[]) { "tool_names": toolNames, }); - core.info('Uploading results'); - const githubToken = core.getInput('token'); - const ph: auth.BearerCredentialHandler = new auth.BearerCredentialHandler(githubToken); - const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); - const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; - const res: http.HttpClientResponse = await client.put(url, payload); - const requestID = res.message.headers["x-github-request-id"]; - - core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. - // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - } else if (res.message.statusCode !== 202) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); - } else { - core.info("Successfully uploaded results"); - } + // Make the upload + await uploadPayload(payload); // Mark that we have made an upload fs.writeFileSync(sentinelFile, ''); From cffc0f7b4e89a6b33851dd8f9033f870784b54ed Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:19:39 +0100 Subject: [PATCH 2/6] fix typo --- lib/upload-lib.js | 2 +- src/upload-lib.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 62d49a065..668f5f8cc 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -55,7 +55,7 @@ function combineSarifFiles(sarifFiles) { } exports.combineSarifFiles = combineSarifFiles; // Upload the given payload. -// If the request fails then this will be retry a small number of times. +// If the request fails then this will retry a small number of times. async function uploadPayload(payload) { core.info('Uploading results'); const githubToken = core.getInput('token'); diff --git a/src/upload-lib.ts b/src/upload-lib.ts index c9b69dfcb..18d43f861 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -48,7 +48,7 @@ export function combineSarifFiles(sarifFiles: string[]): string { } // Upload the given payload. -// If the request fails then this will be retry a small number of times. +// If the request fails then this will retry a small number of times. async function uploadPayload(payload) { core.info('Uploading results'); From e52e34ba17846a8305514ca2459c4102b88a10cf Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:20:21 +0100 Subject: [PATCH 3/6] remove change to behaviour on 500 errors --- lib/upload-lib.js | 6 ++++++ src/upload-lib.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 668f5f8cc..d8f4639d9 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -83,6 +83,12 @@ async function uploadPayload(payload) { // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); } + else if (res.message.statusCode === 500) { + // If the upload fails with 500 then we assume it is a temporary problem + // with turbo-scan and not an error that the user has caused or can fix. + // We avoid marking the job as failed to avoid breaking CI workflows. + core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + } else { core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 18d43f861..1342ccfa8 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -83,6 +83,12 @@ async function uploadPayload(payload) { // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + } else if (res.message.statusCode === 500) { + // If the upload fails with 500 then we assume it is a temporary problem + // with turbo-scan and not an error that the user has caused or can fix. + // We avoid marking the job as failed to avoid breaking CI workflows. + core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + } else { core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); } From 0c4fc16b490c4da59b8fa673963871f640c36ba5 Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:32:09 +0100 Subject: [PATCH 4/6] only retry on 5xx status codes --- lib/upload-lib.js | 18 ++++++++++++------ src/upload-lib.ts | 19 +++++++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index d8f4639d9..a23f54c4b 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -70,11 +70,18 @@ async function uploadPayload(payload) { for (let attempt = 0; attempt <= backoffPeriods.length; attempt++) { const res = await client.put(url, payload); core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 202) { + const statusCode = res.message.statusCode; + if (statusCode === 202) { core.info("Successfully uploaded results"); return; } const requestID = res.message.headers["x-github-request-id"]; + // On any other status code that's not 5xx mark the upload as failed + if (!statusCode || statusCode < 500 || statusCode >= 600) { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; + } + // On a 5xx status code we may retry the request if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + @@ -82,15 +89,14 @@ async function uploadPayload(payload) { await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + continue; } - else if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem + else { + // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - } - else { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; } } } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 1342ccfa8..d404b2685 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -68,13 +68,21 @@ async function uploadPayload(payload) { const res: http.HttpClientResponse = await client.put(url, payload); core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 202) { + const statusCode = res.message.statusCode; + if (statusCode === 202) { core.info("Successfully uploaded results"); return; } const requestID = res.message.headers["x-github-request-id"]; + // On any other status code that's not 5xx mark the upload as failed + if (!statusCode || statusCode < 500 || statusCode >= 600) { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; + } + + // On a 5xx status code we may retry the request if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + @@ -82,15 +90,14 @@ async function uploadPayload(payload) { await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + continue; - } else if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem + } else { + // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - - } else { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; } } } From a23cb1d61aa40e508980e1112d465e92f44d7c76 Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:45:00 +0100 Subject: [PATCH 5/6] include status code is error message --- lib/upload-lib.js | 8 ++++---- src/upload-lib.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index a23f54c4b..e2dd3b32d 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -78,15 +78,15 @@ async function uploadPayload(payload) { const requestID = res.message.headers["x-github-request-id"]; // On any other status code that's not 5xx mark the upload as failed if (!statusCode || statusCode < 500 || statusCode >= 600) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + core.setFailed('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } // On a 5xx status code we may retry the request if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + - ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + - await res.readBody()); + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + + ' seconds: (' + statusCode + ') ' + await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); continue; @@ -95,7 +95,7 @@ async function uploadPayload(payload) { // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index d404b2685..bcb6056dd 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -78,7 +78,7 @@ async function uploadPayload(payload) { // On any other status code that's not 5xx mark the upload as failed if (!statusCode || statusCode < 500 || statusCode >= 600) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + core.setFailed('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } @@ -86,8 +86,8 @@ async function uploadPayload(payload) { if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + - ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + - await res.readBody()); + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + + ' seconds: (' + statusCode + ') ' + await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); continue; @@ -96,7 +96,7 @@ async function uploadPayload(payload) { // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } } From 129ce28897c35736adad3bd570afe34aa4ba9e05 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 1 May 2020 11:59:44 +0100 Subject: [PATCH 6/6] Update upload-lib.ts --- lib/upload-lib.js | 2 +- src/upload-lib.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index e2dd3b32d..631e8fe39 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -93,7 +93,7 @@ async function uploadPayload(payload) { } else { // If the upload fails with 5xx then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. + // and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; diff --git a/src/upload-lib.ts b/src/upload-lib.ts index bcb6056dd..2e8139991 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -94,7 +94,7 @@ async function uploadPayload(payload) { } else { // If the upload fails with 5xx then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. + // and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return;