Skip to content
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

fix: set a crashed flag when cri detects a crash #27909

Merged
merged 16 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ _Released 09/27/2023_

- Fixed an issue where actionability checks trigger a flood of font requests. Removing the font requests has the potential to improve performance and removes clutter from Test Replay. Addressed in [#27860](https://github.com/cypress-io/cypress/pull/27860).
- Fixed network stubbing not permitting status code 999. Fixes [#27567](https://github.com/cypress-io/cypress/issues/27567). Addressed in [#27853](https://github.com/cypress-io/cypress/pull/27853).
- Fixed an issue where a crashed Chrome renderer can cause the Test Replay recorder to hang. Addressed in [#27909](https://github.com/cypress-io/cypress/pull/27909).

## 13.2.0

Expand Down
10 changes: 10 additions & 0 deletions packages/server/lib/browsers/cri-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export const create = async (

let closed = false // has the user called .close on this?
let connected = false // is this currently connected to CDP?
let crashed = false // has this crashed?

let cri: CDPClient
let client: CriClient
Expand Down Expand Up @@ -246,6 +247,11 @@ export const create = async (
if (!process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF) {
cri.on('disconnect', retryReconnect)
}

cri.on('Inspector.targetCrashed', async () => {
debug('crash detected')
crashed = true
})
}

await connect()
Expand All @@ -254,6 +260,10 @@ export const create = async (
targetId: target,

async send (command: CdpCommand, params?: object) {
if (crashed) {
return Promise.reject(new Error(`${command} will not run as the target browser or tab CRI connection has crashed`))
}

const enqueue = () => {
debug('enqueing command', { command, params })

Expand Down
8 changes: 8 additions & 0 deletions packages/server/test/unit/browsers/cri-client_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,14 @@ describe('lib/browsers/cri-client', function () {
.to.be.rejectedWith(err)
})

it('rejects if target has crashed', async function () {
const command = 'DOM.getDocument'
const client = await getClient()

await criStub.on.withArgs('Inspector.targetCrashed').args[0][1]()
await expect(client.send(command, { depth: -1 })).to.be.rejectedWith(`${command} will not run as the target browser or tab CRI connection has crashed`)
})

context('retries', () => {
([
'WebSocket is not open',
Expand Down
20 changes: 17 additions & 3 deletions system-tests/__snapshots__/browser_crash_handling_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ exports['Browser Crash Handling / when the tab crashes in chrome / fails'] = `
Running: chrome_tab_crash.cy.js (1 of 2)


1) navigates to about /html

We detected that the Chrome Renderer process just crashed.

Expand All @@ -41,13 +42,19 @@ https://on.cypress.io/renderer-process-crashed
│ Failing: 1 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0
│ Screenshots: 1
│ Video: false │
│ Duration: X seconds │
│ Spec Ran: chrome_tab_crash.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Screenshots)

- /XXX/XXX/XXX/cypress/screenshots/chrome_tab_crash.cy.js/navigates to about html (1280x720)
(failed).png


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: simple.cy.js (2 of 2)
Expand Down Expand Up @@ -108,6 +115,7 @@ exports['Browser Crash Handling / when the tab crashes in electron / fails'] = `
Running: chrome_tab_crash.cy.js (1 of 2)


1) navigates to about /html

We detected that the Electron Renderer process just crashed.

Expand All @@ -132,13 +140,19 @@ https://on.cypress.io/renderer-process-crashed
│ Failing: 1 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0
│ Screenshots: 1
│ Video: false │
│ Duration: X seconds │
│ Spec Ran: chrome_tab_crash.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Screenshots)

- /XXX/XXX/XXX/cypress/screenshots/chrome_tab_crash.cy.js/navigates to about html (2560x1440)
(failed).png


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: simple.cy.js (2 of 2)
Expand Down Expand Up @@ -492,7 +506,7 @@ exports['Browser Crash Handling / when the tab closes in chrome / fails'] = `
Running: chrome_tab_close.cy.js (1 of 2)


We detected that the Chrome browser process closed unexpectedly.
We detected that the Chrome tab running Cypress tests closed unexpectedly.

We have failed the current spec and aborted the run.

Expand Down
82 changes: 70 additions & 12 deletions system-tests/__snapshots__/record_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3468,7 +3468,7 @@ exports['e2e record capture-protocol enabled protocol runtime errors error in pr

`

exports['capture-protocol api errors upload 500 - retries 8 times continues 1'] = `
exports['capture-protocol api errors upload 500 - retries 7 times and succeeds on the last call continues 1'] = `

====================================================================================================

Expand Down Expand Up @@ -3524,12 +3524,12 @@ exports['capture-protocol api errors upload 500 - retries 8 times continues 1']

- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay
- Test Replay - 1 kB

(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Failed Uploading 2/2 - Internal Server Error
- Test Replay - Done Uploading 1 kB 2/2

====================================================================================================

Expand All @@ -3550,7 +3550,7 @@ exports['capture-protocol api errors upload 500 - retries 8 times continues 1']

`

exports['capture-protocol api errors upload 500 - retries 7 times and succeeds on the last call continues 1'] = `
exports['capture-protocol api errors upload 500 - retries 8 times and fails continues 1'] = `

====================================================================================================

Expand Down Expand Up @@ -3606,12 +3606,12 @@ exports['capture-protocol api errors upload 500 - retries 7 times and succeeds o

- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - 1 kB
- Test Replay

(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Done Uploading 1 kB 2/2
- Test Replay - Failed Uploading 2/2 - Internal Server Error

====================================================================================================

Expand All @@ -3632,7 +3632,7 @@ exports['capture-protocol api errors upload 500 - retries 7 times and succeeds o

`

exports['capture-protocol api errors upload 500 - retries 8 times and fails continues 1'] = `
exports['e2e record capture-protocol enabled crashing does not hang when the tab crashes & continues with next spec 1'] = `

====================================================================================================

Expand All @@ -3641,16 +3641,72 @@ exports['capture-protocol api errors upload 500 - retries 8 times and fails cont
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (record_pass.cy.js)
│ Searched: cypress/e2e/record_pass*
│ Specs: 2 found (chrome_tab_crash.cy.js, record_pass.cy.js) │
│ Searched: cypress/e2e/chrome_tab_crash*, cypress/e2e/record_pass*
│ Params: Tag: false, Group: false, Parallel: false │
│ Run URL: https://dashboard.cypress.io/projects/cjvoj7/runs/12 │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: record_pass.cy.js (1 of 1)
Running: chrome_tab_crash.cy.js (1 of 2)
Estimated: X second(s)


1) navigates to about /html

We detected that the Chrome Renderer process just crashed.

We have failed the current spec but will continue running the next spec.

This can happen for a number of different reasons.

If you're running lots of tests on a memory intense application.
- Try increasing the CPU/memory on the machine you're running on.
- Try enabling experimentalMemoryManagement in your config file.
- Try lowering numTestsKeptInMemory in your config file during 'cypress open'.

You can learn more here:

https://on.cypress.io/renderer-process-crashed

(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 0 │
│ Passing: 0 │
│ Failing: 1 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 1 │
│ Video: false │
│ Duration: X seconds │
│ Estimated: X second(s) │
│ Spec Ran: chrome_tab_crash.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Screenshots)

- /XXX/XXX/XXX/cypress/screenshots/chrome_tab_crash.cy.js/navigates to about html (1000x660)
(failed).png


(Uploading Cloud Artifacts)

- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/chrome_tab_crash.cy.js/navigates to about html (failed).png
- Test Replay - 1 kB

(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB 1/2 /XXX/XXX/XXX/cypress/screenshots/chrome_tab_crash.cy.js/navigates to about html (failed).png
- Test Replay - Done Uploading 1 kB 2/2

────────────────────────────────────────────────────────────────────────────────────────────────────

Running: record_pass.cy.js (2 of 2)
Estimated: X second(s)


Expand Down Expand Up @@ -3693,7 +3749,7 @@ exports['capture-protocol api errors upload 500 - retries 8 times and fails cont
(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Failed Uploading 2/2 - Internal Server Error
- Test Replay - Done Uploading 1 kB 2/2

====================================================================================================

Expand All @@ -3702,9 +3758,11 @@ exports['capture-protocol api errors upload 500 - retries 8 times and fails cont

Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✖ chrome_tab_crash.cy.js XX:XX - - 1 - - │
├────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ✔ record_pass.cy.js XX:XX 2 1 - 1 - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 2 1 - 1 -
✖ 1 of 2 failed (50%) XX:XX 2 1 1 1 -


───────────────────────────────────────────────────────────────────────────────────────────────────────
Expand Down
26 changes: 21 additions & 5 deletions system-tests/lib/protocol-stubs/protocolStub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface {
resetTest: [],
responseEndedWithEmptyBody: [],
}
private cdpClient: any
private scriptToEvaluateId: any

getDbMetadata (): { offset: number, size: number } {
return {
Expand All @@ -54,10 +56,20 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface {
this.events.responseEndedWithEmptyBody = []
}

connectToBrowser = (cdpClient) => {
if (cdpClient) this.events.connectToBrowser.push(true)
connectToBrowser = async (cdpClient) => {
if (cdpClient) {
this.events.connectToBrowser.push(true)
this.cdpClient = cdpClient
}

return Promise.resolve()
const scriptToEvaluateResult = await this.cdpClient.send(
'Page.addScriptToEvaluateOnNewDocument',
{
source: `(function () {})()`,
},
)

this.scriptToEvaluateId = scriptToEvaluateResult.identifier
}

addRunnables = (runnables) => {
Expand All @@ -76,7 +88,7 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface {
}
}

afterSpec = () => {
async afterSpec (): Promise<void> {
this.events.afterSpec.push(true)

// since the order of the logs can vary per run, we sort them by id to ensure the snapshot can be compared
Expand All @@ -90,7 +102,11 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface {
console.log('error writing protocol events', e)
}

return Promise.resolve()
await this.cdpClient.send('Page.removeScriptToEvaluateOnNewDocument', {
identifier: this.scriptToEvaluateId || '',
})
.catch(() => {
})
}

beforeTest = (test) => {
Expand Down
5 changes: 5 additions & 0 deletions system-tests/projects/e2e/cypress/e2e/chrome_tab_crash.cy.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
it('navigates to about /html', () => {
cy.visit('/html')
cy.contains('Herman Melville')
})

it('crashes the chrome tab', () => {
Cypress.automation('remote:debugger:protocol', { command: 'Page.navigate', params: { url: 'chrome://crash', transitionType: 'typed' } })
cy.visit('localhost')
Expand Down
15 changes: 15 additions & 0 deletions system-tests/test/record_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2308,6 +2308,21 @@ describe('e2e record', () => {
})
})

describe('crashing', () => {
enableCaptureProtocol()
it('does not hang when the tab crashes & continues with next spec', function () {
return systemTests.exec(this, {
browser: 'chrome',
key: 'f858a2bc-b469-4e48-be67-0876339ee7e1',
configFile: 'cypress-with-project-id.config.js',
spec: 'chrome_tab_crash*,record_pass*',
record: true,
snapshot: true,
expectedExitCode: 1,
})
})
})

describe('protocol runtime errors', () => {
enableCaptureProtocol()
describe('db size too large', () => {
Expand Down