Skip to content

Conversation

WilcoFiers
Copy link
Contributor

@WilcoFiers WilcoFiers commented Aug 28, 2025

Part of #4860

If a check or matches method throws, catch the error and add a node with an error-occurred none check. This way incomplete results are always reported.

This only happens for axe.run / axe.runPartial. rule.runSync() still throws if an occurs in a rule or check.

Comment on lines -746 to -752
description: 'An error occured while running this rule',
message: err.message,
stack: err.stack,
error: err,
// Add a serialized reference to the node the rule failed on for easier debugging.
// See https://github.com/dequelabs/axe-core/issues/1317.
errorNode: err.errorNode
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I got rid of most of these, and updated how error works:

  • error is now serialized so that it can be carried across frame boundaries
  • description is clobbered by the reporter. It never even left axe (plus it has a typo)
  • message and stack are duplicated on error
  • errorNode wasn't getting pulled through nodeSerializer

ErrorNode in particular was a problem. Because it wasn't serialized, it A. doesn't have the frame selector, and B. leaves the _element property on it, which doesn't play well with serialization (axe Extension would completely error out trying to serialize this).

@WilcoFiers WilcoFiers marked this pull request as ready for review September 2, 2025 15:31
@Copilot Copilot AI review requested due to automatic review settings September 2, 2025 15:31
@WilcoFiers WilcoFiers requested a review from a team as a code owner September 2, 2025 15:31
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements error handling for axe-core checks and rules by catching errors and converting them to incomplete results instead of failing completely. This allows the scanner to report what it could successfully check while noting where errors occurred.

  • Introduces a new SupportError class to wrap errors with additional context
  • Adds error handling to rule matching and check evaluation phases
  • Creates a dedicated "error-occurred" check type for incomplete results

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/core/utils/support-error.js New SupportError class for wrapping errors with context
lib/core/utils/serialize-error.js Utility for serializing errors to JSON
lib/core/base/rule.js Updated to catch errors and wrap them in SupportError
lib/core/base/audit.js Added error handling in audit run and after phases
lib/core/utils/merge-results.js Support for merging error information from frames
lib/checks/shared/error-occurred.json New check definition for error cases
test/integration/full/error-occurred/* Integration tests for error handling
test/core/utils/* Unit tests for new utilities

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

WilcoFiers and others added 2 commits September 2, 2025 17:55
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@@ -0,0 +1,23 @@
import serializeError from './serialize-error';

export default function SupportError({ error, ruleId, method, errorNode }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this is not a class that extends Error? If we're going to rewrite it, we may as well use more modern JS features 🤷‍♂️

Comment on lines +29 to +30
const supportError = new SupportError({ error, method: 'test' });
assert.equal(supportError.method, 'test');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as the name so want to just make sure it's different

Suggested change
const supportError = new SupportError({ error, method: 'test' });
assert.equal(supportError.method, 'test');
const supportError = new SupportError({ error, method: '#matches' });
assert.equal(supportError.method, '#matches');

Comment on lines +22 to +24
const supportError = new SupportError({ error, ruleId: 'test' });
assert.equal(supportError.ruleId, 'test');
assert.include(supportError.message, 'Skipping test rule.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const supportError = new SupportError({ error, ruleId: 'test' });
assert.equal(supportError.ruleId, 'test');
assert.include(supportError.message, 'Skipping test rule.');
const supportError = new SupportError({ error, ruleId: 'aria' });
assert.equal(supportError.ruleId, 'aria');
assert.include(supportError.message, 'Skipping aria rule.');

Comment on lines +35 to +36
const supportError = new SupportError({ error, errorNode: 'test' });
assert.equal(supportError.errorNode, 'test');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const supportError = new SupportError({ error, errorNode: 'test' });
assert.equal(supportError.errorNode, 'test');
const supportError = new SupportError({ error, errorNode: 'err' });
assert.equal(supportError.errorNode, 'err');

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.

3 participants