fix: change regex matching for API error to not contain regex boundaries #2832
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tch a string due to boundary constraints on the regex
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with the regex used to match API error messages by removing the regex boundaries so that error messages containing extra context can be detected for proper telemetry reporting. It also adds additional tests to verify the behavior of getActionsStatus and wrapApiConfigurationError functions across both TypeScript and JavaScript implementations.
- Updated the regex pattern in API client files to remove boundaries.
- Added test cases for getActionsStatus and API error wrapping behavior.
- Extended imports and helper functions in test files to support the new error handling logic.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/status-report.test.ts | Added tests for various error types and updated imports |
| src/api-client.ts | Modified regex to remove boundaries for improved API error matching |
| src/api-client.test.ts | Added tests for the updated API error wrapping behavior |
| lib/status-report.test.js | Added JavaScript tests mirroring TypeScript tests for error handling |
| lib/api-client.test.js | Added JavaScript tests for API error wrapping functionality |
| lib/api-client.js | Modified regex to remove boundaries in API error matching |
Comments suppressed due to low confidence (3)
src/status-report.test.ts:214
- The test description for a ConfigurationError with an additional failure cause states it should return 'failure', but the expected result is 'user-error'. Please update the description to accurately reflect the expected outcome.
t.is(getActionsStatus(new ConfigurationError("exit code 1"), "multiple things went wrong"), "user-error", "getActionsStatus should return failure if passed a configuration error and an additional failure cause");
src/api-client.ts:248
- Removing the regex boundaries could allow unintended partial matches; please confirm that this broader matching is intended and consider adding comments or tests to ensure it doesn't capture unwanted error messages.
/ref .* not found in this repository/.test(e.message)
lib/api-client.js:209
- Similar to the TypeScript version, removing the regex boundaries here may lead to broader than intended matches. Confirm this change with appropriate documentation or tests to avoid potential false positives.
/ref .* not found in this repository/.test(e.message)
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
henrymercer
approved these changes
Mar 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This fixes an issue with a regex pattern match on API errors failing due to regex boundaries on the pattern, which would affect telemetry.
Merge / deployment checklist
Note: This change doesn't affect users and doesn't require any changes to the
readmeor thechangelog.