Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(js-bundles): return error object when sizes cannot be determined #10449

Merged
merged 8 commits into from
Oct 6, 2020
13 changes: 8 additions & 5 deletions lighthouse-core/audits/byte-efficiency/unused-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,20 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
};

if (item.wastedBytes <= unusedThreshold) continue;
items.push(item);

// If there was an error calculating the bundle sizes, we can't
// create any sub-items.
if (!bundle || 'errorMessage' in bundle.sizes) continue;
const sizes = bundle.sizes;

// Augment with bundle data.
if (bundle && unusedJsSummary.sourcesWastedBytes) {
if (unusedJsSummary.sourcesWastedBytes) {
const topUnusedSourceSizes = Object.entries(unusedJsSummary.sourcesWastedBytes)
.sort((a, b) => b[1] - a[1])
.slice(0, 5)
.map(([source, unused]) => {
const total =
source === '(unmapped)' ? bundle.sizes.unmappedBytes : bundle.sizes.files[source];
const total = source === '(unmapped)' ? sizes.unmappedBytes : sizes.files[source];
return {
source,
unused: Math.round(unused * transferRatio),
Expand All @@ -133,8 +138,6 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
}),
};
}

items.push(item);
}

return {
Expand Down
24 changes: 12 additions & 12 deletions lighthouse-core/computed/js-bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ function computeGeneratedFileSizes(map, content) {
const totalBytes = content.length;
let unmappedBytes = totalBytes;

// If the map + contents don't line up, return a result that
// denotes nothing is mapped.
const failureResult = {files: {}, unmappedBytes, totalBytes};

// @ts-expect-error: This function is added in SDK.js. This will eventually be added to CDT.
map.computeLastGeneratedColumns();

Expand All @@ -43,23 +39,27 @@ function computeGeneratedFileSizes(map, content) {
// Lines and columns are zero-based indices. Visually, lines are shown as a 1-based index.

const line = lines[lineNum];
if (line === null) {
log.error('JSBundles', `${map.url()} mapping for line out of bounds: ${lineNum + 1}`);
return failureResult;
if (line === null || line === undefined) {
const errorMessage = `${map.url()} mapping for line out of bounds: ${lineNum + 1}`;
log.error('JSBundles', errorMessage);
return {errorMessage};
}

if (colNum > line.length) {
// eslint-disable-next-line max-len
log.error('JSBundles', `${map.url()} mapping for column out of bounds: ${lineNum + 1}:${colNum}`);
return failureResult;
const errorMessage =
`${map.url()} mapping for column out of bounds: ${lineNum + 1}:${colNum}`;
log.error('JSBundles', errorMessage);
return {errorMessage};
}

let mappingLength = 0;
if (lastColNum !== undefined) {
if (lastColNum > line.length) {
// eslint-disable-next-line max-len
log.error('JSBundles', `${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`);
return failureResult;
const errorMessage =
`${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`;
log.error('JSBundles', errorMessage);
return {errorMessage};
}
mappingLength = lastColNum - colNum;
} else {
Expand Down
2 changes: 2 additions & 0 deletions lighthouse-core/computed/module-duplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class ModuleDuplication {

// Determine size of each `sources` entry.
for (const {rawMap, sizes} of bundles) {
if ('errorMessage' in sizes) continue;

/** @type {SourceData[]} */
const sourceDataArray = [];
sourceDatasMap.set(rawMap, sourceDataArray);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ async function main() {
SourceMaps: [{scriptUrl: '', map: bundleMap}],
};
const bundles = await JsBundles.compute_(artifacts);
if ('errorMessage' in bundles[0].sizes) throw new Error(bundles[0].sizes.errorMessage);
const bundleFileSizes = bundles[0].sizes.files;

const allModules = Object.keys(bundleFileSizes).filter(s => s.startsWith('node_modules'));
Expand Down
49 changes: 43 additions & 6 deletions lighthouse-core/test/computed/js-bundles-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,7 @@ describe('JsBundles computed artifact', () => {
"sourceURL": "src/foo.js",
},
"sizes": Object {
"files": Object {},
"totalBytes": 13,
"unmappedBytes": 13,
"errorMessage": "compiled.js.map mapping for last column out of bounds: 1:14",
},
}
`);
Expand All @@ -314,9 +312,7 @@ describe('JsBundles computed artifact', () => {
"sourceURL": "src/foo.js",
},
"sizes": Object {
"files": Object {},
"totalBytes": 0,
"unmappedBytes": 0,
"errorMessage": "compiled.js.map mapping for column out of bounds: 1:1",
},
}
`);
Expand Down Expand Up @@ -347,5 +343,46 @@ describe('JsBundles computed artifact', () => {
}
`);
});

it('emits error when column out of bounds', async () => {
const newMappings = map.mappings.split(',');
expect(newMappings[1]).toBe('SAAAA');
// Make the column offset very big, force out of bounds.
// See https://www.mattzeunert.com/2016/02/14/how-do-source-maps-work.html
newMappings[1] = 'kD' + 'AAAA';
map.mappings = newMappings.join(',');
expect(await test()).toMatchInlineSnapshot(`
Object {
"entry": SourceMapEntry {
"columnNumber": 642,
"lastColumnNumber": 651,
"lineNumber": 0,
"name": undefined,
"sourceColumnNumber": 18,
"sourceLineNumber": 1,
"sourceURL": "src/foo.js",
},
"sizes": Object {
"errorMessage": "compiled.js.map mapping for last column out of bounds: 1:685",
},
}
`);
});

it('emits error when line out of bounds', async () => {
const newMappings = map.mappings.split(',');
expect(newMappings[1]).toBe('SAAAA');
// Make the line offset very big, force out of bounds.
// See https://sourcemaps.info/spec.html#:~:text=broken%20down%20as%20follows
map.mappings = ';'.repeat(10) + map.mappings;
expect(await test()).toMatchInlineSnapshot(`
Object {
"entry": null,
"sizes": Object {
"errorMessage": "compiled.js.map mapping for line out of bounds: 11",
},
}
`);
});
});
});
2 changes: 1 addition & 1 deletion types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ declare global {
files: Record<string, number>;
unmappedBytes: number;
totalBytes: number;
};
} | {errorMessage: string};
}

/** @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#Attributes */
Expand Down