From 13c548978dbeb9acb50fc752558d02721d771406 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Tue, 10 Mar 2026 12:14:20 +0000 Subject: [PATCH 1/6] Fix retries when uploading databases --- lib/analyze-action-post.js | 6 +-- lib/analyze-action.js | 81 +++++++++++++++++----------- lib/autobuild-action.js | 6 +-- lib/init-action-post.js | 6 +-- lib/init-action.js | 6 +-- lib/resolve-environment-action.js | 6 +-- lib/setup-codeql-action.js | 6 +-- lib/start-proxy-action-post.js | 6 +-- lib/start-proxy-action.js | 6 +-- lib/upload-lib.js | 6 +-- lib/upload-sarif-action-post.js | 6 +-- lib/upload-sarif-action.js | 6 +-- src/api-client.test.ts | 3 +- src/api-client.ts | 14 +++-- src/database-upload.ts | 89 ++++++++++++++++++++----------- 15 files changed, 141 insertions(+), 112 deletions(-) diff --git a/lib/analyze-action-post.js b/lib/analyze-action-post.js index 454c2d9fb4..fb54b6631f 100644 --- a/lib/analyze-action-post.js +++ b/lib/analyze-action-post.js @@ -161404,6 +161404,7 @@ retry.VERSION = VERSION7; // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -161418,10 +161419,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/analyze-action.js b/lib/analyze-action.js index a65d7175cd..cb0bf128ff 100644 --- a/lib/analyze-action.js +++ b/lib/analyze-action.js @@ -106782,6 +106782,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -106796,10 +106797,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); @@ -110969,40 +110967,59 @@ async function cleanupAndUploadDatabases(repositoryNwo, codeql, config, apiDetai includeDiagnostics: false }); bundledDbSize = fs13.statSync(bundledDb).size; - const bundledDbReadStream = fs13.createReadStream(bundledDb); const commitOid = await getCommitOid( getRequiredInput("checkout_path") ); - try { - const startTime = performance.now(); - await client.request( - `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, - { - baseUrl: uploadsBaseUrl, - owner: repositoryNwo.owner, - repo: repositoryNwo.repo, - language, - name: `${language}-database`, - commit_oid: commitOid, - data: bundledDbReadStream, - headers: { - authorization: `token ${apiDetails.auth}`, - "Content-Type": "application/zip", - "Content-Length": bundledDbSize + const maxAttempts = 4; + let uploadDurationMs; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + const bundledDbReadStream = fs13.createReadStream(bundledDb); + try { + const attemptStartTime = performance.now(); + await client.request( + `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, + { + baseUrl: uploadsBaseUrl, + owner: repositoryNwo.owner, + repo: repositoryNwo.repo, + language, + name: `${language}-database`, + commit_oid: commitOid, + data: bundledDbReadStream, + headers: { + authorization: `token ${apiDetails.auth}`, + "Content-Type": "application/zip", + "Content-Length": bundledDbSize + }, + request: { + retries: 0 + } } + ); + uploadDurationMs = performance.now() - attemptStartTime; + break; + } catch (e) { + const httpError = asHTTPError(e); + const isRetryable = !httpError || !DO_NOT_RETRY_STATUSES.includes(httpError.status); + if (!isRetryable || attempt === maxAttempts) { + throw e; } - ); - const endTime = performance.now(); - reports.push({ - language, - zipped_upload_size_bytes: bundledDbSize, - is_overlay_base: shouldUploadOverlayBase, - upload_duration_ms: endTime - startTime - }); - logger.debug(`Successfully uploaded database for ${language}`); - } finally { - bundledDbReadStream.close(); + const backoffMs = 15e3 * Math.pow(2, attempt - 1); + logger.debug( + `Database upload attempt ${attempt} of ${maxAttempts} failed for ${language}: ${getErrorMessage(e)}. Retrying in ${backoffMs / 1e3}s...` + ); + await new Promise((resolve8) => setTimeout(resolve8, backoffMs)); + } finally { + bundledDbReadStream.close(); + } } + reports.push({ + language, + zipped_upload_size_bytes: bundledDbSize, + is_overlay_base: shouldUploadOverlayBase, + upload_duration_ms: uploadDurationMs + }); + logger.debug(`Successfully uploaded database for ${language}`); } catch (e) { logger.warning( `Failed to upload database for ${language}: ${getErrorMessage(e)}` diff --git a/lib/autobuild-action.js b/lib/autobuild-action.js index acd1b250e7..234da35b3f 100644 --- a/lib/autobuild-action.js +++ b/lib/autobuild-action.js @@ -103423,6 +103423,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -103437,10 +103438,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/init-action-post.js b/lib/init-action-post.js index 479129b248..67e8f2689e 100644 --- a/lib/init-action-post.js +++ b/lib/init-action-post.js @@ -164624,6 +164624,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -164638,10 +164639,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/init-action.js b/lib/init-action.js index 5d5a6fa598..e305ac6f12 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -104131,6 +104131,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -104145,10 +104146,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/resolve-environment-action.js b/lib/resolve-environment-action.js index aa3673bd33..11fa8c0c0a 100644 --- a/lib/resolve-environment-action.js +++ b/lib/resolve-environment-action.js @@ -103431,6 +103431,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -103445,10 +103446,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/setup-codeql-action.js b/lib/setup-codeql-action.js index a9eb08eb59..e35c84838c 100644 --- a/lib/setup-codeql-action.js +++ b/lib/setup-codeql-action.js @@ -103540,6 +103540,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -103554,10 +103555,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/start-proxy-action-post.js b/lib/start-proxy-action-post.js index 6fdfe2d8be..610a35a72d 100644 --- a/lib/start-proxy-action-post.js +++ b/lib/start-proxy-action-post.js @@ -161287,6 +161287,7 @@ retry.VERSION = VERSION7; // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -161301,10 +161302,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/start-proxy-action.js b/lib/start-proxy-action.js index 84519a068d..0b3ed7b0c1 100644 --- a/lib/start-proxy-action.js +++ b/lib/start-proxy-action.js @@ -120510,6 +120510,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -120524,10 +120525,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 2f5585bd19..38765bcc6e 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -106416,6 +106416,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -106430,10 +106431,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/upload-sarif-action-post.js b/lib/upload-sarif-action-post.js index dab78eb861..f6a2421c2f 100644 --- a/lib/upload-sarif-action-post.js +++ b/lib/upload-sarif-action-post.js @@ -161287,6 +161287,7 @@ retry.VERSION = VERSION7; // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -161301,10 +161302,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/lib/upload-sarif-action.js b/lib/upload-sarif-action.js index 53f3856536..fd3672bde6 100644 --- a/lib/upload-sarif-action.js +++ b/lib/upload-sarif-action.js @@ -106465,6 +106465,7 @@ function parseRepositoryNwo(input) { // src/api-client.ts var GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +var DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth || apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -106479,10 +106480,7 @@ function createApiClientWithDetails(apiDetails, { allowExternal = false } = {}) error: core5.error }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451] + doNotRetry: DO_NOT_RETRY_STATUSES } }) ); diff --git a/src/api-client.test.ts b/src/api-client.test.ts index d0311d0dc5..f8846e7682 100644 --- a/src/api-client.test.ts +++ b/src/api-client.test.ts @@ -5,6 +5,7 @@ import * as sinon from "sinon"; import * as actionsUtil from "./actions-util"; import * as api from "./api-client"; +import { DO_NOT_RETRY_STATUSES } from "./api-client"; import { setupTests } from "./testing-utils"; import * as util from "./util"; @@ -37,7 +38,7 @@ test.serial("getApiClient", async (t) => { log: sinon.match.any, userAgent: `CodeQL-Action/${actionsUtil.getActionVersion()}`, retry: { - doNotRetry: [400, 410, 422, 451], + doNotRetry: DO_NOT_RETRY_STATUSES, }, }), ); diff --git a/src/api-client.ts b/src/api-client.ts index 13babcd385..4b8cb7b340 100644 --- a/src/api-client.ts +++ b/src/api-client.ts @@ -19,6 +19,15 @@ import { const GITHUB_ENTERPRISE_VERSION_HEADER = "x-github-enterprise-version"; +/** + * HTTP status codes that should not be retried. + * + * The default Octokit list is 400, 401, 403, 404, 410, 422, and 451. We have + * observed transient errors with authentication, so we remove 401, 403, and 404 + * from the default list to ensure that these errors are retried. + */ +export const DO_NOT_RETRY_STATUSES = [400, 410, 422, 451]; + export type GitHubApiCombinedDetails = GitHubApiDetails & GitHubApiExternalRepoDetails; @@ -52,10 +61,7 @@ function createApiClientWithDetails( error: core.error, }, retry: { - // The default is 400, 401, 403, 404, 410, 422, and 451. We have observed transient errors - // with authentication, so we remove 401, 403, and 404 from the default list to ensure that - // these errors are retried. - doNotRetry: [400, 410, 422, 451], + doNotRetry: DO_NOT_RETRY_STATUSES, }, }), ); diff --git a/src/database-upload.ts b/src/database-upload.ts index 41546697f6..b9e2f5b06d 100644 --- a/src/database-upload.ts +++ b/src/database-upload.ts @@ -2,7 +2,11 @@ import * as fs from "fs"; import * as actionsUtil from "./actions-util"; import { AnalysisKind } from "./analyses"; -import { getApiClient, GitHubApiDetails } from "./api-client"; +import { + DO_NOT_RETRY_STATUSES, + getApiClient, + GitHubApiDetails, +} from "./api-client"; import { type CodeQL } from "./codeql"; import { Config } from "./config-utils"; import { Feature, FeatureEnablement } from "./feature-flags"; @@ -11,7 +15,7 @@ import { Logger, withGroupAsync } from "./logging"; import { OverlayDatabaseMode } from "./overlay"; import { RepositoryNwo } from "./repository"; import * as util from "./util"; -import { bundleDb, CleanupLevel, parseGitHubUrl } from "./util"; +import { asHTTPError, bundleDb, CleanupLevel, parseGitHubUrl } from "./util"; /** Information about a database upload. */ export interface DatabaseUploadResult { @@ -105,40 +109,63 @@ export async function cleanupAndUploadDatabases( includeDiagnostics: false, }); bundledDbSize = fs.statSync(bundledDb).size; - const bundledDbReadStream = fs.createReadStream(bundledDb); const commitOid = await gitUtils.getCommitOid( actionsUtil.getRequiredInput("checkout_path"), ); - try { - const startTime = performance.now(); - await client.request( - `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, - { - baseUrl: uploadsBaseUrl, - owner: repositoryNwo.owner, - repo: repositoryNwo.repo, - language, - name: `${language}-database`, - commit_oid: commitOid, - data: bundledDbReadStream, - headers: { - authorization: `token ${apiDetails.auth}`, - "Content-Type": "application/zip", - "Content-Length": bundledDbSize, + // Upload with manual retry logic. We disable Octokit's built-in retries + // because the request body is a ReadStream, which can only be consumed + // once. + const maxAttempts = 4; // 1 initial attempt + 3 retries, identical to the default retry behavior of Octokit + let uploadDurationMs: number | undefined; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + const bundledDbReadStream = fs.createReadStream(bundledDb); + try { + const attemptStartTime = performance.now(); + await client.request( + `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, + { + baseUrl: uploadsBaseUrl, + owner: repositoryNwo.owner, + repo: repositoryNwo.repo, + language, + name: `${language}-database`, + commit_oid: commitOid, + data: bundledDbReadStream, + headers: { + authorization: `token ${apiDetails.auth}`, + "Content-Type": "application/zip", + "Content-Length": bundledDbSize, + }, + request: { + retries: 0, + }, }, - }, - ); - const endTime = performance.now(); - reports.push({ - language, - zipped_upload_size_bytes: bundledDbSize, - is_overlay_base: shouldUploadOverlayBase, - upload_duration_ms: endTime - startTime, - }); - logger.debug(`Successfully uploaded database for ${language}`); - } finally { - bundledDbReadStream.close(); + ); + uploadDurationMs = performance.now() - attemptStartTime; + break; + } catch (e) { + const httpError = asHTTPError(e); + const isRetryable = + !httpError || !DO_NOT_RETRY_STATUSES.includes(httpError.status); + if (!isRetryable || attempt === maxAttempts) { + throw e; + } + const backoffMs = 15_000 * Math.pow(2, attempt - 1); // 15s, 30s, 60s + logger.debug( + `Database upload attempt ${attempt} of ${maxAttempts} failed for ${language}: ${util.getErrorMessage(e)}. Retrying in ${backoffMs / 1000}s...`, + ); + await new Promise((resolve) => setTimeout(resolve, backoffMs)); + } finally { + bundledDbReadStream.close(); + } } + reports.push({ + language, + zipped_upload_size_bytes: bundledDbSize, + is_overlay_base: shouldUploadOverlayBase, + upload_duration_ms: uploadDurationMs, + }); + logger.debug(`Successfully uploaded database for ${language}`); } catch (e) { // Log a warning but don't fail the workflow logger.warning( From ca969a91db22e598a0776e071087dc700e42b199 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Tue, 10 Mar 2026 12:34:47 +0000 Subject: [PATCH 2/6] Add changelog note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d6c8a15e5..f7491eb62f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th - Fixed [a bug](https://github.com/github/codeql-action/issues/3555) which caused the CodeQL Action to fail loading repository properties if a "Multi select" repository property was configured for the repository. [#3557](https://github.com/github/codeql-action/pull/3557) - The CodeQL Action now loads [custom repository properties](https://docs.github.com/en/organizations/managing-organization-settings/managing-custom-properties-for-repositories-in-your-organization) on GitHub Enterprise Server, enabling the customization of features such as `github-codeql-disable-overlay` that was previously only available on GitHub.com. [#3559](https://github.com/github/codeql-action/pull/3559) +- Fixed the retry mechanism for database uploads. Previously this would fail with the error "Response body object should not be disturbed or locked". [#3564](https://github.com/github/codeql-action/pull/3564) ## 4.32.6 - 05 Mar 2026 From edfcb0a509722876e11d1d78b8c49259926b4096 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Tue, 10 Mar 2026 12:49:58 +0000 Subject: [PATCH 3/6] Update tests --- src/database-upload.test.ts | 100 ++++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 28 deletions(-) diff --git a/src/database-upload.test.ts b/src/database-upload.test.ts index 3d8433d8b5..8bd22091c7 100644 --- a/src/database-upload.test.ts +++ b/src/database-upload.test.ts @@ -214,37 +214,81 @@ test.serial( }, ); -test.serial("Don't crash if uploading a database fails", async (t) => { - await withTmpDir(async (tmpDir) => { - setupActionsVars(tmpDir, tmpDir); - sinon - .stub(actionsUtil, "getRequiredInput") - .withArgs("upload-database") - .returns("true"); - sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true); +test.serial( + "Don't crash if uploading a database fails with a non-retryable error", + async (t) => { + await withTmpDir(async (tmpDir) => { + setupActionsVars(tmpDir, tmpDir); + sinon + .stub(actionsUtil, "getRequiredInput") + .withArgs("upload-database") + .returns("true"); + sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true); - await mockHttpRequests(500); + await mockHttpRequests(422); - const loggedMessages = [] as LoggedMessage[]; - await cleanupAndUploadDatabases( - testRepoName, - getCodeQL(), - getTestConfig(tmpDir), - testApiDetails, - createFeatures([]), - getRecordingLogger(loggedMessages), - ); + const loggedMessages = [] as LoggedMessage[]; + await cleanupAndUploadDatabases( + testRepoName, + getCodeQL(), + getTestConfig(tmpDir), + testApiDetails, + createFeatures([]), + getRecordingLogger(loggedMessages), + ); - t.assert( - loggedMessages.find( - (v) => - v.type === "warning" && - v.message === - "Failed to upload database for javascript: some error message", - ) !== undefined, - ); - }); -}); + t.assert( + loggedMessages.find( + (v) => + v.type === "warning" && + v.message === + "Failed to upload database for javascript: some error message", + ) !== undefined, + ); + }); + }, +); + +test.serial( + "Don't crash if uploading a database fails with a retryable error", + async (t) => { + await withTmpDir(async (tmpDir) => { + setupActionsVars(tmpDir, tmpDir); + sinon + .stub(actionsUtil, "getRequiredInput") + .withArgs("upload-database") + .returns("true"); + sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true); + + await mockHttpRequests(500); + + // Stub setTimeout to fire immediately to avoid real delays from retry backoff. + const originalSetTimeout = global.setTimeout; + sinon + .stub(global, "setTimeout") + .callsFake((fn: () => void) => originalSetTimeout(fn, 0)); + + const loggedMessages = [] as LoggedMessage[]; + await cleanupAndUploadDatabases( + testRepoName, + getCodeQL(), + getTestConfig(tmpDir), + testApiDetails, + createFeatures([]), + getRecordingLogger(loggedMessages), + ); + + t.assert( + loggedMessages.find( + (v) => + v.type === "warning" && + v.message === + "Failed to upload database for javascript: some error message", + ) !== undefined, + ); + }); + }, +); test.serial("Successfully uploading a database to github.com", async (t) => { await withTmpDir(async (tmpDir) => { From ee5ede79f765f2577bb55472e38588e585182710 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Tue, 10 Mar 2026 15:51:28 +0000 Subject: [PATCH 4/6] Address review comments --- lib/analyze-action.js | 7 ++++++- src/database-upload.test.ts | 18 +++++++++++++++--- src/database-upload.ts | 7 ++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/analyze-action.js b/lib/analyze-action.js index cb0bf128ff..da459df0f0 100644 --- a/lib/analyze-action.js +++ b/lib/analyze-action.js @@ -111001,7 +111001,12 @@ async function cleanupAndUploadDatabases(repositoryNwo, codeql, config, apiDetai } catch (e) { const httpError = asHTTPError(e); const isRetryable = !httpError || !DO_NOT_RETRY_STATUSES.includes(httpError.status); - if (!isRetryable || attempt === maxAttempts) { + if (!isRetryable) { + throw e; + } else if (attempt === maxAttempts) { + logger.error( + `Maximum retry attempts exhausted (${attempt}), aborting database upload` + ); throw e; } const backoffMs = 15e3 * Math.pow(2, attempt - 1); diff --git a/src/database-upload.test.ts b/src/database-upload.test.ts index 8bd22091c7..ecd0eb8c6b 100644 --- a/src/database-upload.test.ts +++ b/src/database-upload.test.ts @@ -225,7 +225,7 @@ test.serial( .returns("true"); sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true); - await mockHttpRequests(422); + const databaseUploadSpy = await mockHttpRequests(422); const loggedMessages = [] as LoggedMessage[]; await cleanupAndUploadDatabases( @@ -245,6 +245,9 @@ test.serial( "Failed to upload database for javascript: some error message", ) !== undefined, ); + + // Non-retryable errors should not be retried. + t.is(databaseUploadSpy.callCount, 1); }); }, ); @@ -260,11 +263,11 @@ test.serial( .returns("true"); sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true); - await mockHttpRequests(500); + const databaseUploadSpy = await mockHttpRequests(500); // Stub setTimeout to fire immediately to avoid real delays from retry backoff. const originalSetTimeout = global.setTimeout; - sinon + const setTimeoutStub = sinon .stub(global, "setTimeout") .callsFake((fn: () => void) => originalSetTimeout(fn, 0)); @@ -286,6 +289,15 @@ test.serial( "Failed to upload database for javascript: some error message", ) !== undefined, ); + + // Retryable errors should be retried the expected number of times. + t.is(databaseUploadSpy.callCount, 4); + + // setTimeout should have been called with the expected backoff delays. + const setTimeoutDelays = setTimeoutStub.args.map( + (args) => args[1] as number, + ); + t.deepEqual(setTimeoutDelays, [15_000, 30_000, 60_000]); }); }, ); diff --git a/src/database-upload.ts b/src/database-upload.ts index b9e2f5b06d..8d7d49551c 100644 --- a/src/database-upload.ts +++ b/src/database-upload.ts @@ -147,7 +147,12 @@ export async function cleanupAndUploadDatabases( const httpError = asHTTPError(e); const isRetryable = !httpError || !DO_NOT_RETRY_STATUSES.includes(httpError.status); - if (!isRetryable || attempt === maxAttempts) { + if (!isRetryable) { + throw e; + } else if (attempt === maxAttempts) { + logger.error( + `Maximum retry attempts exhausted (${attempt}), aborting database upload`, + ); throw e; } const backoffMs = 15_000 * Math.pow(2, attempt - 1); // 15s, 30s, 60s From cf972cde0e8c119c3d1898bf01c1dee40c129932 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Tue, 10 Mar 2026 15:52:14 +0000 Subject: [PATCH 5/6] Update database upload tests to use `checkExpectedLogMessages` --- src/database-upload.test.ts | 102 ++++++++++++------------------------ 1 file changed, 33 insertions(+), 69 deletions(-) diff --git a/src/database-upload.test.ts b/src/database-upload.test.ts index ecd0eb8c6b..c4ac59e76b 100644 --- a/src/database-upload.test.ts +++ b/src/database-upload.test.ts @@ -15,6 +15,7 @@ import * as gitUtils from "./git-utils"; import { KnownLanguage } from "./languages"; import { RepositoryNwo } from "./repository"; import { + checkExpectedLogMessages, createFeatures, createTestConfig, getRecordingLogger, @@ -93,7 +94,7 @@ test.serial( .returns("false"); sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true); - const loggedMessages = []; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -102,14 +103,9 @@ test.serial( createFeatures([]), getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v: LoggedMessage) => - v.type === "debug" && - v.message === - "Database upload disabled in workflow. Skipping upload.", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Database upload disabled in workflow. Skipping upload.", + ]); }); }, ); @@ -127,7 +123,7 @@ test.serial( await mockHttpRequests(201); - const loggedMessages = []; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -139,14 +135,9 @@ test.serial( createFeatures([]), getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v: LoggedMessage) => - v.type === "debug" && - v.message === - "Not uploading database because 'analysis-kinds: code-scanning' is not enabled.", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Not uploading database because 'analysis-kinds: code-scanning' is not enabled.", + ]); }); }, ); @@ -163,7 +154,7 @@ test.serial("Abort database upload if running against GHES", async (t) => { const config = getTestConfig(tmpDir); config.gitHubVersion = { type: GitHubVariant.GHES, version: "3.0" }; - const loggedMessages = []; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -172,14 +163,9 @@ test.serial("Abort database upload if running against GHES", async (t) => { createFeatures([]), getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v: LoggedMessage) => - v.type === "debug" && - v.message === - "Not running against github.com or GHEC-DR. Skipping upload.", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Not running against github.com or GHEC-DR. Skipping upload.", + ]); }); }); @@ -194,7 +180,7 @@ test.serial( .returns("true"); sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(false); - const loggedMessages = []; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -203,13 +189,9 @@ test.serial( createFeatures([]), getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v: LoggedMessage) => - v.type === "debug" && - v.message === "Not analyzing default branch. Skipping upload.", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Not analyzing default branch. Skipping upload.", + ]); }); }, ); @@ -227,7 +209,7 @@ test.serial( const databaseUploadSpy = await mockHttpRequests(422); - const loggedMessages = [] as LoggedMessage[]; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -237,14 +219,9 @@ test.serial( getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v) => - v.type === "warning" && - v.message === - "Failed to upload database for javascript: some error message", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Failed to upload database for javascript: some error message", + ]); // Non-retryable errors should not be retried. t.is(databaseUploadSpy.callCount, 1); @@ -271,7 +248,7 @@ test.serial( .stub(global, "setTimeout") .callsFake((fn: () => void) => originalSetTimeout(fn, 0)); - const loggedMessages = [] as LoggedMessage[]; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -281,14 +258,9 @@ test.serial( getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v) => - v.type === "warning" && - v.message === - "Failed to upload database for javascript: some error message", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Failed to upload database for javascript: some error message", + ]); // Retryable errors should be retried the expected number of times. t.is(databaseUploadSpy.callCount, 4); @@ -313,7 +285,7 @@ test.serial("Successfully uploading a database to github.com", async (t) => { await mockHttpRequests(201); - const loggedMessages = [] as LoggedMessage[]; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -322,13 +294,9 @@ test.serial("Successfully uploading a database to github.com", async (t) => { createFeatures([]), getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v) => - v.type === "debug" && - v.message === "Successfully uploaded database for javascript", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Successfully uploaded database for javascript", + ]); }); }); @@ -343,7 +311,7 @@ test.serial("Successfully uploading a database to GHEC-DR", async (t) => { const databaseUploadSpy = await mockHttpRequests(201); - const loggedMessages = [] as LoggedMessage[]; + const loggedMessages: LoggedMessage[] = []; await cleanupAndUploadDatabases( testRepoName, getCodeQL(), @@ -356,13 +324,9 @@ test.serial("Successfully uploading a database to GHEC-DR", async (t) => { createFeatures([]), getRecordingLogger(loggedMessages), ); - t.assert( - loggedMessages.find( - (v) => - v.type === "debug" && - v.message === "Successfully uploaded database for javascript", - ) !== undefined, - ); + checkExpectedLogMessages(t, loggedMessages, [ + "Successfully uploaded database for javascript", + ]); t.assert( databaseUploadSpy.calledOnceWith( sinon.match.string, From a63886bff563ff37363fa5140c38d3d121b5e1a6 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Tue, 10 Mar 2026 16:36:02 +0000 Subject: [PATCH 6/6] Refactor: Extract separate function for `uploadBundledDatabase` --- lib/analyze-action.js | 75 +++++++++++++++++++------------- src/database-upload.ts | 98 ++++++++++++++++++++++++++---------------- 2 files changed, 106 insertions(+), 67 deletions(-) diff --git a/lib/analyze-action.js b/lib/analyze-action.js index da459df0f0..c84eec3a52 100644 --- a/lib/analyze-action.js +++ b/lib/analyze-action.js @@ -110952,13 +110952,6 @@ async function cleanupAndUploadDatabases(repositoryNwo, codeql, config, apiDetai await withGroupAsync("Cleaning up databases", async () => { await codeql.databaseCleanupCluster(config, cleanupLevel); }); - const client = getApiClient(); - const uploadsUrl = new URL(parseGitHubUrl(apiDetails.url)); - uploadsUrl.hostname = `uploads.${uploadsUrl.hostname}`; - let uploadsBaseUrl = uploadsUrl.toString(); - if (uploadsBaseUrl.endsWith("/")) { - uploadsBaseUrl = uploadsBaseUrl.slice(0, -1); - } const reports = []; for (const language of config.languages) { let bundledDbSize = void 0; @@ -110973,30 +110966,15 @@ async function cleanupAndUploadDatabases(repositoryNwo, codeql, config, apiDetai const maxAttempts = 4; let uploadDurationMs; for (let attempt = 1; attempt <= maxAttempts; attempt++) { - const bundledDbReadStream = fs13.createReadStream(bundledDb); try { - const attemptStartTime = performance.now(); - await client.request( - `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, - { - baseUrl: uploadsBaseUrl, - owner: repositoryNwo.owner, - repo: repositoryNwo.repo, - language, - name: `${language}-database`, - commit_oid: commitOid, - data: bundledDbReadStream, - headers: { - authorization: `token ${apiDetails.auth}`, - "Content-Type": "application/zip", - "Content-Length": bundledDbSize - }, - request: { - retries: 0 - } - } + uploadDurationMs = await uploadBundledDatabase( + repositoryNwo, + language, + commitOid, + bundledDb, + bundledDbSize, + apiDetails ); - uploadDurationMs = performance.now() - attemptStartTime; break; } catch (e) { const httpError = asHTTPError(e); @@ -111014,8 +110992,6 @@ async function cleanupAndUploadDatabases(repositoryNwo, codeql, config, apiDetai `Database upload attempt ${attempt} of ${maxAttempts} failed for ${language}: ${getErrorMessage(e)}. Retrying in ${backoffMs / 1e3}s...` ); await new Promise((resolve8) => setTimeout(resolve8, backoffMs)); - } finally { - bundledDbReadStream.close(); } } reports.push({ @@ -111038,6 +111014,43 @@ async function cleanupAndUploadDatabases(repositoryNwo, codeql, config, apiDetai } return reports; } +async function uploadBundledDatabase(repositoryNwo, language, commitOid, bundledDb, bundledDbSize, apiDetails) { + const client = getApiClient(); + const uploadsUrl = new URL(parseGitHubUrl(apiDetails.url)); + uploadsUrl.hostname = `uploads.${uploadsUrl.hostname}`; + let uploadsBaseUrl = uploadsUrl.toString(); + if (uploadsBaseUrl.endsWith("/")) { + uploadsBaseUrl = uploadsBaseUrl.slice(0, -1); + } + const bundledDbReadStream = fs13.createReadStream(bundledDb); + try { + const startTime = performance.now(); + await client.request( + `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, + { + baseUrl: uploadsBaseUrl, + owner: repositoryNwo.owner, + repo: repositoryNwo.repo, + language, + name: `${language}-database`, + commit_oid: commitOid, + data: bundledDbReadStream, + headers: { + authorization: `token ${apiDetails.auth}`, + "Content-Type": "application/zip", + "Content-Length": bundledDbSize + }, + // Disable `octokit/plugin-retry.js`, since the request body is a ReadStream which can only be consumed once. + request: { + retries: 0 + } + } + ); + return performance.now() - startTime; + } finally { + bundledDbReadStream.close(); + } +} // src/status-report.ts var os4 = __toESM(require("os")); diff --git a/src/database-upload.ts b/src/database-upload.ts index 8d7d49551c..c7db68fc3d 100644 --- a/src/database-upload.ts +++ b/src/database-upload.ts @@ -85,18 +85,6 @@ export async function cleanupAndUploadDatabases( await codeql.databaseCleanupCluster(config, cleanupLevel); }); - const client = getApiClient(); - - const uploadsUrl = new URL(parseGitHubUrl(apiDetails.url)); - uploadsUrl.hostname = `uploads.${uploadsUrl.hostname}`; - - // Octokit expects the baseUrl to not have a trailing slash, - // but it is included by default in a URL. - let uploadsBaseUrl = uploadsUrl.toString(); - if (uploadsBaseUrl.endsWith("/")) { - uploadsBaseUrl = uploadsBaseUrl.slice(0, -1); - } - const reports: DatabaseUploadResult[] = []; for (const language of config.languages) { let bundledDbSize: number | undefined = undefined; @@ -118,30 +106,15 @@ export async function cleanupAndUploadDatabases( const maxAttempts = 4; // 1 initial attempt + 3 retries, identical to the default retry behavior of Octokit let uploadDurationMs: number | undefined; for (let attempt = 1; attempt <= maxAttempts; attempt++) { - const bundledDbReadStream = fs.createReadStream(bundledDb); try { - const attemptStartTime = performance.now(); - await client.request( - `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, - { - baseUrl: uploadsBaseUrl, - owner: repositoryNwo.owner, - repo: repositoryNwo.repo, - language, - name: `${language}-database`, - commit_oid: commitOid, - data: bundledDbReadStream, - headers: { - authorization: `token ${apiDetails.auth}`, - "Content-Type": "application/zip", - "Content-Length": bundledDbSize, - }, - request: { - retries: 0, - }, - }, + uploadDurationMs = await uploadBundledDatabase( + repositoryNwo, + language, + commitOid, + bundledDb, + bundledDbSize, + apiDetails, ); - uploadDurationMs = performance.now() - attemptStartTime; break; } catch (e) { const httpError = asHTTPError(e); @@ -160,8 +133,6 @@ export async function cleanupAndUploadDatabases( `Database upload attempt ${attempt} of ${maxAttempts} failed for ${language}: ${util.getErrorMessage(e)}. Retrying in ${backoffMs / 1000}s...`, ); await new Promise((resolve) => setTimeout(resolve, backoffMs)); - } finally { - bundledDbReadStream.close(); } } reports.push({ @@ -187,3 +158,58 @@ export async function cleanupAndUploadDatabases( } return reports; } + +/** + * Uploads a bundled database to the GitHub API. + * + * @returns the duration of the upload in milliseconds + */ +async function uploadBundledDatabase( + repositoryNwo: RepositoryNwo, + language: string, + commitOid: string, + bundledDb: string, + bundledDbSize: number, + apiDetails: GitHubApiDetails, +): Promise { + const client = getApiClient(); + + const uploadsUrl = new URL(parseGitHubUrl(apiDetails.url)); + uploadsUrl.hostname = `uploads.${uploadsUrl.hostname}`; + + // Octokit expects the baseUrl to not have a trailing slash, + // but it is included by default in a URL. + let uploadsBaseUrl = uploadsUrl.toString(); + if (uploadsBaseUrl.endsWith("/")) { + uploadsBaseUrl = uploadsBaseUrl.slice(0, -1); + } + + const bundledDbReadStream = fs.createReadStream(bundledDb); + try { + const startTime = performance.now(); + await client.request( + `POST /repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name&commit_oid=:commit_oid`, + { + baseUrl: uploadsBaseUrl, + owner: repositoryNwo.owner, + repo: repositoryNwo.repo, + language, + name: `${language}-database`, + commit_oid: commitOid, + data: bundledDbReadStream, + headers: { + authorization: `token ${apiDetails.auth}`, + "Content-Type": "application/zip", + "Content-Length": bundledDbSize, + }, + // Disable `octokit/plugin-retry.js`, since the request body is a ReadStream which can only be consumed once. + request: { + retries: 0, + }, + }, + ); + return performance.now() - startTime; + } finally { + bundledDbReadStream.close(); + } +}