Skip to content

Commit 08fa092

Browse files
committed
fix: prevent hanging in Playwright acceptance tests with terminal error handling
- Add terminal error detection for ERR_ABORTED and frame detachment errors - Prevent infinite retries for unrecoverable navigation failures - Improve screenshot plugin error handling with browser state checks - Reduce screenshot timeout from 30s to 5s for faster failure - Add terminal error handling in recorder to immediately re-throw fatal errors - Enhance retryFailedStep plugin to skip terminal navigation errors Fixes hanging issues in Playwright acceptance tests when frames are detached or browser connections are lost during iframe/within operations.
1 parent cebab23 commit 08fa092

File tree

4 files changed

+46
-5
lines changed

4 files changed

+46
-5
lines changed

lib/helper/Playwright.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,18 @@ class Playwright extends Helper {
10261026
throw new Error(`Page is not initialized after auto-initialization. this.page=${this.page}, this.isRunning=${this.isRunning}, this.browser=${!!this.browser}, this.browserContext=${!!this.browserContext}`)
10271027
}
10281028

1029-
await this.page.goto(url, { waitUntil: this.options.waitForNavigation })
1029+
try {
1030+
await this.page.goto(url, { waitUntil: this.options.waitForNavigation })
1031+
} catch (err) {
1032+
// Handle terminal navigation errors that shouldn't be retried
1033+
if (err.message && (err.message.includes('ERR_ABORTED') || err.message.includes('frame was detached') || err.message.includes('Target page, context or browser has been closed'))) {
1034+
// Mark this as a terminal error to prevent retries
1035+
const terminalError = new Error(err.message)
1036+
terminalError.isTerminal = true
1037+
throw terminalError
1038+
}
1039+
throw err
1040+
}
10301041

10311042
const performanceTiming = JSON.parse(await this.page.evaluate(() => JSON.stringify(window.performance.timing)))
10321043

lib/plugin/retryFailedStep.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ export default function (config) {
8888
if (!enableRetry) return
8989
if (store.debugMode) return false
9090
if (!store.autoRetries) return false
91+
// Don't retry terminal errors (e.g., frame detachment errors)
92+
if (err && err.isTerminal) return false
93+
// Don't retry navigation errors that are known to be terminal
94+
if (err && err.message && (err.message.includes('ERR_ABORTED') || err.message.includes('frame was detached') || err.message.includes('Target page, context or browser has been closed'))) return false
9195
if (customWhen) return customWhen(err)
9296
return true
9397
}

lib/plugin/screenshotOnFail.js

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,18 @@ export default function (config) {
122122
}
123123
}
124124

125-
// Add timeout wrapper to prevent hanging
125+
// Check if browser/page is still available before attempting screenshot
126+
if (helper.page && helper.page.isClosed && helper.page.isClosed()) {
127+
throw new Error('Browser page has been closed')
128+
}
129+
if (helper.browser && helper.browser.isConnected && !helper.browser.isConnected()) {
130+
throw new Error('Browser has been disconnected')
131+
}
132+
133+
// Add timeout wrapper to prevent hanging with shorter timeout for ESM
126134
const screenshotPromise = helper.saveScreenshot(fileName, options.fullPageScreenshots)
127135
const timeoutPromise = new Promise((_, reject) => {
128-
setTimeout(() => reject(new Error('Screenshot timeout after 30 seconds')), 30000)
136+
setTimeout(() => reject(new Error('Screenshot timeout after 5 seconds')), 5000)
129137
})
130138

131139
await Promise.race([screenshotPromise, timeoutPromise])
@@ -184,8 +192,19 @@ export default function (config) {
184192
if (!quietMode) {
185193
output.plugin('screenshotOnFail', `Failed to save screenshot: ${err.message}`)
186194
}
187-
if (err && err.type && err.type === 'RuntimeError' && err.message && (err.message.indexOf('was terminated due to') > -1 || err.message.indexOf('no such window: target window already closed') > -1)) {
188-
output.log(`Can't make screenshot, ${err}`)
195+
// Enhanced error handling for browser closed scenarios
196+
if (
197+
err &&
198+
((err.message &&
199+
(err.message.includes('Target page, context or browser has been closed') ||
200+
err.message.includes('Browser page has been closed') ||
201+
err.message.includes('Browser has been disconnected') ||
202+
err.message.includes('was terminated due to') ||
203+
err.message.includes('no such window: target window already closed') ||
204+
err.message.includes('Screenshot timeout after'))) ||
205+
(err.type && err.type === 'RuntimeError'))
206+
) {
207+
output.log(`Can't make screenshot, ${err.message}`)
189208
helper.isRunning = false
190209
}
191210
}

lib/recorder.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,13 @@ export default {
279279
if (!promise) promise = Promise.resolve()
280280
return (promise = promise.catch(err => {
281281
if (ignoredErrs.includes(err)) return // already caught
282+
283+
// Handle terminal errors - don't continue, re-throw immediately
284+
if (err && (err.isTerminal || (err.message && (err.message.includes('ERR_ABORTED') || err.message.includes('frame was detached') || err.message.includes('Target page, context or browser has been closed'))))) {
285+
output.log(`${currentQueue()} Terminal Error | ${err} | ${fnDescription || ''}...`)
286+
throw err // Re-throw terminal errors immediately
287+
}
288+
282289
output.log(`${currentQueue()} Error (Non-Terminated) | ${err} | ${fnDescription || ''}...`)
283290
if (!(err instanceof Error)) {
284291
// strange things may happen

0 commit comments

Comments
 (0)