Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions axe.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ declare namespace axe {
toolOptions: RunOptions;
passes: Result[];
violations: Result[];
incomplete: Result[];
incomplete: IncompleteResult[];
inapplicable: Result[];
}
interface Result {
Expand All @@ -167,6 +167,9 @@ declare namespace axe {
tags: TagValue[];
nodes: NodeResult[];
}
interface IncompleteResult extends Result {
error: Omit<SupportError, 'errorNode'>;
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
error: Omit<SupportError, 'errorNode'>;
error?: Omit<SupportError, 'errorNode'>;

}
interface NodeResult {
html: string;
impact?: ImpactValue;
Expand Down Expand Up @@ -204,6 +207,21 @@ declare namespace axe {
fail: string | { [key: string]: string };
incomplete?: string | { [key: string]: string };
}
interface SupportError {
name: string;
message: string;
stack: string;
ruleId?: string;
method?: string;
cause?: SerialError;
errorNode?: DqElement;
}
interface SerialError {
message: string;
stack: string;
name: string;
cause?: SerialError;
}
interface CheckLocale {
[key: string]: CheckMessages;
}
Expand Down Expand Up @@ -461,7 +479,13 @@ declare namespace axe {
isLabelledShadowDomSelector: (
selector: unknown
) => selector is LabelledShadowDomSelector;

SupportError: (
error: Error,
ruleId?: string,
method?: string,
errorNode?: DqElement
) => SupportError;
serializeError: (error: Error) => SerialError;
DqElement: DqElementConstructor;
uuid: (
options?: { random?: Uint8Array | Array<number> },
Expand Down
10 changes: 10 additions & 0 deletions lib/checks/shared/error-occurred.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"id": "error-occurred",
"evaluate": "exists-evaluate",
"metadata": {
"messages": {
"pass": "",
"incomplete": "Axe encountered an error; test the page for this type of problem manually"
}
}
}
48 changes: 25 additions & 23 deletions lib/core/base/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,14 @@ export default class Audit {
'Result for unknown rule. You may be running mismatch axe-core versions'
);
}
return rule.after(ruleResult, options);
try {
return rule.after(ruleResult, options);
} catch (err) {
if (options.debug) {
throw err;
}
return createIncompleteErrorResult(rule, err);
}
});
}
/**
Expand Down Expand Up @@ -732,36 +739,31 @@ function getDefferedRule(rule, context, options) {
rule.run(
context,
options,
// resolve callback for rule `run`
ruleResult => {
// resolve
resolve(ruleResult);
},
// reject callback for rule `run`
ruleResult => resolve(ruleResult),
err => {
// if debug - construct error details
if (!options.debug) {
const errResult = Object.assign(new RuleResult(rule), {
result: constants.CANTTELL,
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
Comment on lines -746 to -752
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).

});
// resolve
resolve(errResult);
} else {
// reject
if (options.debug) {
reject(err);
} else {
resolve(createIncompleteErrorResult(rule, err));
}
}
);
};
}

function createIncompleteErrorResult(rule, { errorNode, ...error }) {
const none = [
{ id: 'error-occurred', result: undefined, data: error, relatedNodes: [] }
];
const node = errorNode || new DqElement(document.documentElement);

return Object.assign(new RuleResult(rule), {
error,
result: constants.CANTTELL,
nodes: [{ any: [], all: [], none, node }]
});
}

/**
* For all the rules, create the helpUrl and add it to the data for that rule
*/
Expand Down
57 changes: 38 additions & 19 deletions lib/core/base/rule.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/*global SupportError */
import { createExecutionContext } from './check';
import RuleResult from './rule-result';
import {
Expand All @@ -8,7 +7,8 @@ import {
queue,
DqElement,
select,
assert
assert,
SupportError
} from '../utils';
import { isVisibleToScreenReaders } from '../../commons/dom';
import constants from '../constants';
Expand Down Expand Up @@ -181,7 +181,16 @@ Rule.prototype.runChecks = function runChecks(
const check = self._audit.checks[c.id || c];
const option = getCheckOption(check, self.id, options);
checkQueue.defer((res, rej) => {
check.run(node, option, context, res, rej);
check.run(node, option, context, res, error => {
rej(
new SupportError({
ruleId: self.id,
method: `${check.id}#evaluate`,
errorNode: new DqElement(node),
error
})
);
});
});
});

Expand Down Expand Up @@ -235,8 +244,7 @@ Rule.prototype.run = function run(context, options = {}, resolve, reject) {
// Matches throws an error when it lacks support for document methods
nodes = this.gatherAndMatchNodes(context, options);
} catch (error) {
// Exit the rule execution if matches fails
reject(new SupportError({ cause: error, ruleId: this.id }));
reject(error);
return;
}

Expand Down Expand Up @@ -312,15 +320,7 @@ Rule.prototype.runSync = function runSync(context, options = {}) {
}

const ruleResult = new RuleResult(this);
let nodes;

