Skip to content

Commit

Permalink
feat: add error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce committed Jun 1, 2017
1 parent 400457b commit 002650b
Show file tree
Hide file tree
Showing 18 changed files with 369 additions and 30 deletions.
15 changes: 14 additions & 1 deletion lighthouse-cli/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import * as path from 'path';
const perfOnlyConfig = require('../lighthouse-core/config/perf.json');
const performanceXServer = require('./performance-experiment/server');
import * as Printer from './printer';
import {askPermission, isDev} from './sentry-prompt';
import {Results} from './types/types';
const pkg = require('../package.json');

Expand Down Expand Up @@ -175,10 +176,22 @@ export async function runLighthouse(
url: string, flags: Flags, config: Object|null): Promise<{}|void> {
let launchedChrome: LaunchedChrome|undefined;

if (typeof flags.disableErrorReporting === 'undefined') {
const isErrorReportingEnabled = await askPermission();
flags.disableErrorReporting = !isErrorReportingEnabled;
}

try {
launchedChrome = await getDebuggableChrome(flags);
flags.port = launchedChrome.port;
const results = await lighthouse(url, flags, config);
const results = await lighthouse(url, flags, config, {
environment: isDev() ? 'development' : 'production',
serverName: 'redacted',
release: pkg.version,
tags: {
channel: 'cli',
}
});

const artifacts = results.artifacts;
delete results.artifacts;
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-cli/cli-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {GetValidOutputOptions, OutputMode} from './printer';
export interface Flags {
skipAutolaunch: boolean, port: number, selectChrome: boolean, chromeFlags: string, output: any,
outputPath: string, interactive: boolean, saveArtifacts: boolean, saveAssets: boolean,
view: boolean, maxWaitForLoad: number
view: boolean, maxWaitForLoad: number, disableErrorReporting?: boolean
}

export function getFlags() {
Expand Down Expand Up @@ -67,6 +67,7 @@ export function getFlags() {
],
'Configuration:')
.describe({
'disable-error-logging': 'Disable error logging',
'disable-storage-reset':
'Disable clearing the browser cache and other storage APIs before a run',
'disable-device-emulation': 'Disable Nexus 5X emulation',
Expand Down
59 changes: 59 additions & 0 deletions lighthouse-cli/sentry-prompt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { existsSync } from 'fs';
import { join as joinPath } from 'path';
import { inquirer, Configstore } from './shim-modules';
const log = require('../lighthouse-core/lib/log');

const MAXIMUM_WAIT_TIME = 20 * 1000;

const MESSAGE = [
'Lighthouse would like to report back any errors that might occur while auditing. \n ',
'Information such as the URL you are auditing, its subresources, your operating system, Chrome, ',
'and Lighthouse versions may be recorded. Would you be willing to have Lighthouse automatically ',
'report this information to the team to aid in improving the product?'
].join('');

async function prompt() {
if (!process.stdout.isTTY || process.env.CI) {
// Default non-interactive sessions to false
return false;
}

let timeout: NodeJS.Timer;

const prompt = inquirer.prompt([
{ type: 'confirm', name: 'isErrorReportingEnabled', default: false, message: MESSAGE },
]);

const timeoutPromise = new Promise((resolve: (a: boolean) => {}) => {
timeout = setTimeout(() => {
prompt.ui.close();
process.stdout.write('\n');
log.warn('CLI', 'No response to error logging preference, errors will not be reported.');
resolve(false);
}, MAXIMUM_WAIT_TIME);
});

return Promise.race([
prompt.then((result: { isErrorReportingEnabled: boolean}) => {
clearTimeout(timeout);
return result.isErrorReportingEnabled;
}),
timeoutPromise,
]);
}

export async function askPermission() {
const configstore = new Configstore('lighthouse');
let isErrorReportingEnabled = configstore.get('isErrorReportingEnabled');
if (typeof isErrorReportingEnabled === 'boolean') {
return isErrorReportingEnabled;
}

isErrorReportingEnabled = await prompt();
configstore.set('isErrorReportingEnabled', isErrorReportingEnabled);
return isErrorReportingEnabled;
}

export function isDev() {
return existsSync(joinPath(__dirname, '../.git'));
}
25 changes: 24 additions & 1 deletion lighthouse-cli/shim-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,27 @@ try {
};
}

export {opn, updateNotifier};
let inquirer;
try {
inquirer = require('inquirer');
} catch (e) {
inquirer = {
prompt() {
console.error('module `inquirer` not installed. Not logging errors.');
return Promise.resolve(false);
}
}
}

let Configstore;
try {
Configstore = require('configstore');
} catch (e) {
Configstore = function Configstore() {
console.error('module `configstore` not installed. Not persisting user choices.');
this.get = () => false;
this.set = () => undefined;
}
}

export { opn, updateNotifier, inquirer, Configstore };
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

const Audit = require('../audit');
const Formatter = require('../../report/formatter');
const Sentry = require('../../lib/sentry');

const KB_IN_BYTES = 1024;

Expand Down Expand Up @@ -116,6 +117,14 @@ class UnusedBytes extends Audit {
const v1TableHeadings = Audit.makeV1TableHeadings(result.headings);
const v2TableDetails = Audit.makeV2TableDetails(result.headings, results);

if (debugString) {
// Use captureException to preserve the stack and take advantage of Sentry grouping
Sentry.captureException(new Error(debugString), {
tags: {audit: this.meta.name},
level: 'warning',
});
}

return {
debugString,
displayValue,
Expand Down
12 changes: 10 additions & 2 deletions lighthouse-core/audits/load-fast-enough-for-pwa.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const Audit = require('./audit');
const URL = require('../lib/url-shim');
const Emulation = require('../lib/emulation');
const Formatter = require('../report/formatter');
const Sentry = require('../lib/sentry');

// Maximum TTFI to be considered "fast" for PWA baseline checklist
// https://developers.google.com/web/progressive-web-apps/checklist
Expand Down Expand Up @@ -122,10 +123,17 @@ class LoadFastEnough4Pwa extends Audit {
}

if (!areLatenciesAll3G) {
// eslint-disable-next-line max-len
const debugString = `First Interactive was found at ${timeToFirstInteractive.toLocaleString()}, however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`;

Sentry.captureMessage('Network request latencies were not realistic', {
tags: {audit: this.meta.name},
level: 'warning',
});

return {
rawValue: true,
// eslint-disable-next-line max-len
debugString: `First Interactive was found at ${timeToFirstInteractive.toLocaleString()}, however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`,
debugString,
extendedInfo,
details
};
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const emulation = require('../lib/emulation');
const Element = require('../lib/element');
const EventEmitter = require('events').EventEmitter;
const URL = require('../lib/url-shim');
const ExpectedError = require('../lib/expected-error');

const log = require('../lib/log.js');
const DevtoolsLog = require('./devtools-log');
Expand Down Expand Up @@ -287,7 +288,7 @@ class Driver {
}

// If both the data and the url are empty strings, the page had no manifest.
return reject('No web app manifest found.');
return reject(ExpectedError(new Error('No web app manifest found.')));
}

resolve(response);
Expand Down
8 changes: 6 additions & 2 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const log = require('../lib/log.js');
const Audit = require('../audits/audit');
const URL = require('../lib/url-shim');
const NetworkRecorder = require('../lib/network-recorder.js');
const Sentry = require('../lib/sentry');

/**
* @typedef {!Object<string, !Array<!Promise<*>>>}
Expand Down Expand Up @@ -132,12 +133,15 @@ class GatherRunner {
* Test any error output from the promise, absorbing non-fatal errors and
* throwing on fatal ones so that run is stopped.
* @param {!Promise<*>} promise
* @param {string=} gathererName
* @return {!Promise<*>}
*/
static recoverOrThrow(promise) {
static recoverOrThrow(promise, gathererName) {
return promise.catch(err => {
if (err.fatal) {
throw err;
} else {
Sentry.captureException(err, {tags: {gatherer: gathererName}, level: 'warning'});
}
});
}
Expand Down Expand Up @@ -184,7 +188,7 @@ class GatherRunner {
return chain.then(_ => {
const artifactPromise = Promise.resolve().then(_ => gatherer.beforePass(options));
gathererResults[gatherer.name] = [artifactPromise];
return GatherRunner.recoverOrThrow(artifactPromise);
return GatherRunner.recoverOrThrow(artifactPromise, gatherer.name);
});
}, pass);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

const Gatherer = require('../gatherer');
const URL = require('../../../lib/url-shim');
const Sentry = require('../../../lib/sentry');

/* global document, Image, atob */

Expand Down Expand Up @@ -137,7 +138,14 @@ class OptimizedImages extends Gatherer {
return imageRecords.reduce((promise, record) => {
return promise.then(results => {
return this.calculateImageStats(driver, record)
.catch(err => ({failed: true, err}))
.catch(err => {
Sentry.captureException(err, {
tags: {gatherer: 'optimized-images'},
extra: {imageUrl: URL.elideDataURI(record.url)},
level: 'warning',
});
return {failed: true, err};
})
.then(stats => {
if (!stats) {
return results;
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gatherers/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Manifest extends Gatherer {
return manifestParser(response.data, response.url, options.url);
})
.catch(err => {
if (err === 'No web app manifest found.') {
if (err.message === 'No web app manifest found.') {
return null;
}

Expand Down
6 changes: 4 additions & 2 deletions lighthouse-core/gather/gatherers/start-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

const Gatherer = require('./gatherer');
const URL = require('../../lib/url-shim');
const ExpectedError = require('../../lib/expected-error');
const manifestParser = require('../../lib/manifest-parser');

class StartUrl extends Gatherer {
Expand Down Expand Up @@ -64,7 +65,8 @@ class StartUrl extends Gatherer {
})
.then(manifest => {
if (!manifest.value.start_url || !manifest.value.start_url.raw) {
return Promise.reject(new Error(`No web app manifest found on page ${options.url}`));
const error = new Error(`No web app manifest found on page ${options.url}`);
return Promise.reject(ExpectedError(error));
}

if (manifest.value.start_url.debugString) {
Expand All @@ -77,7 +79,7 @@ class StartUrl extends Gatherer {

afterPass(options, tracingData) {
if (!this.startUrl) {
return Promise.reject(new Error('No start_url found inside the manifest'));
return Promise.reject(ExpectedError(new Error('No start_url found inside the manifest')));
}

const networkRecords = tracingData.networkRecords;
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const Config = require('./config/config');
*
*/

module.exports = function(url, flags = {}, configJSON) {
module.exports = function(url, flags = {}, configJSON, environmentData) {
const startTime = Date.now();
return Promise.resolve().then(_ => {
if (!url) {
Expand All @@ -54,7 +54,7 @@ module.exports = function(url, flags = {}, configJSON) {
const connection = new ChromeProtocol(flags.port);

// kick off a lighthouse run
return Runner.run(connection, {url, flags, config})
return Runner.run(connection, {url, flags, config, environmentData})
.then(lighthouseResults => {
// Annotate with time to run lighthouse.
const endTime = Date.now();
Expand Down
8 changes: 8 additions & 0 deletions lighthouse-core/lib/expected-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = function ExpectedError(error) {
if (!(error instanceof Error)) {
error = new Error(error);
}

error.expected = true;
return error;
};
54 changes: 54 additions & 0 deletions lighthouse-core/lib/sentry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const log = require('./log');

// Fix the polyfill. See https://github.com/GoogleChrome/lighthouse/issues/73
self.setImmediate = function (callback) {
const args = [...arguments].slice(1);
Promise.resolve().then(() => callback(...args));
return 0;
};

const noop = () => undefined;
const sentryDelegate = module.exports = {
config() {
return {install: noop};
},
context(data, functionToWrap) {
if (typeof functionToWrap === 'undefined') {
functionToWrap = data;
}

functionToWrap();
},
captureMessage: noop,
captureException: noop,
captureBreadcrumb: noop,
setContext: noop,
mergeContext: noop,
init(useSentry, config) {
if (!useSentry) {
return;
}

config = Object.assign({}, config, {
allowSecretKey: true,
});

try {
const Sentry = require('raven');
Sentry.config('https://a6bb0da87ee048cc9ae2a345fc09ab2e:63a7029f46f74265981b7e005e0f69f8@sentry.io/174697', config).install();
Object.keys(sentryDelegate).forEach(functionName => {
if (functionName === 'init') {
return;
}

sentryDelegate[functionName] = (...args) => Sentry[functionName](...args);
sentryDelegate.captureException = (...args) => {
if (args[0] && args[0].expected) return;
Sentry.captureException(...args);
}
});
} catch (e) {
log.warn('sentry', 'Could not load raven library, errors will not be reported.');
}
}
}
Loading

0 comments on commit 002650b

Please sign in to comment.