process: add lineLength to source-map-cache

Without the line lengths of in-memory transpiled source, it's not possible to convert from byte ofsets to line/column offsets. PR-URL: #29863 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Oct 14, 2019 · 4ca61f4 · 4ca61f4
1 parent f8f6a21
commit 4ca61f4
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 55 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -1171,8 +1171,9 @@ If found, Source Map data is appended to the top-level key `source-map-cache`
 on the JSON coverage object.
 
 `source-map-cache` is an object with keys representing the files source maps
-were extracted from, and the values include the raw source-map URL
-(in the key `url`) and the parsed Source Map V3 information (in the key `data`).
+were extracted from, and values which include the raw source-map URL
+(in the key `url`), the parsed Source Map V3 information (in the key `data`),
+and the line lengths of the source file (in the key `lineLengths`).
 
 ```json
 {
@@ -1198,7 +1199,13 @@ were extracted from, and the values include the raw source-map URL
         ],
         "mappings": "MAAMA,IACJC,YAAaC",
         "sourceRoot": "./"
-      }
+      },
+      "lineLengths": [
+        13,
+        62,
+        38,
+        27
+      ]
     }
   }
 }

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -25,7 +25,7 @@ function prepareMainThreadExecution(expandArgv1 = false) {
   // prepareStackTrace method, replacing the default in errors.js.
   if (getOptionValue('--enable-source-maps')) {
     const { prepareStackTrace } =
-      require('internal/source_map/source_map_cache');
+      require('internal/source_map/prepare_stack_trace');
     const { setPrepareStackTraceCallback } = internalBinding('errors');
     setPrepareStackTraceCallback(prepareStackTrace);
   }

diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js
@@ -0,0 +1,52 @@
+'use strict';
+
+const debug = require('internal/util/debuglog').debuglog('source_map');
+const { findSourceMap } = require('internal/source_map/source_map_cache');
+const { overrideStackTrace } = require('internal/errors');
+
+// Create a prettified stacktrace, inserting context from source maps
+// if possible.
+const ErrorToString = Error.prototype.toString; // Capture original toString.
+const prepareStackTrace = (globalThis, error, trace) => {
+  // API for node internals to override error stack formatting
+  // without interfering with userland code.
+  // TODO(bcoe): add support for source-maps to repl.
+  if (overrideStackTrace.has(error)) {
+    const f = overrideStackTrace.get(error);
+    overrideStackTrace.delete(error);
+    return f(error, trace);
+  }
+
+  const { SourceMap } = require('internal/source_map/source_map');
+  const errorString = ErrorToString.call(error);
+
+  if (trace.length === 0) {
+    return errorString;
+  }
+  const preparedTrace = trace.map((t, i) => {
+    let str = i !== 0 ? '\n    at ' : '';
+    str = `${str}${t}`;
+    try {
+      const sourceMap = findSourceMap(t.getFileName(), error);
+      if (sourceMap && sourceMap.data) {
+        const sm = new SourceMap(sourceMap.data);
+        // Source Map V3 lines/columns use zero-based offsets whereas, in
+        // stack traces, they start at 1/1.
+        const [, , url, line, col] =
+                   sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
+        if (url && line !== undefined && col !== undefined) {
+          str +=
+            `\n        -> ${url.replace('file://', '')}:${line + 1}:${col + 1}`;
+        }
+      }
+    } catch (err) {
+      debug(err.stack);
+    }
+    return str;
+  });
+  return `${errorString}\n    at ${preparedTrace.join('')}`;
+};
+
+module.exports = {
+  prepareStackTrace,
+};
diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js
@@ -17,7 +17,6 @@ const cjsSourceMapCache = new WeakMap();
 // on filenames.
 const esmSourceMapCache = new Map();
 const { fileURLToPath, URL } = require('url');
-const { overrideStackTrace } = require('internal/errors');
 
 let experimentalSourceMaps;
 function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
@@ -38,18 +37,22 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
 
   const match = content.match(/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/);
   if (match) {
+    const data = dataFromUrl(basePath, match.groups.sourceMappingURL);
+    const url = data ? null : match.groups.sourceMappingURL;
     if (cjsModuleInstance) {
       cjsSourceMapCache.set(cjsModuleInstance, {
         filename,
-        url: match.groups.sourceMappingURL,
-        data: dataFromUrl(basePath, match.groups.sourceMappingURL)
+        lineLengths: lineLengths(content),
+        data,
+        url
       });
     } else {
       // If there is no cjsModuleInstance assume we are in a
       // "modules/esm" context.
       esmSourceMapCache.set(filename, {
-        url: match.groups.sourceMappingURL,
-        data: dataFromUrl(basePath, match.groups.sourceMappingURL)
+        lineLengths: lineLengths(content),
+        data,
+        url
       });
     }
   }
@@ -73,6 +76,18 @@ function dataFromUrl(basePath, sourceMappingURL) {
   }
 }
 
+// Cache the length of each line in the file that a source map was extracted
+// from. This allows translation from byte offset V8 coverage reports,
+// to line/column offset Source Map V3.
+function lineLengths(content) {
+  // We purposefully keep \r as part of the line-length calculation, in
+  // cases where there is a \r\n separator, so that this can be taken into
+  // account in coverage calculations.
+  return content.split(/\n|\u2028|\u2029/).map((line) => {
+    return line.length;
+  });
+}
+
 function sourceMapFromFile(sourceMapFile) {
   try {
     const content = fs.readFileSync(sourceMapFile, 'utf8');
@@ -161,56 +176,14 @@ function appendCJSCache(obj) {
     const value = cjsSourceMapCache.get(Module._cache[key]);
     if (value) {
       obj[`file://${key}`] = {
