Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
OronW committed Jun 10, 2023
2 parents 0d07e2a + 333a667 commit 806673e
Show file tree
Hide file tree
Showing 28 changed files with 673 additions and 237 deletions.
14 changes: 14 additions & 0 deletions cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,20 @@ const expectations = {
},
},
},
'network-rtt': {
details: {
items: [
{origin: 'http://localhost:10200', rtt: '>0'},
],
},
},
'network-server-latency': {
details: {
items: [
{origin: 'http://localhost:10200', serverResponseTime: '>0'},
],
},
},
'metrics': {
// Flaky in DevTools
_excludeRunner: 'devtools',
Expand Down
7 changes: 5 additions & 2 deletions core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,14 @@ class Audit {
/**
* @param {typeof Audit} audit
* @param {string | LH.IcuMessage} errorMessage
* @param {string=} errorStack
* @return {LH.RawIcu<LH.Audit.Result>}
*/
static generateErrorAuditResult(audit, errorMessage) {
static generateErrorAuditResult(audit, errorMessage, errorStack) {
return Audit.generateAuditResult(audit, {
score: null,
errorMessage,
errorStack,
});
}

Expand All @@ -371,7 +373,7 @@ class Audit {
let scoreDisplayMode = audit.meta.scoreDisplayMode || Audit.SCORING_MODES.BINARY;

// But override if product contents require it.
if (product.errorMessage) {
if (product.errorMessage !== undefined) {
// Error result.
scoreDisplayMode = Audit.SCORING_MODES.ERROR;
} else if (product.notApplicable) {
Expand Down Expand Up @@ -407,6 +409,7 @@ class Audit {
displayValue: product.displayValue,
explanation: product.explanation,
errorMessage: product.errorMessage,
errorStack: product.errorStack,
warnings: product.warnings,

details: product.details,
Expand Down
6 changes: 6 additions & 0 deletions core/audits/network-requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Audit} from './audit.js';
import UrlUtils from '../lib/url-utils.js';
import {NetworkRecords} from '../computed/network-records.js';
import {MainResource} from '../computed/main-resource.js';
import {EntityClassification} from '../computed/entity-classification.js';

class NetworkRequests extends Audit {
/**
Expand All @@ -31,6 +32,8 @@ class NetworkRequests extends Audit {
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const records = await NetworkRecords.request(devtoolsLog, context);
const classifiedEntities = await EntityClassification.request(
{URL: artifacts.URL, devtoolsLog}, context);
const earliestRendererStartTime = records.reduce(
(min, record) => Math.min(min, record.rendererStartTime),
Infinity
Expand Down Expand Up @@ -61,6 +64,8 @@ class NetworkRequests extends Audit {
((record.frameId === mainFrameId) || undefined) :
undefined;

const entity = classifiedEntities.entityByUrl.get(record.url);

return {
url: UrlUtils.elideDataURI(record.url),
sessionTargetType: record.sessionTargetType,
Expand All @@ -77,6 +82,7 @@ class NetworkRequests extends Audit {
priority: record.priority,
isLinkPreload,
experimentalFromMainFrame,
entity: entity?.name,
lrEndTimeDeltaMs: endTimeDeltaMs, // Only exists on Lightrider runs
lrTCPMs: TCPMs, // Only exists on Lightrider runs
lrRequestMs: requestMs, // Only exists on Lightrider runs
Expand Down
18 changes: 13 additions & 5 deletions core/gather/driver/execution-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,22 @@ class ExecutionContext {

this._session.setNextProtocolTimeout(timeout);
const response = await this._session.sendCommand('Runtime.evaluate', evaluationParams);
if (response.exceptionDetails) {

const ex = response.exceptionDetails;
if (ex) {
// An error occurred before we could even create a Promise, should be *very* rare.
// Also occurs when the expression is not valid JavaScript.
const errorMessage = response.exceptionDetails.exception ?
response.exceptionDetails.exception.description :
response.exceptionDetails.text;
return Promise.reject(new Error(`Evaluation exception: ${errorMessage}`));
const elidedExpression = expression.replace(/\s+/g, ' ').substring(0, 100);
const messageLines = [
'Runtime.evaluate exception',
`Expression: ${elidedExpression}\n---- (elided)`,
!ex.stackTrace ? `Parse error at: ${ex.lineNumber + 1}:${ex.columnNumber + 1}` : null,
ex.exception?.description || ex.text,
].filter(Boolean);
const evaluationError = new Error(messageLines.join('\n'));
return Promise.reject(evaluationError);
}

// Protocol should always return a 'result' object, but it is sometimes undefined. See #6026.
if (response.result === undefined) {
return Promise.reject(
Expand Down
2 changes: 2 additions & 0 deletions core/gather/gatherers/inspector-issues.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ class InspectorIssues extends FRGatherer {
quirksModeIssue: [],
cookieIssue: [],
sharedArrayBufferIssue: [],
stylesheetLoadingIssue: [],
federatedAuthUserInfoRequestIssue: [],
};
const keys = /** @type {Array<keyof LH.Artifacts['InspectorIssues']>} */(Object.keys(artifact));
for (const key of keys) {
Expand Down
2 changes: 1 addition & 1 deletion core/legacy/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class LegacyResolvedConfig {
}

/**
* @deprecated `Config.fromJson` should be used instead.
* @deprecated `LegacyResolvedConfig.fromJson` should be used instead.
* @constructor
* @param {LH.Config} config
* @param {{settings: LH.Config.Settings, passes: ?LH.Config.Pass[], audits: ?LH.Config.AuditDefn[]}} opts
Expand Down
19 changes: 19 additions & 0 deletions core/lib/asset-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import fs from 'fs';
import path from 'path';
import stream from 'stream';
import url from 'url';

import log from 'lighthouse-logger';

Expand All @@ -16,6 +17,7 @@ import {MetricTraceEvents} from './traces/metric-trace-events.js';
import {NetworkAnalysis} from '../computed/network-analysis.js';
import {LoadSimulator} from '../computed/load-simulator.js';
import {LighthouseError} from '../lib/lh-error.js';
import {LH_ROOT} from '../../root.js';

const optionsFilename = 'options.json';
const artifactsFilename = 'artifacts.json';
Expand Down Expand Up @@ -425,6 +427,22 @@ function normalizeTimingEntries(timings) {
}
}

/**
* @param {LH.Result} lhr
*/
function elideAuditErrorStacks(lhr) {
const baseCallFrameUrl = url.pathToFileURL(LH_ROOT);
for (const auditResult of Object.values(lhr.audits)) {
if (auditResult.errorStack) {
auditResult.errorStack = auditResult.errorStack
// Make paths relative to the repo root.
.replaceAll(baseCallFrameUrl.pathname, '')
// Remove line/col info.
.replaceAll(/:\d+:\d+/g, '');
}
}
}

export {
saveArtifacts,
saveFlowArtifacts,
Expand All @@ -438,4 +456,5 @@ export {
saveLanternNetworkData,
stringifyReplacer,
normalizeTimingEntries,
elideAuditErrorStacks,
};
26 changes: 15 additions & 11 deletions core/lib/lh-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,18 @@ const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings);
const LHERROR_SENTINEL = '__LighthouseErrorSentinel';
const ERROR_SENTINEL = '__ErrorSentinel';
/**
* @typedef {{sentinel: '__LighthouseErrorSentinel', code: string, stack?: string, [p: string]: string|undefined}} SerializedLighthouseError
* @typedef {{sentinel: '__ErrorSentinel', message: string, code?: string, stack?: string}} SerializedBaseError
* @typedef {{sentinel: '__LighthouseErrorSentinel', code: string, stack?: string, cause?: unknown, properties?: {[p: string]: string|undefined}}} SerializedLighthouseError
* @typedef {{sentinel: '__ErrorSentinel', message: string, code?: string, stack?: string, cause?: unknown}} SerializedBaseError
*/

class LighthouseError extends Error {
/**
* @param {LighthouseErrorDefinition} errorDefinition
* @param {Record<string, string|undefined>=} properties
* @param {ErrorOptions=} options
*/
constructor(errorDefinition, properties) {
super(errorDefinition.code);
constructor(errorDefinition, properties, options) {
super(errorDefinition.code, options);
this.name = 'LighthouseError';
this.code = errorDefinition.code;
// Add additional properties to be ICU replacements in the error string.
Expand Down Expand Up @@ -163,26 +164,28 @@ class LighthouseError extends Error {
if (err instanceof LighthouseError) {
// Remove class props so that remaining values were what was passed in as `properties`.
// eslint-disable-next-line no-unused-vars
const {name, code, message, friendlyMessage, lhrRuntimeError, stack, ...properties} = err;
const {name, code, message, friendlyMessage, lhrRuntimeError, stack, cause, ...properties} = err;

return {
sentinel: LHERROR_SENTINEL,
code,
stack,
...properties,
cause,
properties: /** @type {{ [p: string]: string | undefined }} */ (properties),
};
}

// Unexpected errors won't be LighthouseErrors, but we want them serialized as well.
if (err instanceof Error) {
const {message, stack} = err;
const {message, stack, cause} = err;
// @ts-expect-error - code can be helpful for e.g. node errors, so preserve it if it's present.
const code = err.code;
return {
sentinel: ERROR_SENTINEL,
message,
code,
stack,
cause,
};
}

Expand All @@ -203,17 +206,18 @@ class LighthouseError extends Error {
if (possibleError.sentinel === LHERROR_SENTINEL) {
// Include sentinel in destructuring so it doesn't end up in `properties`.
// eslint-disable-next-line no-unused-vars
const {sentinel, code, stack, ...properties} = /** @type {SerializedLighthouseError} */ (possibleError);
const {code, stack, cause, properties} = /** @type {SerializedLighthouseError} */ (possibleError);
const errorDefinition = LighthouseError.errors[/** @type {keyof typeof ERRORS} */ (code)];
const lhError = new LighthouseError(errorDefinition, properties);
const lhError = new LighthouseError(errorDefinition, properties, {cause});
lhError.stack = stack;

return lhError;
}

if (possibleError.sentinel === ERROR_SENTINEL) {
const {message, code, stack} = /** @type {SerializedBaseError} */ (possibleError);
const error = new Error(message);
const {message, code, stack, cause} = /** @type {SerializedBaseError} */ (possibleError);
const opts = cause ? {cause} : undefined;
const error = new Error(message, opts);
Object.assign(error, {code, stack});
return error;
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/tracehouse/trace-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ class TraceProcessor {
return Boolean(
evt.name === 'FrameCommittedInBrowser' &&
evt.args.data?.frame &&
evt.args.data.url
evt.args.data.url !== undefined
);
}).forEach(evt => {
framesById.set(evt.args.data.frame, {
Expand Down
6 changes: 4 additions & 2 deletions core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ class Runner {

// Create a friendlier display error and mark it as expected to avoid duplicates in Sentry
const error = new LighthouseError(LighthouseError.errors.ERRORED_REQUIRED_ARTIFACT,
{artifactName, errorMessage: artifactError.message});
{artifactName, errorMessage: artifactError.message}, {cause: artifactError});
// @ts-expect-error Non-standard property added to Error
error.expected = true;
throw error;
Expand Down Expand Up @@ -468,7 +468,9 @@ class Runner {
Sentry.captureException(err, {tags: {audit: audit.meta.id}, level: 'error'});
// Errors become error audit result.
const errorMessage = err.friendlyMessage ? err.friendlyMessage : err.message;
auditResult = Audit.generateErrorAuditResult(audit, errorMessage);
// Prefer the stack trace closest to the error.
const stack = err.cause?.stack ?? err.stack;
auditResult = Audit.generateErrorAuditResult(audit, errorMessage, stack);
}

log.timeEnd(status);
Expand Down
5 changes: 5 additions & 0 deletions core/scripts/cleanup-LHR-for-diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

import {readFileSync, writeFileSync} from 'fs';

import {elideAuditErrorStacks} from '../lib/asset-saver.js';

const filename = process.argv[2];
const extraFlag = process.argv[3];
if (!filename) throw new Error('No filename provided.');
Expand Down Expand Up @@ -45,6 +47,9 @@ function cleanAndFormatLHR(lhrString) {
auditResult.description = '**Excluded from diff**';
}
}

elideAuditErrorStacks(lhr);

// Ensure we have a final newline to conform to .editorconfig
return `${JSON.stringify(lhr, null, 2)}\n`;
}
1 change: 1 addition & 0 deletions core/scripts/update-flow-fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ async function generateFlowResult() {
// Normalize some data so it doesn't change on every update.
for (const {lhr} of flowResult.steps) {
assetSaver.normalizeTimingEntries(lhr.timing.entries);
assetSaver.elideAuditErrorStacks(lhr);
lhr.timing.total = lhr.timing.entries.length;
}

Expand Down
24 changes: 24 additions & 0 deletions core/test/audits/network-requests-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,28 @@ describe('Network requests audit', () => {
isLinkPreload: true,
}]);
});

it('should include if network request was first or third party', async () => {
const records = [
{url: 'https://example.com/'},
{url: 'https://www.googletagmanager.com/gtm.js'},
];

const artifacts = {
devtoolsLogs: {
[NetworkRequests.DEFAULT_PASS]: networkRecordsToDevtoolsLog(records),
},
URL: {mainDocumentUrl: 'https://example.com/'},
GatherContext,
};
const output = await NetworkRequests.audit(artifacts, {computedCache: new Map()});

expect(output.details.items).toMatchObject([{
url: 'https://example.com/',
entity: 'example.com',
}, {
url: 'https://www.googletagmanager.com/gtm.js',
entity: 'Google Tag Manager',
}]);
});
});
Loading

0 comments on commit 806673e

Please sign in to comment.