Wrap API configuration errors when setting up CodeQL by henrymercer · Pull Request #3217 · github/codeql-action · GitHub

Update generic API configuration error handling to handle HTTP errors with the httpStatusCode property instead of the statusCode property, and update the CodeQL setup logic to use this error handling.

Risk assessment

For internal use only. Please select the risk level of this change:

Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Advanced setup - Impacts users who have custom workflows.
Default setup - Impacts users who use default setup.
Code Scanning - Impacts Code Scanning (i.e. analysis-kinds: code-scanning).
Code Quality - Impacts Code Quality (i.e. analysis-kinds: code-quality).
Third-party analyses - Impacts third-party analyses (i.e. upload-sarif).
GHES - Impacts GitHub Enterprise Server.

How did/will you validate this change?

Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).
End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

Rollback - Change can only be disabled by rolling back the release or releasing a new version with a fix.

How will you know if something goes wrong after this change is released?

Telemetry - I rely on existing telemetry or have made changes to the telemetry.
- Alerts - New or existing monitors will trip if something goes wrong with this change.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

Pull Request Overview

This PR enhances error handling for API configuration errors by updating the HTTP error detection mechanism and extending error handling coverage to the CodeQL setup logic. The main improvement is replacing the simple isHTTPError type check with a more robust asHTTPError function that can handle different HTTP error formats.

Key Changes

Replaced isHTTPError boolean function with asHTTPError converter function that normalizes HTTP errors
Extended API configuration error wrapping to handle rate limiting (429 status) as configuration errors
Updated CodeQL setup logic to use the improved error handling wrapper

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/util.ts	Replaced isHTTPError with asHTTPError function to handle both status and httpStatusCode properties
src/api-client.ts	Enhanced wrapApiConfigurationError to handle 429 rate limit errors and use new HTTP error detection
src/codeql.ts	Updated CodeQL setup to use API configuration error wrapping and removed specific RequestError handling
src/upload-lib.ts	Updated to use new asHTTPError function for consistent error handling
src/trap-caching.ts	Updated import and usage to use new HTTP error detection method
src/status-report.ts	Updated import and usage to use new HTTP error detection method
src/feature-flags.ts	Updated to use new HTTP error detection method
lib/*.js	Generated JavaScript files reflecting the TypeScript changes

[nitpick] The error message check has been made more generic by removing 'for installation', but this could potentially match unintended error messages. Consider using a more specific pattern or combining with status code checks for better accuracy.

Suggested change

      
                  httpError.message.includes("API rate limit exceeded") ||
          
                  httpError.message.includes("commit not found") ||
          
                  httpError.message.includes("Resource not accessible by integration") ||
          
                  /ref .* not found in this repository/.test(httpError.message)
          
                  ((httpError.message.includes("API rate limit exceeded") &&
          
                    (httpError.status === 403 || httpError.status === 429)) ||
          
                    httpError.message.includes("commit not found") ||
          
                    httpError.message.includes("Resource not accessible by integration") ||
          
                    /ref .* not found in this repository/.test(httpError.message))

[nitpick] The removal of the RequestError status 429 check from the ErrorClass condition logic appears intentional since this is now handled by wrapApiConfigurationError, but the comment should be updated to reflect that rate limiting is now handled upstream.

Suggested change

      
                  e instanceof util.ConfigurationError ||
          
                  e instanceof util.ConfigurationError ||
          
                  // Rate limiting (HTTP 429) is now handled upstream by wrapApiConfigurationError.

LGTM. Just a couple of small suggestions I thought of while reviewing.

Thanks for implementing those suggestions, and even making the checks in asHTTPError more thorough than suggested!

henrymercer added 2 commits October 20, 2025 14:38

Handle HTTP errors with httpStatusCode property

c64c407

Wrap API configuration errors when setting up CodeQL

a6b9514

henrymercer requested a review from a team as a code owner October 20, 2025 14:03

Copilot AI review requested due to automatic review settings October 20, 2025 14:03

Copilot AI reviewed Oct 20, 2025

View reviewed changes

mbg previously approved these changes Oct 20, 2025

View reviewed changes

Comment thread src/api-client.ts Outdated

Comment thread src/util.ts

Require message field too

40e2646

henrymercer dismissed mbg’s stale review via a0c5447 October 21, 2025 09:31

Simplify API error checks

e6e649a

henrymercer force-pushed the henrymercer/http-error-handling branch from a0c5447 to e6e649a Compare October 21, 2025 09:32

Merge branch 'main' into henrymercer/http-error-handling

804fc66

github-actions Bot added the size/S Should be easy to review label Oct 21, 2025

henrymercer requested a review from mbg October 21, 2025 10:31

mbg approved these changes Oct 21, 2025

View reviewed changes

henrymercer merged commit 8e53c48 into main Oct 21, 2025
243 checks passed

henrymercer deleted the henrymercer/http-error-handling branch October 21, 2025 11:15

This was referenced Oct 24, 2025

Merge main into releases/v4 #3235

Merged

Merge releases/v4 into releases/v3 #3237

Merged

+                } catch (rawError) {
+                  const e = api.wrapApiConfigurationError(rawError);
                   const ErrorClass =
                     e instanceof util.ConfigurationError ||

Wrap API configuration errors when setting up CodeQL by henrymercer · Pull Request #3217 · github/codeql-action · GitHub

Uh oh!

Wrap API configuration errors when setting up CodeQL - #3217

Wrap API configuration errors when setting up CodeQL#3217
henrymercer merged 5 commits into
mainfrom
henrymercer/http-error-handling

henrymercer commented Oct 20, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 20, 2025

Uh oh!

Copilot AI Oct 20, 2025

Uh oh!

mbg left a comment

Uh oh!

Uh oh!

Uh oh!

mbg left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	e instanceof util.ConfigurationError \|\|
	e instanceof util.ConfigurationError \|\|
	// Rate limiting (HTTP 429) is now handled upstream by wrapApiConfigurationError.

Wrap API configuration errors when setting up CodeQL by henrymercer · Pull Request #3217 · github/codeql-action · GitHub

Uh oh!

Wrap API configuration errors when setting up CodeQL - #3217

Wrap API configuration errors when setting up CodeQL#3217henrymercer merged 5 commits intomaingithub/codeql-action:mainfrom henrymercer/http-error-handlinggithub/codeql-action:henrymercer/http-error-handlingCopy head branch name to clipboard

Conversation

henrymercer commented Oct 20, 2025

Risk assessment

Which use cases does this change impact?

How did/will you validate this change?

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

How will you know if something goes wrong after this change is released?

Merge / deployment checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Wrap API configuration errors when setting up CodeQL#3217
henrymercer merged 5 commits into
mainfrom
henrymercer/http-error-handling