-
Notifications
You must be signed in to change notification settings - Fork 3.3k
chore: (studio) ensure to strip out paths from all data when reporting errors in studio #32135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: (studio) ensure to strip out paths from all data when reporting errors in studio #32135
Conversation
…errors in prompt and studio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the studio error reporting functionality to ensure all file paths are stripped from sensitive data before sending error reports to the cloud. The changes add an early exit when crash reporting is disabled and apply path stripping to method arguments.
- Added a check to prevent error reporting when
CYPRESS_CRASH_REPORTS
is set to '0' - Enhanced path stripping to include studio method arguments in addition to error messages and stack traces
- Updated test coverage to validate the new crash reporting behavior and path stripping functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/server/lib/cloud/api/studio/report_studio_error.ts | Added crash reporting disable check and applied path stripping to method arguments |
packages/server/test/unit/cloud/api/studio/report_studio_error_spec.ts | Added test for crash reporting disable behavior and updated test data to validate path stripping in arguments |
Comments suppressed due to low confidence (2)
packages/server/test/unit/cloud/api/studio/report_studio_error_spec.ts:24
- The test cleanup deletes CYPRESS_CRASH_REPORTS but there's no test case that validates the behavior when this environment variable is set to values other than '0' (e.g., '1', 'true', undefined). Consider adding tests to ensure error reporting works correctly in these cases.
delete process.env.CYPRESS_CRASH_REPORTS
packages/server/test/unit/cloud/api/studio/report_studio_error_spec.ts:180
- The test only validates path stripping for a simple object structure. Consider adding tests for more complex nested structures, arrays with paths, and edge cases where paths might appear in different formats to ensure comprehensive coverage of the stripPath functionality.
const args = ['arg1', { key: '/path/to/file.js' }]
cypress
|
Project |
cypress
|
Branch Review |
ryanm/chore/sanitize-error-reporting
|
Run status |
|
Run duration | 11m 10s |
Commit |
|
Committer | Ryan Manuel |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
687
|
View all changes introduced in this branch ↗︎ |
UI Coverage
37.62%
|
|
---|---|
|
131
|
|
79
|
Accessibility
97.38%
|
|
---|---|
|
3 critical
7 serious
1 moderate
1 minor
|
|
62
|
Additional details
We missed a spot where we were stripping paths in studio. This updates the logic to ensure we are properly stripping paths everywhere.
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?