Skip to content

Commit c496c2e

Browse files
authored
fix(browser): fail instead of hanging when the browser stops responding (#10956)
1 parent a8aa1df commit c496c2e

5 files changed

Lines changed: 215 additions & 3 deletions

File tree

packages/browser-playwright/src/playwright.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,14 @@ export class PlaywrightBrowserProvider implements BrowserProvider {
621621
await this._throwIfClosing(page)
622622
this.pages.set(sessionId, page)
623623

624+
// fail the run immediately with an attributed error; otherwise the crash
625+
// is only visible as a websocket disconnect, if the browser closes it at all
626+
page.on('crash', () => {
627+
debug?.('[%s][%s] the page crashed', sessionId, this.browserName)
628+
const session = this.project.vitest._browserSessions.getSession(sessionId)
629+
session?.fail(new Error(`The ${this.browserName} page crashed while running tests. This can happen if the browser ran out of memory.`))
630+
})
631+
624632
if (process.env.VITEST_PW_DEBUG) {
625633
page.on('requestfailed', (request) => {
626634
console.error(

packages/browser/src/node/rpc.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { MockerRegistry } from '@vitest/mocker'
22
import type { IncomingMessage } from 'node:http'
33
import type { Duplex } from 'node:stream'
44
import type { TestError } from 'vitest'
5-
import type { BrowserCommandContext, ResolveSnapshotPathHandlerContext, TestProject } from 'vitest/node'
5+
import type { BrowserCommandContext, ResolveSnapshotPathHandlerContext, TestProject, Vitest } from 'vitest/node'
66
import type { WebSocket } from 'ws'
77
import type { WebSocketBrowserEvents, WebSocketBrowserHandlers } from '../types'
88
import type { ParentBrowserProject } from './projectParent'
@@ -22,6 +22,26 @@ const debug = createDebugger('vitest:browser:api')
2222

2323
const BROWSER_API_PATH = '/__vitest_browser_api__'
2424

25+
const DEFAULT_HEARTBEAT_INTERVAL = 15_000
26+
const HEARTBEAT_MAX_MISSED = 2
27+
let warnedInvalidHeartbeatInterval = false
28+
29+
function resolveHeartbeatInterval(vitest: Vitest): number {
30+
const rawInterval = process.env.VITEST_BROWSER_HEARTBEAT_INTERVAL
31+
if (!rawInterval) {
32+
return DEFAULT_HEARTBEAT_INTERVAL
33+
}
34+
const interval = Number(rawInterval)
35+
if (Number.isNaN(interval)) {
36+
if (!warnedInvalidHeartbeatInterval) {
37+
warnedInvalidHeartbeatInterval = true
38+
vitest.logger.warn(`VITEST_BROWSER_HEARTBEAT_INTERVAL is expected to be a number, received "${rawInterval}". Using the default interval of ${DEFAULT_HEARTBEAT_INTERVAL}ms instead.`)
39+
}
40+
return DEFAULT_HEARTBEAT_INTERVAL
41+
}
42+
return interval
43+
}
44+
2545
export function setupBrowserRpc(globalServer: ParentBrowserProject, defaultMockerRegistry: MockerRegistry): void {
2646
const vite = globalServer.vite
2747
const vitest = globalServer.vitest
@@ -94,8 +114,36 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject, defaultMocke
94114

95115
debug?.('[%s] Browser API connected to %s', rpcId, type)
96116

117+
// if the browser stops answering pings, terminate the socket so the
118+
// "close" handler below rejects pending calls (like `createTesters`)
119+
// instead of the run hanging forever; timeouts that live in the browser
120+
// (`testTimeout`, iframe ack) cannot fire once its process is frozen
121+
const heartbeatInterval = resolveHeartbeatInterval(vitest)
122+
let missedPongs = 0
123+
ws.on('pong', () => {
124+
missedPongs = 0
125+
})
126+
const heartbeat = heartbeatInterval > 0
127+
? setInterval(() => {
128+
if (ws.readyState !== ws.OPEN) {
129+
return
130+
}
131+
if (missedPongs >= HEARTBEAT_MAX_MISSED) {
132+
debug?.('[%s] %s did not respond to %s heartbeat pings, terminating the connection', rpcId, type, missedPongs)
133+
rpc.$close(
134+
new Error(`[vitest] The browser ${type} did not respond to a heartbeat ping for ${missedPongs * heartbeatInterval}ms. The browser process might be frozen or killed. Closing the connection.`),
135+
)
136+
ws.terminate()
137+
return
138+
}
139+
missedPongs++
140+
ws.ping()
141+
}, heartbeatInterval).unref()
142+
: undefined
143+
97144
ws.on('close', () => {
98145
debug?.('[%s] Browser API disconnected from %s', rpcId, type)
146+
clearInterval(heartbeat)
99147
offCancel()
100148
clients.delete(rpcId)
101149
globalServer.removeCDPHandler(rpcId)

packages/vitest/src/node/pools/browser.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { detectCodeBlock } from '../../utils/test-helpers'
1818

1919
const debug = createDebugger('vitest:browser:pool')
2020

21+
const PROVIDER_CLOSE_TIMEOUT = 10_000
22+
2123
export function createBrowserPool(vitest: Vitest): ProcessPool {
2224
const providers = new Set<BrowserProvider>()
2325

@@ -164,7 +166,22 @@ export function createBrowserPool(vitest: Vitest): ProcessPool {
164166
return {
165167
name: 'browser',
166168
async close() {
167-
await Promise.all(Array.from(providers, provider => provider.close()))
169+
// a frozen or crashed browser never answers the close message;
170+
// don't wait for it forever, the browser process is killed
171+
// when this process exits anyway
172+
await Promise.all(Array.from(providers, (provider) => {
173+
let timer: ReturnType<typeof setTimeout>
174+
return Promise.race([
175+
Promise.resolve(provider.close()).finally(() => clearTimeout(timer)),
176+
new Promise<void>((resolve) => {
177+
timer = setTimeout(() => {
178+
vitest.logger.warn(`The browser did not close within ${PROVIDER_CLOSE_TIMEOUT}ms. The browser process will be killed when the process exits.`)
179+
resolve()
180+
}, PROVIDER_CLOSE_TIMEOUT)
181+
timer.unref()
182+
}),
183+
])
184+
}))
168185
vitest._browserSessions.sessionIds.clear()
169186
providers.clear()
170187
vitest.projects.forEach((project) => {

test/browser/specs/bail-out.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ test('fails gracefully when browser crashes', async () => {
77
reporters: [['verbose', { isTTY: false }]],
88
})
99

10-
expect(stderr).toContain('Browser connection was closed while running tests. Was the page closed unexpectedly?')
10+
// the crash is reported over CDP and as a websocket disconnect;
11+
// whichever arrives first fails the run
12+
expect(stderr).toMatch(
13+
/page crashed while running tests|Browser connection was closed while running tests/,
14+
)
1115
})
1216

1317
test('vitest bails out when the iframe is no longer accessible', async () => {
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { execSync } from 'node:child_process'
2+
import { expect, onTestFinished, test } from 'vitest'
3+
import { instances, provider, runInlineBrowserTests } from './utils'
4+
5+
// walk the process tree because the provider does not expose the browser pid
6+
function findDescendantBrowserProcesses(): number[] {
7+
const output = execSync('ps -eo pid=,ppid=,args=', { encoding: 'utf-8' })
8+
const childrenByParent = new Map<number, number[]>()
9+
const argsByPid = new Map<number, string>()
10+
for (const line of output.split('\n')) {
11+
const [pidRaw, ppidRaw, ...args] = line.trim().split(/\s+/)
12+
const pid = Number(pidRaw)
13+
const ppid = Number(ppidRaw)
14+
if (!Number.isInteger(pid) || !Number.isInteger(ppid)) {
15+
continue
16+
}
17+
const children = childrenByParent.get(ppid) ?? []
18+
children.push(pid)
19+
childrenByParent.set(ppid, children)
20+
argsByPid.set(pid, args.join(' '))
21+
}
22+
const browserPids: number[] = []
23+
const queue = [process.pid]
24+
while (queue.length) {
25+
const pid = queue.shift()!
26+
for (const child of childrenByParent.get(pid) ?? []) {
27+
queue.push(child)
28+
if (/headless[ _]shell|chromium|chrome/i.test(argsByPid.get(child) ?? '')) {
29+
browserPids.push(child)
30+
}
31+
}
32+
}
33+
return browserPids
34+
}
35+
36+
function signalAll(pids: number[], signal: NodeJS.Signals) {
37+
for (const pid of pids) {
38+
try {
39+
process.kill(pid, signal)
40+
}
41+
catch {
42+
// the process is already gone
43+
}
44+
}
45+
}
46+
47+
// SIGSTOP freezes the browser without closing its websocket, standing in for
48+
// any browser death that leaves the socket open (vitest-dev/vitest#10791);
49+
// requires a locally launched playwright browser and POSIX signals
50+
test.runIf(
51+
provider.name === 'playwright'
52+
&& process.platform !== 'win32'
53+
&& !process.env.BROWSER_WS_ENDPOINT,
54+
)('fails instead of hanging when the browser stops responding mid-run', { timeout: 60_000 }, async () => {
55+
process.env.VITEST_BROWSER_HEARTBEAT_INTERVAL = '1000'
56+
let frozenPids: number[] = []
57+
onTestFinished(() => {
58+
delete process.env.VITEST_BROWSER_HEARTBEAT_INTERVAL
59+
signalAll(frozenPids, 'SIGCONT')
60+
})
61+
62+
const { ctx, fs } = await runInlineBrowserTests(
63+
{
64+
'basic.test.ts': `
65+
import { test } from 'vitest'
66+
67+
test('first', () => {})
68+
69+
test('never finishes', async () => {
70+
await new Promise(resolve => setTimeout(resolve, 60_000))
71+
})
72+
`,
73+
},
74+
{
75+
reporters: [
76+
{
77+
onTestCaseResult() {
78+
if (!frozenPids.length) {
79+
frozenPids = findDescendantBrowserProcesses()
80+
expect(frozenPids.length).toBeGreaterThan(0)
81+
signalAll(frozenPids, 'SIGSTOP')
82+
}
83+
},
84+
// unfreeze before `startVitest` closes the provider, so the
85+
// browser can answer the close message
86+
onTestRunEnd() {
87+
signalAll(frozenPids, 'SIGCONT')
88+
},
89+
},
90+
],
91+
browser: {
92+
instances: [instances[0]],
93+
},
94+
},
95+
)
96+
97+
const unhandledErrors = ctx!.state.getUnhandledErrors() as Error[]
98+
const messages = unhandledErrors.map((error) => {
99+
const cause = error.cause as Error | undefined
100+
return cause ? `${error.message} ${cause.message}` : error.message
101+
})
102+
expect(messages).toContainEqual(
103+
`Failed to run the test ${fs.resolveFile('basic.test.ts')}. `
104+
+ `[vitest] The browser orchestrator did not respond to a heartbeat ping for 2000ms. `
105+
+ `The browser process might be frozen or killed. Closing the connection.`,
106+
)
107+
})
108+
109+
test('warns when VITEST_BROWSER_HEARTBEAT_INTERVAL is not a number and uses the default', async () => {
110+
process.env.VITEST_BROWSER_HEARTBEAT_INTERVAL = 'not-a-number'
111+
onTestFinished(() => {
112+
delete process.env.VITEST_BROWSER_HEARTBEAT_INTERVAL
113+
})
114+
115+
const { ctx, stderr } = await runInlineBrowserTests(
116+
{
117+
'basic.test.ts': `
118+
import { test } from 'vitest'
119+
120+
test('works', () => {})
121+
`,
122+
},
123+
{
124+
browser: {
125+
instances: [instances[0]],
126+
},
127+
},
128+
)
129+
130+
expect(stderr).toContain(
131+
'VITEST_BROWSER_HEARTBEAT_INTERVAL is expected to be a number, received "not-a-number". '
132+
+ 'Using the default interval of 15000ms instead.',
133+
)
134+
expect(ctx!.state.getUnhandledErrors()).toEqual([])
135+
})

0 commit comments

Comments
 (0)
Sponsor
SponsoredKunjungi sekarang
Promo