From b3c5896623a85af2960802fc4fd624f2e25ecf07 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Mar 2020 14:08:15 -0700 Subject: [PATCH 1/6] core(js-bundles): return error object when sizes cannot be determined --- .../byte-efficiency/unused-javascript.js | 4 +++- lighthouse-core/computed/js-bundles.js | 20 +++++++++---------- .../computed/module-duplication.js | 2 ++ .../test/computed/js-bundles-test.js | 8 ++------ types/artifacts.d.ts | 2 +- 5 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/unused-javascript.js b/lighthouse-core/audits/byte-efficiency/unused-javascript.js index 1ff94e71eaae..073ba476aef8 100644 --- a/lighthouse-core/audits/byte-efficiency/unused-javascript.js +++ b/lighthouse-core/audits/byte-efficiency/unused-javascript.js @@ -115,6 +115,8 @@ class UnusedJavaScript extends ByteEfficiencyAudit { */ static createBundleMultiData(item, wasteData, bundle, lengths, bundleSourceUnusedThreshold) { if (!bundle.script.content) return; + const sizes = bundle.sizes; + if ('error' in sizes) return; /** @type {Record} */ const files = {}; @@ -151,7 +153,7 @@ class UnusedJavaScript extends ByteEfficiencyAudit { .sort(([_, unusedBytes1], [__, unusedBytes2]) => unusedBytes2 - unusedBytes1) .slice(0, 5) .map(([key, unusedBytes]) => { - const total = key === '(unmapped)' ? bundle.sizes.unmappedBytes : bundle.sizes.files[key]; + const total = key === '(unmapped)' ? sizes.unmappedBytes : sizes.files[key]; return { key, unused: Math.round(unusedBytes * transferRatio), diff --git a/lighthouse-core/computed/js-bundles.js b/lighthouse-core/computed/js-bundles.js index 6ee004a2376d..c839eeeb072a 100644 --- a/lighthouse-core/computed/js-bundles.js +++ b/lighthouse-core/computed/js-bundles.js @@ -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-ignore: This function is added in SDK.js. This will eventually be added to CDT. map.computeLastGeneratedColumns(); @@ -44,22 +40,24 @@ function computeGeneratedFileSizes(map, content) { const line = lines[lineNum]; if (line === null) { - log.error('JSBundles', `${map.url()} mapping for line out of bounds: ${lineNum + 1}`); - return failureResult; + const error = `${map.url()} mapping for line out of bounds: ${lineNum + 1}`; + log.error('JSBundles', error); + return {error}; } 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 error = `${map.url()} mapping for column out of bounds: ${lineNum + 1}:${colNum}`; + log.error('JSBundles', error); + return {error}; } 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 error = `${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`; + log.error('JSBundles', error); + return {error}; } mappingLength = lastColNum - colNum; } else { diff --git a/lighthouse-core/computed/module-duplication.js b/lighthouse-core/computed/module-duplication.js index 69bc2d949273..bdbc001d0abb 100644 --- a/lighthouse-core/computed/module-duplication.js +++ b/lighthouse-core/computed/module-duplication.js @@ -57,6 +57,8 @@ class ModuleDuplication { // Determine size of each `sources` entry. for (const {rawMap, sizes} of bundles) { + if ('error' in sizes) continue; + /** @type {SourceData[]} */ const sourceDataArray = []; sourceDatasMap.set(rawMap, sourceDataArray); diff --git a/lighthouse-core/test/computed/js-bundles-test.js b/lighthouse-core/test/computed/js-bundles-test.js index 702c58f65575..a71d28b9d6fc 100644 --- a/lighthouse-core/test/computed/js-bundles-test.js +++ b/lighthouse-core/test/computed/js-bundles-test.js @@ -292,9 +292,7 @@ describe('JSBundles computed artifact', () => { "sourceURL": "src/foo.js", }, "sizes": Object { - "files": Object {}, - "totalBytes": 13, - "unmappedBytes": 13, + "error": "compiled.js.map mapping for last column out of bounds: 1:14", }, } `); @@ -314,9 +312,7 @@ describe('JSBundles computed artifact', () => { "sourceURL": "src/foo.js", }, "sizes": Object { - "files": Object {}, - "totalBytes": 0, - "unmappedBytes": 0, + "error": "compiled.js.map mapping for column out of bounds: 1:1", }, } `); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index a5ca7517f793..a0ca1c14abb8 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -307,7 +307,7 @@ declare global { files: Record; unmappedBytes: number; totalBytes: number; - }; + } | {error: string}; } /** @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#Attributes */ From d517ec5aad15b6ed8115da340b75be7cff8b5f0e Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 28 Sep 2020 15:17:43 -0500 Subject: [PATCH 2/6] add test for coverage --- .../test/computed/js-bundles-test.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/lighthouse-core/test/computed/js-bundles-test.js b/lighthouse-core/test/computed/js-bundles-test.js index 4a4019c77a7b..53ed74377dc4 100644 --- a/lighthouse-core/test/computed/js-bundles-test.js +++ b/lighthouse-core/test/computed/js-bundles-test.js @@ -343,5 +343,30 @@ describe('JsBundles computed artifact', () => { } `); }); + + it('5', async () => { + const newMappings = map.mappings.split(','); + expect(newMappings[1]).toBe('SAAAA'); + // Make the line offset very big, force out of bounds. + // See https://www.mattzeunert.com/2016/02/14/how-do-source-maps-work.html + newMappings[1] = 'kDAAAA'; + 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 { + "error": "compiled.js.map mapping for last column out of bounds: 1:685", + }, + } + `); + }); }); }); From 5014757e223a8e6a18a5cae36137986b1ad9894d Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 1 Oct 2020 16:20:07 -0500 Subject: [PATCH 3/6] Update lighthouse-core/audits/byte-efficiency/unused-javascript.js Co-authored-by: Brendan Kenny --- lighthouse-core/audits/byte-efficiency/unused-javascript.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/audits/byte-efficiency/unused-javascript.js b/lighthouse-core/audits/byte-efficiency/unused-javascript.js index 8c09a538f7d0..5b3832b2a2ae 100644 --- a/lighthouse-core/audits/byte-efficiency/unused-javascript.js +++ b/lighthouse-core/audits/byte-efficiency/unused-javascript.js @@ -106,7 +106,7 @@ class UnusedJavaScript extends ByteEfficiencyAudit { if (item.wastedBytes <= unusedThreshold) continue; items.push(item); - // If there was an error calculated the bundle sizes, we can't + // If there was an error calculating the bundle sizes, we can't // create any sub-items. if (!bundle || 'error' in bundle.sizes) continue; const sizes = bundle.sizes; From f4bfbda98861c5dce73cf13c1a573594bbef4812 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 1 Oct 2020 17:14:58 -0500 Subject: [PATCH 4/6] error message --- lighthouse-core/computed/js-bundles.js | 22 ++++++++++--------- .../create-polyfill-size-estimation.js | 2 +- .../test/computed/js-bundles-test.js | 22 ++++++++++++++++--- types/artifacts.d.ts | 2 +- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/computed/js-bundles.js b/lighthouse-core/computed/js-bundles.js index b97a7cbaf931..9f836fb1a13f 100644 --- a/lighthouse-core/computed/js-bundles.js +++ b/lighthouse-core/computed/js-bundles.js @@ -39,25 +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) { - const error = `${map.url()} mapping for line out of bounds: ${lineNum + 1}`; - log.error('JSBundles', error); - return {error}; + 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) { - const error = `${map.url()} mapping for column out of bounds: ${lineNum + 1}:${colNum}`; - log.error('JSBundles', error); - return {error}; + 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 - const error = `${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`; - log.error('JSBundles', error); - return {error}; + const errorMessage = + `${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`; + log.error('JSBundles', errorMessage); + return {errorMessage}; } mappingLength = lastColNum - colNum; } else { diff --git a/lighthouse-core/scripts/legacy-javascript/create-polyfill-size-estimation.js b/lighthouse-core/scripts/legacy-javascript/create-polyfill-size-estimation.js index d60a796b732d..cfea478e1185 100644 --- a/lighthouse-core/scripts/legacy-javascript/create-polyfill-size-estimation.js +++ b/lighthouse-core/scripts/legacy-javascript/create-polyfill-size-estimation.js @@ -88,7 +88,7 @@ async function main() { SourceMaps: [{scriptUrl: '', map: bundleMap}], }; const bundles = await JsBundles.compute_(artifacts); - if ('error' in bundles[0].sizes) throw new Error(bundles[0].sizes.error); + 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')); diff --git a/lighthouse-core/test/computed/js-bundles-test.js b/lighthouse-core/test/computed/js-bundles-test.js index 53ed74377dc4..5aeafbdc0e67 100644 --- a/lighthouse-core/test/computed/js-bundles-test.js +++ b/lighthouse-core/test/computed/js-bundles-test.js @@ -344,12 +344,12 @@ describe('JsBundles computed artifact', () => { `); }); - it('5', async () => { + it('emits error when column out of bounds', async () => { const newMappings = map.mappings.split(','); expect(newMappings[1]).toBe('SAAAA'); - // Make the line offset very big, force out of bounds. + // 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] = 'kDAAAA'; + newMappings[1] = 'kD' + 'AAAA'; map.mappings = newMappings.join(','); expect(await test()).toMatchInlineSnapshot(` Object { @@ -368,5 +368,21 @@ describe('JsBundles computed artifact', () => { } `); }); + + 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 { + "error": "compiled.js.map mapping for line out of bounds: 11", + }, + } + `); + }); }); }); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 936da14ee3c3..ee59a7a299b6 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -326,7 +326,7 @@ declare global { files: Record; unmappedBytes: number; totalBytes: number; - } | {error: string}; + } | {errorMessage: string}; } /** @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#Attributes */ From a7c98acf52c6d74657985d4cb334e5f332a92610 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 1 Oct 2020 17:47:42 -0500 Subject: [PATCH 5/6] tsc --- lighthouse-core/audits/byte-efficiency/unused-javascript.js | 2 +- lighthouse-core/computed/module-duplication.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/unused-javascript.js b/lighthouse-core/audits/byte-efficiency/unused-javascript.js index 5b3832b2a2ae..0b031864cd63 100644 --- a/lighthouse-core/audits/byte-efficiency/unused-javascript.js +++ b/lighthouse-core/audits/byte-efficiency/unused-javascript.js @@ -108,7 +108,7 @@ class UnusedJavaScript extends ByteEfficiencyAudit { // If there was an error calculating the bundle sizes, we can't // create any sub-items. - if (!bundle || 'error' in bundle.sizes) continue; + if (!bundle || 'errorMessage' in bundle.sizes) continue; const sizes = bundle.sizes; // Augment with bundle data. diff --git a/lighthouse-core/computed/module-duplication.js b/lighthouse-core/computed/module-duplication.js index 6222293eb312..73b974868893 100644 --- a/lighthouse-core/computed/module-duplication.js +++ b/lighthouse-core/computed/module-duplication.js @@ -91,7 +91,7 @@ class ModuleDuplication { // Determine size of each `sources` entry. for (const {rawMap, sizes} of bundles) { - if ('error' in sizes) continue; + if ('errorMessage' in sizes) continue; /** @type {SourceData[]} */ const sourceDataArray = []; From 6732fb3907b81dcc32092169835708e08a3df795 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 2 Oct 2020 15:31:07 -0500 Subject: [PATCH 6/6] update --- lighthouse-core/test/computed/js-bundles-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/test/computed/js-bundles-test.js b/lighthouse-core/test/computed/js-bundles-test.js index 5aeafbdc0e67..7a68b0f48907 100644 --- a/lighthouse-core/test/computed/js-bundles-test.js +++ b/lighthouse-core/test/computed/js-bundles-test.js @@ -292,7 +292,7 @@ describe('JsBundles computed artifact', () => { "sourceURL": "src/foo.js", }, "sizes": Object { - "error": "compiled.js.map mapping for last column out of bounds: 1:14", + "errorMessage": "compiled.js.map mapping for last column out of bounds: 1:14", }, } `); @@ -312,7 +312,7 @@ describe('JsBundles computed artifact', () => { "sourceURL": "src/foo.js", }, "sizes": Object { - "error": "compiled.js.map mapping for column out of bounds: 1:1", + "errorMessage": "compiled.js.map mapping for column out of bounds: 1:1", }, } `); @@ -363,7 +363,7 @@ describe('JsBundles computed artifact', () => { "sourceURL": "src/foo.js", }, "sizes": Object { - "error": "compiled.js.map mapping for last column out of bounds: 1:685", + "errorMessage": "compiled.js.map mapping for last column out of bounds: 1:685", }, } `); @@ -379,7 +379,7 @@ describe('JsBundles computed artifact', () => { Object { "entry": null, "sizes": Object { - "error": "compiled.js.map mapping for line out of bounds: 11", + "errorMessage": "compiled.js.map mapping for line out of bounds: 11", }, } `);