Add "Requires authentication" to wrapApiConfigurationError
#3450
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances error handling in the CodeQL Action by adding "Requires authentication" to the list of recognized authentication error messages in the wrapApiConfigurationError function. This ensures that when the GitHub API returns a "Requires authentication" error, it is properly wrapped into a user-friendly ConfigurationError message that guides users to check their token validity and permissions.
Changes:
- Added "Requires authentication" error message check to
wrapApiConfigurationErrorfunction insrc/api-client.ts - Updated generated JavaScript files in the
libdirectory to reflect the TypeScript changes
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/api-client.ts | Added "Requires authentication" to the authentication error check condition, maintaining consistency with existing "Bad credentials" and "Not Found" checks |
| lib/upload-sarif-action.js | Generated JavaScript - reflects the TypeScript source changes |
| lib/upload-lib.js | Generated JavaScript - reflects the TypeScript source changes |
| lib/setup-codeql-action.js | Generated JavaScript - reflects the TypeScript source changes |
| lib/init-action.js | Generated JavaScript - reflects the TypeScript source changes |
| lib/init-action-post.js | Generated JavaScript - reflects the TypeScript source changes |
| lib/analyze-action.js | Generated JavaScript - reflects the TypeScript source changes |
| httpError.message.includes("Bad credentials") || | ||
| httpError.message.includes("Not Found") | ||
| httpError.message.includes("Not Found") || | ||
| httpError.message.includes("Requires authentication") |
There was a problem hiding this comment.
The new error message check for "Requires authentication" lacks test coverage. The existing test in src/api-client.test.ts (line 108) covers "Bad credentials" and "Not Found" cases but doesn't include a test case for "Requires authentication". Consider adding a test case similar to the existing ones to ensure this error is properly wrapped into a ConfigurationError with the token suggestion message.
No description provided.