-
Notifications
You must be signed in to change notification settings - Fork 840
feat: incomplete with node on which an error occurred #4863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 5 commits
afd906f
274ebf2
bd65841
fe61589
4665381
9014c5e
75c027b
34e827b
347fb89
b06ffb1
ec3a93c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
} | ||
}); | ||
} | ||
/** | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that I got rid of most of these, and updated how
ErrorNode in particular was a problem. Because it wasn't serialized, it A. doesn't have the frame selector, and B. leaves the |
||
}); | ||
// 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 | ||
*/ | ||
|
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 { | ||||||
|
@@ -8,7 +7,8 @@ import { | |||||
queue, | ||||||
DqElement, | ||||||
select, | ||||||
assert | ||||||
assert, | ||||||
SupportError | ||||||
} from '../utils'; | ||||||
import { isVisibleToScreenReaders } from '../../commons/dom'; | ||||||
import constants from '../constants'; | ||||||
|
@@ -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 | ||||||
}) | ||||||
); | ||||||
}); | ||||||
}); | ||||||
}); | ||||||
|
||||||
|
@@ -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; | ||||||
} | ||||||
|
||||||
|
@@ -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); | ||||||
} | ||||||
|
@@ -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), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: new DqElement can be passed a virtualNode directly
Suggested change
|
||||||
error: error | ||||||
}); | ||||||
} | ||||||
}); | ||||||
|
||||||
if (options.performanceTimer) { | ||||||
performanceTimer.mark(markMatchesEnd); | ||||||
|
@@ -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, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be a test that would have caught this:
Suggested change
|
||||||
error | ||||||
}); | ||||||
} | ||||||
|
||||||
if (this.reviewOnFail) { | ||||||
afterResults.forEach(checkResult => { | ||||||
|
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; | ||
} |
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 }) { | ||
straker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.