try {
nodes = this.gatherAndMatchNodes(context, options);
} catch (error) {
// Exit the rule execution if matches fails
throw new SupportError({ cause: error, ruleId: this.id });
}

const nodes = this.gatherAndMatchNodes(context, options);
if (options.performanceTimer) {
this._logGatherPerformance(nodes);
}
Expand Down Expand Up @@ -451,7 +451,18 @@ Rule.prototype.gatherAndMatchNodes = function gatherAndMatchNodes(
performanceTimer.mark(markMatchesStart);
}

nodes = nodes.filter(node => this.matches(node.actualNode, node, context));
nodes = nodes.filter(node => {
try {
return this.matches(node.actualNode, node, context);
} catch (error) {
throw new SupportError({
ruleId: this.id,
method: `#matches`,
errorNode: new DqElement(node.actualNode),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: new DqElement can be passed a virtualNode directly

Suggested change
errorNode: new DqElement(node.actualNode),
errorNode: new DqElement(node),

error: error
});
}
});

if (options.performanceTimer) {
performanceTimer.mark(markMatchesEnd);
Expand Down Expand Up @@ -542,12 +553,20 @@ function sanitizeNodes(result) {
*/
Rule.prototype.after = function after(result, options) {
const afterChecks = findAfterChecks(this);
const ruleID = this.id;
afterChecks.forEach(check => {
const beforeResults = findCheckResults(result.nodes, check.id);
const checkOption = getCheckOption(check, ruleID, options);

const afterResults = check.after(beforeResults, checkOption.options);
const checkOption = getCheckOption(check, this.id, options);
let afterResults;
try {
afterResults = check.after(beforeResults, checkOption.options);
} catch (error) {
throw new SupportError({
ruleId: this.id,
method: `${check.id}#after`,
errorNode: result.nodes?.[0].node,
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a test that would have caught this:

Suggested change
errorNode: result.nodes?.[0].node,
errorNode: result.nodes?.[0]?.node,

error
});
}

if (this.reviewOnFail) {
afterResults.forEach(checkResult => {
Expand Down
13 changes: 0 additions & 13 deletions lib/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,3 @@ if (typeof window.getComputedStyle === 'function') {
}
// local namespace for common functions
let commons;

function SupportError(error) {
this.name = 'SupportError';
this.cause = error.cause;
this.message = `\`${error.cause}\` - feature unsupported in your environment.`;
if (error.ruleId) {
this.ruleId = error.ruleId;
this.message += ` Skipping ${this.ruleId} rule.`;
}
this.stack = new Error().stack;
}
SupportError.prototype = Object.create(Error.prototype);
SupportError.prototype.constructor = SupportError;
2 changes: 2 additions & 0 deletions lib/core/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export { default as ruleShouldRun } from './rule-should-run';
export { default as filterHtmlAttrs } from './filter-html-attrs';
export { default as select } from './select';
export { default as sendCommandToFrame } from './send-command-to-frame';
export { default as serializeError } from './serialize-error';
export { default as SupportError } from './support-error';
export { default as setScrollState } from './set-scroll-state';
export { default as shadowSelect } from './shadow-select';
export { default as shadowSelectAll } from './shadow-select-all';
Expand Down
25 changes: 25 additions & 0 deletions lib/core/utils/serialize-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Serializes an error to a JSON object
* @param e - The error to serialize
* @returns A JSON object representing the error
*/
export default function serializeError(e, iteration = 0) {
if (typeof e !== 'object' || e === null) {
return { message: String(e) };
}
const err = e; // Instead of "object"}
const serial = { ...err };
delete serial.errorNode; // We expose this separately
// Copy error.message / name / stack, these don't serialize otherwise
for (const prop of ['message', 'stack', 'name']) {
if (typeof err[prop] === 'string') {
serial[prop] = err[prop];
}
}
// Recursively serialize cause up to 10 levels deep
if (err.cause) {
serial.cause =
iteration < 10 ? serializeError(err.cause, iteration + 1) : '...';
}
return serial;
}
23 changes: 23 additions & 0 deletions lib/core/utils/support-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import serializeError from './serialize-error';

export default function SupportError({ error, ruleId, method, errorNode }) {
this.name = error.name ?? 'SupportError';
this.message = error.message;
this.stack = error.stack;
if (error.cause) {
this.cause = serializeError(error.cause);
}
if (ruleId) {
this.ruleId = ruleId;
this.message += ` Skipping ${this.ruleId} rule.`;
}
if (method) {
this.method = method;
}
if (errorNode) {
this.errorNode = errorNode;
}
}

SupportError.prototype = Object.create(Error.prototype);
SupportError.prototype.constructor = SupportError;
4 changes: 4 additions & 0 deletions locales/_template.json
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,10 @@
"pass": "Document has a non-empty <title> element",
"fail": "Document does not have a non-empty <title> element"
},
"error-occurred": {
"pass": "",
"incomplete": "Axe encountered an error; test the page for this type of problem manually"
},
"exists": {
"pass": "Element does not exist",
"incomplete": "Element exists"
Expand Down
Loading