Skip to content

Commit

Permalink
fix: evaluateAsync behavior
Browse files Browse the repository at this point in the history
addresses GoogleChrome#1000 and GoogleChrome#976

1. Ensures all `Runtime.evaluate` calls result in the native Promise
2. Transforms all errors produced during the evaluation to a standard object that can be rejected by our driver wrapper
  • Loading branch information
patrickhulce committed Nov 23, 2016
1 parent dcdabb7 commit 88bfa18
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 8 deletions.
2 changes: 2 additions & 0 deletions lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
Original file line number Diff line number Diff line change
Expand Up @@ -280,5 +280,7 @@ <h2>Do better web tester page</h2>
}
</script>

<!-- Import zone.js to test Promise polyfill -->
<script src="https://unpkg.com/zone.js?main=browser"></script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ class LinkBlockingFirstPaintAudit extends Audit {
if (typeof artifact === 'undefined' || artifact.value === -1) {
return {
rawValue: -1,
debugString: 'TagsBlockingFirstPaint gatherer did not run'
debugString: (artifact && artifact.debugString) ||
'TagsBlockingFirstPaint gatherer did not run'
};
}

Expand Down
38 changes: 35 additions & 3 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,49 @@ class Driver {
(_ => reject(new Error('The asynchronous expression exceeded the allotted time of 60s'))),
60000
);

// The `exceptionDetails` provided by the debugger protocol does not contain the useful
// information such as name, message, and stack trace of the error when it's wrapped in a
// promise. Instead, map to a successful object that contains this information.
/* istanbul ignore next */
const errorWrapper = err => {
err = err || new Error();
const fallbackMessage = typeof err === 'string' ? err : 'unknown error';

resolve({
__failedInBrowser: true,
name: err.name || 'Error',
message: err.message || fallbackMessage,
stack: err.stack || (new Error()).stack,
});
};

this.sendCommand('Runtime.evaluate', {
expression: asyncExpression,
expression: `(function wrapInNativePromise () {
var __nativePromise = window.__nativePromise || Promise;
return new __nativePromise(function (resolve) {
var wrapError = ${errorWrapper.toString()};
try {
__nativePromise.resolve(${asyncExpression}).then(resolve, wrapError);
} catch (e) {
wrapError(e);
}
});
}())`,
includeCommandLineAPI: true,
awaitPromise: true,
returnByValue: true
}).then(result => {
clearTimeout(asyncTimeout);
const value = result.result.value;

if (result.exceptionDetails) {
reject(result.exceptionDetails.exception.value);
// An error occurred before we could even enter our try block, should be *very* rare
reject(new Error('an unknown driver error occurred'));
} if (value.__failedInBrowser) {
reject(Object.assign(new Error(), value));
} else {
resolve(result.result.value);
resolve(value);
}
}).catch(err => {
clearTimeout(asyncTimeout);
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class GatherRunner {
return driver.assertNoSameOriginServiceWorkerClients(options.url)
.then(_ => driver.beginEmulation(options.flags))
.then(_ => driver.enableRuntimeEvents())
.then(_ => driver.evaluateScriptOnLoad('window.__nativePromise = Promise;'))
.then(_ => driver.cleanAndDisableBrowserCaches())
.then(_ => driver.clearDataForOrigin(options.url));
}
Expand Down
6 changes: 5 additions & 1 deletion lighthouse-core/gather/gatherers/accessibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,13 @@ class Accessibility extends Gatherer {

afterPass(options) {
const driver = options.driver;
const expression = `(function () {
${axe};
return (${runA11yChecks.toString()}());
})()`;

return driver
.evaluateAsync(`${axe};(${runA11yChecks.toString()}())`)
.evaluateAsync(expression)
.then(returnedValue => {
if (!returnedValue) {
this.artifact = Accessibility._errorAccessibility('Unable to parse axe results');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ function collectTagsThatBlockFirstPaint() {
});
resolve(tagList);
} catch (e) {
reject(`Unable to gather Scripts/Stylesheets/HTML Imports on the page: ${e.message}`);
const friendly = 'Unable to gather Scripts/Stylesheets/HTML Imports on the page';
reject(new Error(`${friendly}: ${e.message}`));
}
});
}
Expand Down Expand Up @@ -117,10 +118,10 @@ class TagsBlockingFirstPaint extends Gatherer {
.then(artifact => {
this.artifact = artifact;
})
.catch(debugString => {
.catch(err => {
this.artifact = {
value: -1,
debugString
debugString: err.toString()
};
});
}
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/test/gather/fake-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ module.exports = {
enableRuntimeEvents() {
return Promise.resolve();
},
evaluateScriptOnLoad() {
return Promise.resolve();
},
cleanAndDisableBrowserCaches() {},
clearDataForOrigin() {},
beginTrace() {
Expand Down

0 comments on commit 88bfa18

Please sign in to comment.