Skip to content

Commit

Permalink
fix(audit/without-javascript): non-string error handling (#971)
Browse files Browse the repository at this point in the history
Handles the issue with a two-pronged solution.

1. Ensure if `innerText` returns something other than a string to surface the error in the gatherer
2. Ensure the type of the resulting artifact is a string before calling `.trim`

Addresses #967
  • Loading branch information
patrickhulce authored and brendankenny committed Nov 18, 2016
1 parent cb2ebfd commit 0220f26
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lighthouse-core/audits/without-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ class WithoutJavaScript extends Audit {
*/
static audit(artifacts) {
const artifact = artifacts.HTMLWithoutJavaScript;
if (artifact.value === -1) {
if (!artifact || typeof artifact.value !== 'string') {
return WithoutJavaScript.generateAuditResult({
rawValue: -1,
debugString: artifact.debugString
debugString: (artifact && artifact.debugString) ||
'HTMLWithoutJavaScript gatherer did not complete successfully'
});
}

Expand Down
4 changes: 4 additions & 0 deletions lighthouse-core/gather/gatherers/html-without-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class HTMLWithoutJavaScript extends Gatherer {

return options.driver.evaluateAsync(`(${getBodyText.toString()}())`)
.then(result => {
if (typeof result !== 'string') {
throw new Error('result was not a string');
}

this.artifact = {
value: result
};
Expand Down
12 changes: 12 additions & 0 deletions lighthouse-core/test/audits/without-javascript-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ describe('Progressive Enhancement: without javascript audit', () => {
assert.equal(result.debugString, debugString);
});

it('does not error on non-string input', () => {
const artifacts = {
HTMLWithoutJavaScript: {
value: {}
}
};

const result = withoutJsAudit.audit(artifacts);
assert.equal(result.score, -1);
assert.ok(result.debugString);
});

it('fails when the js-less body is empty', () => {
const artifacts = {
HTMLWithoutJavaScript: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,19 @@ describe('HTML without JavaScript gatherer', () => {
});
});

it('handles driver returning non-string', () => {
return htmlWithoutJavaScriptGather.afterPass({
driver: {
evaluateAsync() {
return Promise.resolve(null);
}
}
}).then(_ => {
assert.equal(htmlWithoutJavaScriptGather.artifact.value, -1);
assert.ok(htmlWithoutJavaScriptGather.artifact.debugString);
});
});

it('handles driver failure', () => {
return htmlWithoutJavaScriptGather.afterPass({
driver: {
Expand Down

0 comments on commit 0220f26

Please sign in to comment.