fix: add webpack support to the Sourcemapper (#640)

Fixes: #616

Author: DominicKramer

src/agent/io/sourcemapper.ts

@@ -24,6 +24,7 @@ import {findScriptsFuzzy} from '../util/utils';
 import pLimit = require('p-limit');
 
 const CONCURRENCY = 10;
+const WEBPACK_PREFIX = 'webpack://';
 const readFilep = promisify(fs.readFile);
 
 /** @define {string} */ const MAP_EXT = '.map';
@@ -32,6 +33,9 @@ export interface MapInfoInput {
   outputFile: string;
   mapFile: string;
   mapConsumer: sourceMap.RawSourceMap;
+  // the sources are in ascending order from
+  // shortest to longest
+  sources: string[];
 }
 
 export interface MapInfoOutput {
@@ -91,27 +95,36 @@ async function processSourcemap(
   const parentDir = path.dirname(mapPath);
   const outputPath = path.normalize(path.join(parentDir, outputBase));
 
-  const sources = Array.prototype.slice.call(consumer.sources)
-                      .filter((value: string) => {
-                        // filter out any empty string, null, or undefined
-                        // sources
-                        return !!value;
-                      })
-                      .map((relPath: string) => {
-                        // 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 (sources.length === 0) {
+  // the sources are in ascending order from shortest to longest
+  const nonemptySources = consumer.sources.filter(val => !!val)
+                              .sort((src1, src2) => src1.length - src2.length);
+
+  const normalizedSources =
+      nonemptySources
+          .map((src: string) => {
+            if (src.toLowerCase().startsWith(WEBPACK_PREFIX)) {
+              return src.substring(WEBPACK_PREFIX.length);
+            }
+            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
+            return path.normalize(path.join(parentDir, relPath));
+          });
+
+  if (normalizedSources.length === 0) {
     throw new Error('No sources listed in the sourcemap file ' + mapPath);
   }
-  sources.forEach((src: string) => {
-    infoMap.set(
-        path.normalize(src),
-        {outputFile: outputPath, mapFile: mapPath, mapConsumer: consumer});
-  });
+  for (const src of normalizedSources) {
+    infoMap.set(path.normalize(src), {
+      outputFile: outputPath,
+      mapFile: mapPath,
+      mapConsumer: consumer,
+      sources: nonemptySources
+    });
+  }
 }
 
 export class SourceMapper {
@@ -198,9 +211,25 @@ export class SourceMapper {
       return null;
     }
 
+    const relPath = path.relative(path.dirname(entry.mapFile), inputPath)
+                        .replace(/\\/g, '/');
+
+    /**
+     * Note: Since `entry.sources` is in ascending order from shortest
+     *       to longest, the first source path that ends with the
+     *       relative path is necessarily the shortest source path
+     *       that ends with the relative path.
+     */
+    let source: string|undefined;
+    for (const src of entry.sources) {
+      if (src.endsWith(relPath)) {
+        source = src;
+        break;
+      }
+    }
+
     const sourcePos = {
-      source: path.relative(path.dirname(entry.mapFile), inputPath)
-                  .replace(/\\/g, '/'),
+      source: source || relPath,
       line: lineNumber + 1,  // the SourceMapConsumer expects the line number
                              // to be one-based but expects the column number
       column: colNumber      // to be zero-based

test/fixtures/sourcemapper/webpack-ts/in.ts_

@@ -0,0 +1,8 @@
+
+function someFunction() {
+  const x = 1;
+  const y = 2;
+  console.log(x + y);
+}
+
+someFunction();

test/fixtures/sourcemapper/webpack-ts/out.js

@@ -0,0 +1,16 @@
+{
+  "name": "webpack-ts",
+  "version": "1.0.0",
+  "description": "",
+  "main": "webpack.config.js",
+  "scripts": {
+    "compile": "webpack"
+  },
+  "keywords": [],
+  "author": "",
+  "license": "ISC",
+  "devDependencies": {
+    "webpack": "^4.29.4",
+    "webpack-cli": "^3.2.3"
+  }
+}

test/fixtures/sourcemapper/webpack-ts/out.js.map

@@ -0,0 +1,15 @@
+
+const path = require('path');
+
+module.exports = {
+  mode: 'none',
+  entry: './in.ts_',
+  output: {
+    path: path.resolve(__dirname),
+    filename: 'out.js'
+  },
+  devtool: 'source-map',
+  optimization: {
+    nodeEnv: false
+  }
+};

test/fixtures/sourcemapper/webpack-ts/package.json

@@ -60,7 +60,10 @@ function testTool(
     it('for tool ' + tool +
            ' it states that it has mapping info for files it knows about',
        (done) => {
-         assert.strictEqual(sourcemapper.hasMappingInfo(inputFilePath), true);
+         assert.strictEqual(
+             sourcemapper.hasMappingInfo(inputFilePath), true,
+             `The sourcemapper should have information about '${
+                 inputFilePath}'`);
          done();
        });
 
@@ -72,16 +75,22 @@ function testTool(
              sourcemapper.hasMappingInfo(relativeInputFilePath), true);
          const movedPath =
              path.join('/some/other/base/dir/', relativeInputFilePath);
-         assert.strictEqual(sourcemapper.hasMappingInfo(movedPath), true);
+         assert.strictEqual(
+             sourcemapper.hasMappingInfo(movedPath), true,
+             `The sourcemapper should have information about paths similar to '${
+                 movedPath}'`);
          done();
        });
 
     it('for tool ' + tool +
            ' it states that it does not have mapping info for a file it ' +
            'doesn\'t recognize',
        (done) => {
+         const invalidPath = inputFilePath + '_INVALID';
          assert.strictEqual(
-             sourcemapper.hasMappingInfo(inputFilePath + '_INVALID'), false);
+             sourcemapper.hasMappingInfo(invalidPath), false,
+             `The source mapper should not have information the path '${
+                 invalidPath}' it doesn't recognize`);
          done();
        });
 
@@ -93,7 +102,8 @@ function testTool(
       assert.strictEqual(info!.file, outputFilePath);
       assert.strictEqual(
           info!.line, expectedOutputLine,
-          ' invalid mapping for input line ' + inputLine);
+          ' invalid mapping for input line ' + inputLine +
+              ' Expected: ' + expectedOutputLine + ', Found: ' + info!.line);
     };
 
     it('for tool ' + tool + ' it properly maps line numbers', (done) => {
@@ -134,3 +144,8 @@ testTool(
       [1, 1], [2, 7], [3, 8], [4, 9], [6, 12], [7, 13], [9, 20], [10, 23],
       [11, 24], [13, 31], [15, 33], [17, 36], [19, 38], [20, 40], [21, 44]
     ]);
+
+testTool(
+    'Webpack with Typescript', path.join('webpack-ts', 'out.js.map'),
+    path.join('webpack-ts', 'in.ts_'), path.join('webpack-ts', 'out.js'),
+    [[3, 93], [4, 94], [8, 97]]);