Skip to content

feat: add --continueOnError flag to skip a hung/crashing document - #466

Open
sajeeshnaniyil wants to merge 1 commit into
daisy:masterfrom
sajeeshnaniyil:feat/continue-on-per-page-error
Open

feat: add --continueOnError flag to skip a hung/crashing document#466
sajeeshnaniyil wants to merge 1 commit into
daisy:masterfrom
sajeeshnaniyil:feat/continue-on-per-page-error

Conversation

@sajeeshnaniyil

Copy link
Copy Markdown

Adds a --continueOnError flag (also -C, also ACE_CONTINUE_ON_ERROR=1) that turns a per-document failure inside the HTML checker's pMap from fail-fast into log-and-skip. Default behavior is unchanged.

Closes #465.

What changes

  • packages/ace-core/src/checker/checker-chromium.js — the pMap mapper now wraps checkSingle() in a try/catch when continueOnError is set. Failing docs get a winston.warn("Skipping document …") and yield null; nulls are filtered from the results before consolidate() sees them.
  • packages/ace-core/src/checker/checker.js — accepts and forwards the flag.
  • packages/ace-core/src/core/ace.js — computes continueOnError from options.continueOnError || process.env.ACE_CONTINUE_ON_ERROR === '1', following the same shape as doNotReportMedia.
  • packages/ace-cli-shared/src/index.js — exposes -C, --continueOnError in the CLI help + meow flags.

Why

Some real-world EPUBs occasionally have a single content document that either hangs the axe runner or times out. Today that single per-document rejection propagates through pMap's default fail-fast and aborts the entire audit — no report is produced even when 100+ other documents completed fine. Consumers who'd rather trade a small amount of coverage for a completed report have no escape hatch.

This flag is that escape hatch, opt-in only.

For the environment story of what was hanging on our end, and how we ended up not needing this in production once we reverted our own overzealous Electron flags — see #465 and its follow-up comment. The flag is still broadly useful independent of our specific pathology.

Compat

  • No signature break for existing callers — continueOnError is optional at every layer.
  • No change to the report shape or the CLI's exit codes.
  • Off by default, so daisy/ace's current fail-fast semantics are preserved unless someone explicitly opts in.

Tests

I skipped adding a unit test because there aren't any existing tests for checker-chromium.js / checker.js to build on, and simulating a per-document hang inside the pMap in isolation would need a fair bit of mock scaffolding for pMap + axeRunner. Happy to add one if you'd prefer — pointer to any preferred mocking pattern would speed that up.

Commit

Angular-style per CONTRIBUTING.md:

feat: add --continueOnError flag to skip a hung/crashing document

- Wrap checkSingle() inside pMap in a try/catch when continueOnError is set;
  log a warning and yield null for the failing doc instead of aborting.
- Filter nulls from the results before consolidation.
- Plumb the option through checker.js and ace.js in the same shape as
  doNotReportMedia (options.continueOnError || env ACE_CONTINUE_ON_ERROR).
- Expose the CLI flag as -C / --continueOnError.

Default behavior is unchanged: without the flag, pMap's fail-fast still
aborts the whole audit on the first per-doc rejection.

Closes daisy#465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants