- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.4k
fix: throw when error in before:browser:launch #32784
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
Conversation
| 
  cypress 
   | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Project | cypress | 
| Branch Review | fix-hang-in-before-browser-launch | 
| Run status |  | 
| Run duration | 26m 47s | 
| Commit |  | 
| Committer | Jennifer Shehane | 
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|  | 1 | 
|  | 11 | 
|  | 1102 | 
|  | 4 | 
|  | 26754 | 
| View all changes introduced in this branch ↗︎ | |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
| UI Coverage 45.35% | |
|---|---|
|  | 189 | 
|  | 161 | 
| Accessibility 97.98% | |
|---|---|
|  | 4 critical8 serious2 moderate2 minor | 
|  | 101 | 
Tests for review
 cypress/e2e/commands/window.cy.js • 1 failed test • 5x-driver-firefox
   cypress/e2e/commands/window.cy.js • 1 failed test • 5x-driver-firefox 
  | Test | Artifacts | |||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| ... > only logs once | 
 | |||||||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| issue 28527 > fails and then retries and verifies about:blank is not displayed | Test ReplayScreenshots | |
 e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-inject-document-domain-chrome
   e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-inject-document-domain-chrome 
  | Test | Artifacts | |
|---|---|---|
| cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary | Test Replay | |
 commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox
   commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox 
  | Test | Artifacts | |||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| ... > stops waiting when an xhr request is canceled | 
 | |||||||||||||
| Test | Artifacts | |||||||
|---|---|---|---|---|---|---|---|---|
| src/cy/commands/files > #readFile > retries to read when ENOENT | 
 | |||||||
| Test | Artifacts | |
|---|---|---|
| cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary | The first 5 flaky specs are shown, see all 11 specs in Cypress Cloud. | |
|  | ||
| // Re-throw the error with proper context using the existing PLUGINS_RUN_EVENT_ERROR | ||
| errors.throwErr('PLUGINS_RUN_EVENT_ERROR', 'before:browser:launch', err) | ||
| } | 
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.
Bug: Telemetry Span Leak in Error Handling
Resource leak: The telemetry span is not properly ended if extendLaunchOptionsFromPlugins() throws an error. The try-catch block only wraps plugins.execute(), but extendLaunchOptionsFromPlugins() is called after the try-catch and can throw UNEXPECTED_BEFORE_BROWSER_LAUNCH_PROPERTIES. When it throws, the span started on line 138 will never be ended, causing a telemetry span leak. The try-catch should be extended to include the call to extendLaunchOptionsFromPlugins(), or a finally block should be used to ensure the span is always ended.
before:browser:launchcauses Cypress to hang when launching the browser #32775Additional details
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?Note
Catches errors from
before:browser:launch, rethrows asPLUGINS_RUN_EVENT_ERROR, updates messaging to reference the configuration file, and adds unit/snapshot test coverage.plugins.execute('before:browser:launch', ...)in try/catch inpackages/server/lib/browsers/utils.ts; end span and rethrow viaerrors.throwErr('PLUGINS_RUN_EVENT_ERROR', 'before:browser:launch', err).PLUGINS_RUN_EVENT_ERRORtext to say "configuration file" instead of "plugins file" inpackages/errors/src/errors.ts.executeBeforeBrowserLaunchandextendLaunchOptionsFromPluginsinpackages/server/test/unit/browsers/utils_spec.ts(success, error, and validation cases).packages/errors/test/__snapshots__/PLUGINS_RUN_EVENT_ERROR.ansiand system test snapshots to reflect new error messaging and behavior.Written by Cursor Bugbot for commit 4c00287. This will update automatically on new commits. Configure here.