daisy / daisy/ace

Non-deterministic per-page renderer hang in `ace-axe-runner-electron` aborts entire audit; propose "continue on per-page timeout" behavior

Open
#465 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
98
Forks
29
PR merge metrics
No merged PRs in 30d

Description

# Non-deterministic per-page renderer hang in `ace-axe-runner-electron` aborts entire audit; propose "continue on per-page timeout" behavior

**Related:** #318 (open, 2020), #406 (closed, 2024), #429 (closed, 2024)

## Summary

Under Electron in a GPU-less Linux container, `ace-axe-runner-electron` occasionally hangs a single BrowserWindow indefinitely during `axe.run` execution. Because `checker-chromium.js` uses `pMap` with default fail-fast semantics, that single per-page timeout aborts the entire audit — no report is produced despite 100+ other pages having completed successfully.

Two observations distinguish this from previous reports:

1. **The hang is a race**, not a per-page pathology. Across four runs of the same 119-page EPUB with different rule configs, four *different* pages hung. Same DOM, same code, different victim.
2. **The same page finishes in ~2 seconds under headless Chromium** (puppeteer), but hangs forever under Electron via ACE runner. So it's not the DOM triggering a slow axe rule — it's an interaction between the DOM and the Electron runner environment.

## Environment

- ACE 1.4.x (via `@daisy/ace` NPM)
- Node 22.x
- Debian 12 container, no GPU
- Xvfb + dbus-run-session, `ELECTRON_DISABLE_SANDBOX=1`
- Electron ships with `no-sandbox` switch (matches DAISY's default in `packages/ace-axe-runner-electron/src/cli.js`)
- 119-page EPUB, mixed content (Bible commentary — dense `` markup, inline styles, no scripts/tables/media)

## Evidence

**Run 1** — all rules, `--timeout 900000`: audit aborts at 15 min timeout on `f120.xhtml`.
**Run 2** — `runOnly: {type:'tag', values:['cat.color']}`, `--timeout 60000`: hangs on `f114.xhtml`.
**Run 3** — repeat of Run 2: hangs on `f104.xhtml`.
**Run 4** — all rules, `--timeout 60000` (post-patch verification): hangs on `f117.xhtml`, audit completes because of our local patch.

Diagnostic run with headless Chromium + puppeteer (bypassing the ACE runner but running the same axe with the same rule config): all 119 pages complete in <7 s each; max 6.85 s on `f46.xhtml`; average 1.9 s.

Runner log excerpt showing the hang path (from Run 1):

```
[ACE-AXE] axeRunner 5000ms timeout [[EXTEND]] (...) f120.xhtml
[ACE-AXE] axeRunner 900000ms timeout [[FAIL]] (...) f120.xhtml
error: Ace HTML check error: Failed to check Content Document 'f120.xhtml': Timeout :( 900000ms
error: Ace processing error: Error: Failed to check Content Document 'f120.xhtml': Timeout :( 900000ms
```

## Why this differs from #441

#441 is about slow OPF/nav parsing on 20K+ manifest EPUBs — a `DOMParser`/XPath bottleneck. This report is about the Electron runner itself: individual pages hang unpredictably, and one hung page kills the whole run.

## Two proposed changes (behavior-preserving by default)

### 1. `packages/ace-core/src/checker/checker-chromium.js` — continue past per-page timeouts

Currently:

```js
return pMap(epub.contentDocs, doc => {
return checkSingle(doc, epub, lang, doNotReportMedia, axeRunner);
}, { concurrency: ... }).then(...).catch(async err => {
winston.error(`Ace HTML check error: ...`);
throw new Error(err);
});
```

Proposed (opt-in via `--continue-on-error` CLI flag or `ACE_CONTINUE_ON_ERROR=1`, off by default):

```js
return pMap(epub.contentDocs, async doc => {
try {
return await checkSingle(doc, epub, lang, doNotReportMedia, axeRunner);
} catch (err) {
if (!continueOnError) throw err;
winston.warn(`Skipping document ${doc.filepath || doc.href}: ${err.message}`);
return null; // filtered out downstream
}
}, { concurrency: ... }).then(async results => {
await axeRunner.close();
return results.filter(r => r !== null);
})
```

The report gains a `skippedDocuments: [...]` field so consumers know which pages didn't contribute.

This addresses the "and maybe continue" ask from #406 directly.

### 2. `packages/ace-axe-runner-electron/src/init.js` — recycle BrowserWindow on `[[FAIL]]`

When the extended-timeout path fires (`browserWindow.ace__timeoutExtended === true`), the frozen window is left in `browserWindows[]`. Subsequent spine items routed to that pool slot will hang again.

Proposed: after sending the `AXE_RUNNER_RUN_` error, `browserWindow.destroy()` and rebuild a fresh window in the same pool slot. Prevents cascading failures when multiple pages might hang, and prevents renderer state accumulation across long runs.

Draft helper is ~50 lines; happy to submit as a separate PR from (1) since it's independent.

## Would a PR be welcome?

Both patches are running in our production container against real customer EPUBs. Zero-regression on non-hanging books; the previously-unusable 415018 now completes with 1 page skipped (out of 119). Would you like me to open PR(s) against `master` or a specific branch?

Contributor guide

Open the contributing guide

Research direction

Start in packages/ace-core/src/checker/checker-chromium.js at the pMap error path, then inspect packages/ace-axe-runner-electron/src/init.js around the extended-timeout handling. Compare the proposed opt-in continuation and BrowserWindow recycling with existing timeout behavior. Done means skipped documents are reported, frozen windows are replaced, default behavior is preserved, and the 119-page verification scenario completes with its skipped page recorded.

Written by the indexing model from the issue text.

Assessment

Tech stack
electron, javascript
Domain
accessibility, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.