From 93c80678391a8fde9420433f968c06965d2048f5 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 6 Oct 2019 11:37:42 -0700 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/29863 Reviewed-By: Gus Caplan Reviewed-By: David Carlier Reviewed-By: James M Snell --- doc/api/cli.md | 13 +++- lib/internal/bootstrap/pre_execution.js | 2 +- .../source_map/prepare_stack_trace.js | 52 +++++++++++++ lib/internal/source_map/source_map_cache.js | 75 ++++++------------- node.gyp | 1 + test/parallel/test-source-map.js | 30 ++++++++ 6 files changed, 118 insertions(+), 55 deletions(-) create mode 100644 lib/internal/source_map/prepare_stack_trace.js diff --git a/doc/api/cli.md b/doc/api/cli.md index 4e851d3c6e27af..f588664c1992e2 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1191,8 +1191,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 { @@ -1218,7 +1219,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 index f9593101561416..c7e9e9433aa8d4 100644 --- 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 new file mode 100644 index 00000000000000..d6c5fce60a29e3 --- /dev/null +++ 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 index 94a4165546b77c..0e77203e9d6f1d 100644 --- 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=(?[^\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 index 4f12f5bbe5ba3d..aa2b361f9548ac 100644 --- 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 index 41a315e3f8184c..2ded13a631dd8c 100644 --- 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) {