Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/errors/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1309,7 +1309,7 @@ export const AllCypressErrors = {
},
PLUGINS_RUN_EVENT_ERROR: (arg1: string, arg2: Error) => {
return errTemplate`\
An error was thrown in your plugins file while executing the handler for the ${fmt.highlight(arg1)} event.
An error was thrown in your configuration file while executing the handler for the ${fmt.highlight(arg1)} event.

The error we received was:

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
An error was thrown in your plugins file while executing the handler for the before:spec event.
An error was thrown in your configuration file while executing the handler for the before:spec event.

The error we received was:

Expand Down
11 changes: 10 additions & 1 deletion packages/server/lib/browsers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,16 @@ async function executeBeforeBrowserLaunch (browser, launchOptions: typeof defaul
isHeadless: browser.isHeadless,
})

const pluginConfigResult = await plugins.execute('before:browser:launch', browser, launchOptions)
let pluginConfigResult

try {
pluginConfigResult = await plugins.execute('before:browser:launch', browser, launchOptions)
} catch (err) {
span?.end()

// Re-throw the error with proper context using the existing PLUGINS_RUN_EVENT_ERROR
errors.throwErr('PLUGINS_RUN_EVENT_ERROR', 'before:browser:launch', err)
}
Copy link

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.

Fix in Cursor Fix in Web


span?.end()

Expand Down
265 changes: 265 additions & 0 deletions packages/server/test/unit/browsers/utils_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
require('../../spec_helper')
import 'chai-as-promised'
import { expect } from 'chai'
import sinon from 'sinon'
import utils from '../../../lib/browsers/utils'
import * as plugins from '../../../lib/plugins'
import * as errors from '../../../lib/errors'

describe('lib/browsers/utils', () => {
let browser: any
let launchOptions: any
let options: any
let pluginsHasStub: sinon.SinonStub
let pluginsExecuteStub: sinon.SinonStub
let errorsThrowErrStub: sinon.SinonStub

beforeEach(() => {
browser = {
name: 'chrome',
channel: 'stable',
version: '120.0.0',
isHeadless: false,
}

launchOptions = {
preferences: {},
extensions: [],
args: [],
env: {},
}

options = {}

pluginsHasStub = sinon.stub(plugins, 'has')
pluginsExecuteStub = sinon.stub(plugins, 'execute')
errorsThrowErrStub = sinon.stub(errors, 'throwErr')
})

afterEach(() => {
sinon.restore()
})

describe('executeBeforeBrowserLaunch', () => {
context('when before:browser:launch hook is not registered', () => {
it('should not execute the hook and return launchOptions unchanged', async () => {
pluginsHasStub.withArgs('before:browser:launch').returns(false)

const result = await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(result).to.equal(launchOptions)
expect(pluginsExecuteStub).not.to.be.called
expect(errorsThrowErrStub).not.to.be.called
})
})

context('when before:browser:launch hook is registered', () => {
beforeEach(() => {
pluginsHasStub.withArgs('before:browser:launch').returns(true)
})

context('successful execution', () => {
it('should execute successfully when plugin returns null', async () => {
pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).resolves(null)

const result = await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(result).to.equal(launchOptions)
expect(errorsThrowErrStub).not.to.be.called
})

it('should execute successfully when plugin returns undefined', async () => {
pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).resolves(undefined)

const result = await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(result).to.equal(launchOptions)
expect(errorsThrowErrStub).not.to.be.called
})

it('should execute successfully when plugin returns valid launch options', async () => {
const pluginResult = {
preferences: { 'test-preference': 'value' },
args: ['--test-arg'],
extensions: ['/path/to/extension'],
env: { TEST_ENV: 'value' },
}

pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).resolves(pluginResult)

const result = await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(result).to.equal(launchOptions)
expect(errorsThrowErrStub).not.to.be.called
})
})

context('error handling', () => {
it('should handle plugin errors', async () => {
const testError = new Error('Something went wrong in the plugin')

testError.stack = 'Error: Something went wrong in the plugin\n at Object.<anonymous> (/path/to/plugins.js:5:10)'

pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).rejects(testError)

await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(errorsThrowErrStub).to.be.calledWith('PLUGINS_RUN_EVENT_ERROR', 'before:browser:launch', testError)
})
})

context('launch options validation', () => {
it('should handle unexpected properties in plugin result', async () => {
const pluginResult = {
preferences: { 'test-preference': 'value' },
invalidProperty: 'should-not-be-here',
anotherInvalidProperty: 'also-invalid',
}

pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).resolves(pluginResult)

await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(errorsThrowErrStub).to.be.calledWith(
'UNEXPECTED_BEFORE_BROWSER_LAUNCH_PROPERTIES',
['invalidProperty', 'anotherInvalidProperty'],
['preferences', 'extensions', 'args', 'env'],
)
})

it('should handle mixed valid and invalid properties', async () => {
const pluginResult = {
preferences: { 'test-preference': 'value' },
args: ['--test-arg'],
invalidProperty: 'should-not-be-here',
}

pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).resolves(pluginResult)

await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(errorsThrowErrStub).to.be.calledWith(
'UNEXPECTED_BEFORE_BROWSER_LAUNCH_PROPERTIES',
['invalidProperty'],
['preferences', 'extensions', 'args', 'env'],
)
})
})
})
})

describe('extendLaunchOptionsFromPlugins', () => {
let pluginConfigResult: any

beforeEach(() => {
pluginConfigResult = {
preferences: { 'test-preference': 'value' },
args: ['--test-arg'],
extensions: ['/path/to/extension'],
env: { TEST_ENV: 'value' },
}
})

context('successful extension', () => {
it('should extend launch options with plugin result', () => {
const result = utils.extendLaunchOptionsFromPlugins(launchOptions, pluginConfigResult, options)

expect(result).to.equal(launchOptions)
expect(launchOptions.preferences).to.deep.equal({ 'test-preference': 'value' })
expect(launchOptions.args).to.deep.equal(['--test-arg'])
expect(launchOptions.extensions).to.deep.equal(['/path/to/extension'])
expect(launchOptions.env).to.deep.equal({ TEST_ENV: 'value' })
})

it('should merge object properties correctly', () => {
launchOptions.preferences = { existing: 'value' }
pluginConfigResult.preferences = { new: 'value' }

utils.extendLaunchOptionsFromPlugins(launchOptions, pluginConfigResult, options)

expect(launchOptions.preferences).to.deep.equal({
existing: 'value',
new: 'value',
})
})

it('should replace non-object properties', () => {
launchOptions.args = ['existing-arg']
pluginConfigResult.args = ['new-arg']

utils.extendLaunchOptionsFromPlugins(launchOptions, pluginConfigResult, options)

expect(launchOptions.args).to.deep.equal(['new-arg'])
})
})

context('error handling', () => {
it('should handle unexpected properties', () => {
pluginConfigResult.invalidProperty = 'value'

utils.extendLaunchOptionsFromPlugins(launchOptions, pluginConfigResult, options)

expect(errorsThrowErrStub).to.be.calledWith(
'UNEXPECTED_BEFORE_BROWSER_LAUNCH_PROPERTIES',
['invalidProperty'],
['preferences', 'extensions', 'args', 'env'],
)
})

it('should handle multiple unexpected properties', () => {
pluginConfigResult.invalidProperty1 = 'value1'
pluginConfigResult.invalidProperty2 = 'value2'

utils.extendLaunchOptionsFromPlugins(launchOptions, pluginConfigResult, options)

expect(errorsThrowErrStub).to.be.calledWith(
'UNEXPECTED_BEFORE_BROWSER_LAUNCH_PROPERTIES',
['invalidProperty1', 'invalidProperty2'],
['preferences', 'extensions', 'args', 'env'],
)
})
})
})

describe('integration scenarios', () => {
beforeEach(() => {
pluginsHasStub.withArgs('before:browser:launch').returns(true)
})

it('should handle plugin errors', async () => {
const testError = new TypeError('Cannot read property "a" of undefined')

testError.stack = 'TypeError: Cannot read property "a" of undefined\n at Object.<anonymous> (/path/to/plugins.js:2:15)'

pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).rejects(testError)

await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(errorsThrowErrStub).to.be.calledWith('PLUGINS_RUN_EVENT_ERROR', 'before:browser:launch', testError)
})

it('should handle complex plugin scenarios with mixed success and failure', async () => {
// First call succeeds
pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).resolves({
preferences: { 'test-preference': 'value' },
})

let result = await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(result).to.equal(launchOptions)
expect(errorsThrowErrStub).not.to.be.called

// Reset for second call
errorsThrowErrStub.resetHistory()

// Second call fails
const testError = new Error('Plugin failed on second call')

pluginsExecuteStub.withArgs('before:browser:launch', browser, launchOptions).rejects(testError)

await utils.executeBeforeBrowserLaunch(browser, launchOptions, options)

expect(errorsThrowErrStub).to.be.calledWith('PLUGINS_RUN_EVENT_ERROR', 'before:browser:launch', testError)
})
})
})
10 changes: 8 additions & 2 deletions system-tests/__snapshots__/deprecated_spec.ts.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ exports['deprecated before:browser:launch args / displays errors thrown and abor
────────────────────────────────────────────────────────────────────────────────────────────────────

Running: app.cy.js (1 of 2)
Error thrown from plugins handler
An error was thrown in your configuration file while executing the handler for the before:browser:launch event.

The error we received was:

Error: Error thrown from plugins handler
[stack trace lines]
`
Expand All @@ -71,7 +74,10 @@ exports['deprecated before:browser:launch args / displays promises rejected and
────────────────────────────────────────────────────────────────────────────────────────────────────

Running: app.cy.js (1 of 2)
Promise rejected from plugins handler
An error was thrown in your configuration file while executing the handler for the before:browser:launch event.

The error we received was:

Error: Promise rejected from plugins handler
[stack trace lines]
`
2 changes: 1 addition & 1 deletion system-tests/__snapshots__/plugin_run_events_spec.ts.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ exports['e2e plugin run events / fails run if event handler throws'] = `
────────────────────────────────────────────────────────────────────────────────────────────────────

Running: run_events_spec_1.cy.js (1 of 2)
An error was thrown in your plugins file while executing the handler for the before:spec event.
An error was thrown in your configuration file while executing the handler for the before:spec event.

The error we received was:

Expand Down
Loading