From 67d83b97168e5062663a57f43079e1c4938449a8 Mon Sep 17 00:00:00 2001 From: Louis Ye Date: Thu, 3 Jun 2021 17:44:13 -0400 Subject: [PATCH 1/6] fix(deps): upgrade to source-map 0.7.3 --- package.json | 2 +- src/agent/io/sourcemapper.ts | 81 +++++++++++++++++++++--------------- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/package.json b/package.json index e8dac865..a99b0fc2 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "gcp-metadata": "^4.0.0", "p-limit": "^3.0.1", "semver": "^7.0.0", - "source-map": "^0.6.1", + "source-map": "^0.7.3", "split": "^1.0.0" }, "devDependencies": { diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index 84e8787e..a8687e1c 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -29,7 +29,7 @@ const readFilep = promisify(fs.readFile); export interface MapInfoInput { outputFile: string; mapFile: string; - mapConsumer: sourceMap.RawSourceMap; + mapConsumer: sourceMap.SourceMapConsumer; // the sources are in ascending order from // shortest to longest sources: string[]; @@ -59,24 +59,23 @@ async function processSourcemap( } mapPath = path.normalize(mapPath); - let contents; + let rawSourceMapString; try { - contents = await readFilep(mapPath, 'utf8'); + rawSourceMapString = await readFilep(mapPath, 'utf8'); } catch (e) { throw new Error('Could not read sourcemap file ' + mapPath + ': ' + e); } - let consumer: sourceMap.RawSourceMap; + let rawSourceMap; try { - // TODO: Determine how to reconsile the type conflict where `consumer` - // is constructed as a SourceMapConsumer but is used as a - // RawSourceMap. - // TODO: Resolve the cast of `contents as any` (This is needed because the - // type is expected to be of `RawSourceMap` but the existing - // working code uses a string.) - consumer = new sourceMap.SourceMapConsumer( - contents as {} as sourceMap.RawSourceMap - ) as {} as sourceMap.RawSourceMap; + rawSourceMap = JSON.parse(rawSourceMapString); + } catch (e) { + throw new Error('Could not parse the raw sourcemap ' + mapPath + ': ' + e); + } + + let consumer: sourceMap.SourceMapConsumer; + try { + consumer = await new sourceMap.SourceMapConsumer(rawSourceMapString); } catch (e) { throw new Error( 'An error occurred while reading the ' + @@ -93,18 +92,33 @@ async function processSourcemap( * containing the map file. Otherwise, use the name of the output * file (with the .map extension removed) as the output file. */ - const outputBase = consumer.file - ? consumer.file + const outputBase = rawSourceMap.file + ? rawSourceMap.file : path.basename(mapPath, '.map'); const parentDir = path.dirname(mapPath); const outputPath = path.normalize(path.join(parentDir, outputBase)); - // the sources are in ascending order from shortest to longest - const nonemptySources = consumer.sources - .filter(val => !!val) - .sort((src1, src2) => src1.length - src2.length); + // The paths of the sources that are relative to the source map file. Sort + // them in ascending order from shortest to longest. + // For webpack file path, normalize the path after the webpack prefix so that + // the source map library can recognize it. + const sourcesRelToSrcmap = rawSourceMap.sources + .filter((val: string) => !!val) + .map((val: string) => { + if (val.toLowerCase().startsWith(WEBPACK_PREFIX)) { + return ( + WEBPACK_PREFIX + path.normalize(val.substr(WEBPACK_PREFIX.length)) + ); + } + return val; + }) + .sort((src1: string, src2: string) => src1.length - src2.length); - const normalizedSources = nonemptySources + // The paths of the sources that are relative to the current process's working + // directory. These are the ones that are used for the fuzzy search. For + // webpack file path, the prefix is filtered out for better fuzzy search + // result. + const sourcesRelToProc = sourcesRelToSrcmap .map((src: string) => { if (src.toLowerCase().startsWith(WEBPACK_PREFIX)) { return src.substring(WEBPACK_PREFIX.length); @@ -112,21 +126,21 @@ async function processSourcemap( return src; }) .map((relPath: string) => { - // resolve the paths relative to the map file so that - // they are relative to the process's current working - // directory + // resolve the paths relative to the map file so that they are relative to + // the process's current working directory return path.normalize(path.join(parentDir, relPath)); }); - if (normalizedSources.length === 0) { + if (sourcesRelToProc.length === 0) { throw new Error('No sources listed in the sourcemap file ' + mapPath); } - for (const src of normalizedSources) { + + for (const src of sourcesRelToProc) { infoMap.set(path.normalize(src), { outputFile: outputPath, mapFile: mapPath, mapConsumer: consumer, - sources: nonemptySources, + sources: sourcesRelToSrcmap, }); } } @@ -245,10 +259,7 @@ export class SourceMapper { column: colNumber, // to be zero-based }; - // TODO: Determine how to remove the explicit cast here. - const consumer: sourceMap.SourceMapConsumer = - entry.mapConsumer as {} as sourceMap.SourceMapConsumer; - const allPos = consumer.allGeneratedPositionsFor(sourcePos); + const allPos = entry.mapConsumer.allGeneratedPositionsFor(sourcePos); /* * Based on testing, it appears that the following code is needed to * properly get the correct mapping information. @@ -256,16 +267,18 @@ export class SourceMapper { * In particular, the generatedPositionFor() alone doesn't appear to * give the correct mapping information. */ - const mappedPos: sourceMap.Position = + const mappedPos: sourceMap.NullablePosition = allPos && allPos.length > 0 ? allPos.reduce((accumulator, value) => { - return value.line < accumulator.line ? value : accumulator; + return (value.line ?? 0) < (accumulator.line ?? 0) + ? value + : accumulator; }) - : consumer.generatedPositionFor(sourcePos); + : entry.mapConsumer.generatedPositionFor(sourcePos); return { file: entry.outputFile, - line: mappedPos.line - 1, // convert the one-based line numbers returned + line: (mappedPos.line ?? 0) - 1, // convert the one-based line numbers returned // by the SourceMapConsumer to the expected // zero-based output. // TODO: The `sourceMap.Position` type definition has a `column` From d518bd7dc51ec607f49362866cb29c7793ed809f Mon Sep 17 00:00:00 2001 From: Louis Ye Date: Fri, 4 Jun 2021 09:41:51 -0400 Subject: [PATCH 2/6] fix: add debugging info --- src/agent/io/sourcemapper.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index a8687e1c..18347fac 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -142,6 +142,10 @@ async function processSourcemap( mapConsumer: consumer, sources: sourcesRelToSrcmap, }); + + consumer.eachMapping(function(m) { + console.log('# m', m); + }); } } @@ -234,9 +238,16 @@ export class SourceMapper { return null; } + console.log('# inputPath', inputPath); + console.log('# entry.sources', entry.sources); + console.log('# entry.outputFile', entry.outputFile); + console.log('# entry.mapFile', entry.mapFile); + const relPath = path .relative(path.dirname(entry.mapFile), inputPath) .replace(/\\/g, '/'); + + console.log('# relPath', relPath); /** * Note: Since `entry.sources` is in ascending order from shortest @@ -251,6 +262,8 @@ export class SourceMapper { break; } } + + console.log('# source', source); const sourcePos = { source: source || relPath, @@ -258,6 +271,8 @@ export class SourceMapper { // to be one-based but expects the column number column: colNumber, // to be zero-based }; + + console.log('# sourcePos', sourcePos); const allPos = entry.mapConsumer.allGeneratedPositionsFor(sourcePos); /* @@ -267,6 +282,9 @@ export class SourceMapper { * In particular, the generatedPositionFor() alone doesn't appear to * give the correct mapping information. */ + + console.log('# allPos', allPos); + const mappedPos: sourceMap.NullablePosition = allPos && allPos.length > 0 ? allPos.reduce((accumulator, value) => { @@ -275,6 +293,8 @@ export class SourceMapper { : accumulator; }) : entry.mapConsumer.generatedPositionFor(sourcePos); + + console.log('# mappedPos', mappedPos); return { file: entry.outputFile, From 28a9ef1e5d6f052c06de10d7f938b42cc71de3a6 Mon Sep 17 00:00:00 2001 From: Louis Ye Date: Fri, 4 Jun 2021 09:45:09 -0400 Subject: [PATCH 3/6] fix: only run webpack tool related cases --- src/agent/io/sourcemapper.ts | 14 +++++++------- test/test-sourcemapper.ts | 5 ++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index 18347fac..d84d2cfc 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -143,7 +143,7 @@ async function processSourcemap( sources: sourcesRelToSrcmap, }); - consumer.eachMapping(function(m) { + consumer.eachMapping(m => { console.log('# m', m); }); } @@ -246,7 +246,7 @@ export class SourceMapper { const relPath = path .relative(path.dirname(entry.mapFile), inputPath) .replace(/\\/g, '/'); - + console.log('# relPath', relPath); /** @@ -262,7 +262,7 @@ export class SourceMapper { break; } } - + console.log('# source', source); const sourcePos = { @@ -271,7 +271,7 @@ export class SourceMapper { // to be one-based but expects the column number column: colNumber, // to be zero-based }; - + console.log('# sourcePos', sourcePos); const allPos = entry.mapConsumer.allGeneratedPositionsFor(sourcePos); @@ -282,9 +282,9 @@ export class SourceMapper { * In particular, the generatedPositionFor() alone doesn't appear to * give the correct mapping information. */ - + console.log('# allPos', allPos); - + const mappedPos: sourceMap.NullablePosition = allPos && allPos.length > 0 ? allPos.reduce((accumulator, value) => { @@ -293,7 +293,7 @@ export class SourceMapper { : accumulator; }) : entry.mapConsumer.generatedPositionFor(sourcePos); - + console.log('# mappedPos', mappedPos); return { diff --git a/test/test-sourcemapper.ts b/test/test-sourcemapper.ts index c46fde09..c5a2484f 100644 --- a/test/test-sourcemapper.ts +++ b/test/test-sourcemapper.ts @@ -51,7 +51,7 @@ function testTool( const inputFilePath = path.join(BASE_PATH, relativeInputFilePath); const outputFilePath = path.join(BASE_PATH, relativeOutputFilePath); - describe('sourcemapper for tool ' + tool, () => { + describe.only('sourcemapper for tool ' + tool, () => { let sourcemapper: sm.SourceMapper; it('for tool ' + tool, async () => { @@ -146,6 +146,7 @@ function testTool( }); } +/* testTool( 'Babel', path.join('babel', 'out.js.map'), @@ -251,6 +252,8 @@ testTool( ] ); +/* + testTool( 'Webpack with Typescript', path.join('webpack-ts', 'out.js.map'), From 9bcc56e3e3996e1d20319f8b0859cd0396e2cc01 Mon Sep 17 00:00:00 2001 From: Louis Ye Date: Fri, 4 Jun 2021 09:47:41 -0400 Subject: [PATCH 4/6] fix: uncomment the webpack tool test --- test/test-sourcemapper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test-sourcemapper.ts b/test/test-sourcemapper.ts index c5a2484f..59ae04f5 100644 --- a/test/test-sourcemapper.ts +++ b/test/test-sourcemapper.ts @@ -146,7 +146,7 @@ function testTool( }); } -/* +/* testTool( 'Babel', path.join('babel', 'out.js.map'), @@ -252,7 +252,7 @@ testTool( ] ); -/* +*/ testTool( 'Webpack with Typescript', From a025299352e39717a9c3453b08442dc079a38d4b Mon Sep 17 00:00:00 2001 From: Louis Ye Date: Fri, 4 Jun 2021 10:10:49 -0400 Subject: [PATCH 5/6] fix: make webpack relative paths always use unix format --- src/agent/io/sourcemapper.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index d84d2cfc..43a521b7 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -107,7 +107,8 @@ async function processSourcemap( .map((val: string) => { if (val.toLowerCase().startsWith(WEBPACK_PREFIX)) { return ( - WEBPACK_PREFIX + path.normalize(val.substr(WEBPACK_PREFIX.length)) + WEBPACK_PREFIX + + path.normalize(val.substr(WEBPACK_PREFIX.length)).replace(/\\/g, '/') ); } return val; @@ -115,10 +116,11 @@ async function processSourcemap( .sort((src1: string, src2: string) => src1.length - src2.length); // The paths of the sources that are relative to the current process's working - // directory. These are the ones that are used for the fuzzy search. For - // webpack file path, the prefix is filtered out for better fuzzy search + // directory. These are the ones that are used for the fuzzy search (thus are + // platform specific, e.g. using '\\' on Windows and using '/' in Unix, etc.). + // For webpack file path, the prefix is filtered out for better fuzzy search // result. - const sourcesRelToProc = sourcesRelToSrcmap + const normalizedSourcesRelToProc = sourcesRelToSrcmap .map((src: string) => { if (src.toLowerCase().startsWith(WEBPACK_PREFIX)) { return src.substring(WEBPACK_PREFIX.length); @@ -131,11 +133,11 @@ async function processSourcemap( return path.normalize(path.join(parentDir, relPath)); }); - if (sourcesRelToProc.length === 0) { + if (normalizedSourcesRelToProc.length === 0) { throw new Error('No sources listed in the sourcemap file ' + mapPath); } - for (const src of sourcesRelToProc) { + for (const src of normalizedSourcesRelToProc) { infoMap.set(path.normalize(src), { outputFile: outputPath, mapFile: mapPath, From 5c13629619fb4d9cef755c97ea18a72a100911ae Mon Sep 17 00:00:00 2001 From: Louis Ye Date: Fri, 4 Jun 2021 10:17:07 -0400 Subject: [PATCH 6/6] fix: clear debugging info --- src/agent/io/sourcemapper.ts | 21 --------------------- test/test-sourcemapper.ts | 5 +---- 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/src/agent/io/sourcemapper.ts b/src/agent/io/sourcemapper.ts index 43a521b7..3de4ffa8 100644 --- a/src/agent/io/sourcemapper.ts +++ b/src/agent/io/sourcemapper.ts @@ -144,10 +144,6 @@ async function processSourcemap( mapConsumer: consumer, sources: sourcesRelToSrcmap, }); - - consumer.eachMapping(m => { - console.log('# m', m); - }); } } @@ -240,17 +236,9 @@ export class SourceMapper { return null; } - console.log('# inputPath', inputPath); - console.log('# entry.sources', entry.sources); - console.log('# entry.outputFile', entry.outputFile); - console.log('# entry.mapFile', entry.mapFile); - const relPath = path .relative(path.dirname(entry.mapFile), inputPath) .replace(/\\/g, '/'); - - console.log('# relPath', relPath); - /** * Note: Since `entry.sources` is in ascending order from shortest * to longest, the first source path that ends with the @@ -265,8 +253,6 @@ export class SourceMapper { } } - console.log('# source', source); - const sourcePos = { source: source || relPath, line: lineNumber + 1, // the SourceMapConsumer expects the line number @@ -274,8 +260,6 @@ export class SourceMapper { column: colNumber, // to be zero-based }; - console.log('# sourcePos', sourcePos); - const allPos = entry.mapConsumer.allGeneratedPositionsFor(sourcePos); /* * Based on testing, it appears that the following code is needed to @@ -284,9 +268,6 @@ export class SourceMapper { * In particular, the generatedPositionFor() alone doesn't appear to * give the correct mapping information. */ - - console.log('# allPos', allPos); - const mappedPos: sourceMap.NullablePosition = allPos && allPos.length > 0 ? allPos.reduce((accumulator, value) => { @@ -296,8 +277,6 @@ export class SourceMapper { }) : entry.mapConsumer.generatedPositionFor(sourcePos); - console.log('# mappedPos', mappedPos); - return { file: entry.outputFile, line: (mappedPos.line ?? 0) - 1, // convert the one-based line numbers returned diff --git a/test/test-sourcemapper.ts b/test/test-sourcemapper.ts index 59ae04f5..c46fde09 100644 --- a/test/test-sourcemapper.ts +++ b/test/test-sourcemapper.ts @@ -51,7 +51,7 @@ function testTool( const inputFilePath = path.join(BASE_PATH, relativeInputFilePath); const outputFilePath = path.join(BASE_PATH, relativeOutputFilePath); - describe.only('sourcemapper for tool ' + tool, () => { + describe('sourcemapper for tool ' + tool, () => { let sourcemapper: sm.SourceMapper; it('for tool ' + tool, async () => { @@ -146,7 +146,6 @@ function testTool( }); } -/* testTool( 'Babel', path.join('babel', 'out.js.map'), @@ -252,8 +251,6 @@ testTool( ] ); -*/ - testTool( 'Webpack with Typescript', path.join('webpack-ts', 'out.js.map'),