-        url: value.url,
-        data: value.data
+        lineLengths: value.lineLengths,
+        data: value.data,
+        url: value.url
       };
     }
   });
 }
 
-// Create a prettified stacktrace, inserting context from source maps
-// if possible.
-const ErrorToString = Error.prototype.toString; // Capture original toString.
-const prepareStackTrace = (globalThis, error, trace) => {
-  // API for node internals to override error stack formatting
-  // without interfering with userland code.
-  // TODO(bcoe): add support for source-maps to repl.
-  if (overrideStackTrace.has(error)) {
-    const f = overrideStackTrace.get(error);
-    overrideStackTrace.delete(error);
-    return f(error, trace);
-  }
-
-  const { SourceMap } = require('internal/source_map/source_map');
-  const errorString = ErrorToString.call(error);
-
-  if (trace.length === 0) {
-    return errorString;
-  }
-  const preparedTrace = trace.map((t, i) => {
-    let str = i !== 0 ? '\n    at ' : '';
-    str = `${str}${t}`;
-    try {
-      const sourceMap = findSourceMap(t.getFileName(), error);
-      if (sourceMap && sourceMap.data) {
-        const sm = new SourceMap(sourceMap.data);
-        // Source Map V3 lines/columns use zero-based offsets whereas, in
-        // stack traces, they start at 1/1.
-        const [, , url, line, col] =
-                   sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
-        if (url && line !== undefined && col !== undefined) {
-          str +=
-            `\n        -> ${url.replace('file://', '')}:${line + 1}:${col + 1}`;
-        }
-      }
-    } catch (err) {
-      debug(err.stack);
-    }
-    return str;
-  });
-  return `${errorString}\n    at ${preparedTrace.join('')}`;
-};
-
 // Attempt to lookup a source map, which is either attached to a file URI, or
 // keyed on an error instance.
 function findSourceMap(uri, error) {
@@ -230,8 +203,8 @@ function findSourceMap(uri, error) {
 }
 
 module.exports = {
+  findSourceMap,
   maybeCacheSourceMap,
-  prepareStackTrace,
   rekeySourceMap,
   sourceMapCacheToObject,
 };
diff --git a/node.gyp b/node.gyp
@@ -176,6 +176,7 @@
       'lib/internal/repl/history.js',
       'lib/internal/repl/utils.js',
       'lib/internal/socket_list.js',
+      'lib/internal/source_map/prepare_stack_trace.js',
       'lib/internal/source_map/source_map.js',
       'lib/internal/source_map/source_map_cache.js',
       'lib/internal/test/binding.js',

diff --git a/test/parallel/test-source-map.js b/test/parallel/test-source-map.js
@@ -193,6 +193,36 @@ function nextdir() {
   );
 }
 
+// Does not persist url parameter if source-map has been parsed.
+{
+  const coverageDirectory = nextdir();
+  spawnSync(process.execPath, [
+    require.resolve('../fixtures/source-map/inline-base64.js')
+  ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
+  const sourceMap = getSourceMapFromCache(
+    'inline-base64.js',
+    coverageDirectory
+  );
+  assert.strictEqual(sourceMap.url, null);
+}
+
+// Persists line lengths for in-memory representation of source file.
+{
+  const coverageDirectory = nextdir();
+  spawnSync(process.execPath, [
+    require.resolve('../fixtures/source-map/istanbul-throw.js')
+  ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
+  const sourceMap = getSourceMapFromCache(
+    'istanbul-throw.js',
+    coverageDirectory
+  );
+  if (common.isWindows) {
+    assert.deepStrictEqual(sourceMap.lineLengths, [1086, 31, 185, 649, 0]);
+  } else {
+    assert.deepStrictEqual(sourceMap.lineLengths, [1085, 30, 184, 648, 0]);
+  }
+}
+
 function getSourceMapFromCache(fixtureFile, coverageDirectory) {
   const jsonFiles = fs.readdirSync(coverageDirectory);
   for (const jsonFile of jsonFiles) {