Skip to content

Commit

Permalink
core(lcp-element): gracefully handle error in phase table (#15329)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Aug 3, 2023
1 parent 07810e2 commit da96e52
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 15 deletions.
28 changes: 13 additions & 15 deletions core/audits/largest-contentful-paint-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/

import log from 'lighthouse-logger';

import {Audit} from './audit.js';
import * as i18n from '../lib/i18n/i18n.js';
import {LargestContentfulPaint} from '../computed/metrics/largest-contentful-paint.js';
import {LCPBreakdown} from '../computed/metrics/lcp-breakdown.js';
import {Sentry} from '../lib/sentry.js';

const UIStrings = {
/** Descriptive title of a diagnostic audit that provides the element that was determined to be the Largest Contentful Paint. */
Expand Down Expand Up @@ -49,19 +52,6 @@ class LargestContentfulPaintElement extends Audit {
};
}

/**
* @param {LH.Artifacts.MetricComputationDataInput} metricComputationData
* @param {LH.Audit.Context} context
* @return {Promise<number|undefined>}
*/
static async getOptionalLCPMetric(metricComputationData, context) {
try {
const {timing: metricLcp} =
await LargestContentfulPaint.request(metricComputationData, context);
return metricLcp;
} catch {}
}

/**
* @param {LH.Artifacts} artifacts
* @return {LH.Audit.Details.Table|undefined}
Expand Down Expand Up @@ -139,11 +129,19 @@ class LargestContentfulPaintElement extends Audit {
const items = [elementTable];
let displayValue;

const metricLcp = await this.getOptionalLCPMetric(metricComputationData, context);
if (metricLcp) {
try {
const {timing: metricLcp} =
await LargestContentfulPaint.request(metricComputationData, context);
displayValue = str_(i18n.UIStrings.ms, {timeInMs: metricLcp});

const phaseTable = await this.makePhaseTable(metricLcp, metricComputationData, context);
items.push(phaseTable);
} catch (err) {
Sentry.captureException(err, {
tags: {audit: this.meta.id},
level: 'error',
});
log.error(this.meta.id, err.message);
}

const details = Audit.makeListDetails(items);
Expand Down
43 changes: 43 additions & 0 deletions core/test/audits/largest-contentful-paint-element-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,47 @@ describe('Performance: largest-contentful-paint-element audit', () => {
expect(auditResult.displayValue).toBeUndefined();
expect(auditResult.details).toBeUndefined();
});

it('doesn\'t throw an error when the phase table gets an error', async () => {
const artifacts = {
TraceElements: [{
traceEventType: 'largest-contentful-paint',
node: {
devtoolsNodePath: '1,HTML,3,BODY,5,DIV,0,HEADER',
selector: 'div.l-header > div.chorus-emc__content',
nodeLabel: 'My Test Label',
snippet: '<h1 class="test-class">',
},
type: 'text',
}],
settings: JSON.parse(JSON.stringify(defaultSettings)),
traces: {
defaultPass: createTestTrace({
traceEnd: 6000,
largestContentfulPaint: 8000,
}),
},
devtoolsLogs: {
defaultPass: [],
},
URL: {
requestedUrl,
mainDocumentUrl,
finalDisplayedUrl: mainDocumentUrl,
},
GatherContext: {gatherMode: 'navigation'},
};

const context = {settings: artifacts.settings, computedCache: new Map()};
const auditResult = await LargestContentfulPaintElementAudit.audit(artifacts, context);

expect(auditResult.score).toEqual(1);
expect(auditResult.notApplicable).toBeUndefined();
expect(auditResult.displayValue).toBeUndefined();
expect(auditResult.details.items).toHaveLength(1);
expect(auditResult.details.items[0].items).toHaveLength(1);
expect(auditResult.details.items[0].items[0].node.path).toEqual('1,HTML,3,BODY,5,DIV,0,HEADER');
expect(auditResult.details.items[0].items[0].node.nodeLabel).toEqual('My Test Label');
expect(auditResult.details.items[0].items[0].node.snippet).toEqual('<h1 class="test-class">');
});
});

0 comments on commit da96e52

Please sign in to comment.