Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: add lineLength to source-map-cache #29863

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
]
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
52 changes: 52 additions & 0 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
@@ -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');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled prepare_stack_trace.js into its own file, because it seemed like it wasn't directly related to the source map caching functionality (and source_map_cache.js was starting to get a bit confusing).


// 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,
};
75 changes: 24 additions & 51 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Copy link
Contributor Author

@bcoe bcoe Oct 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's getting to the point where we write quite a bit of data to disk (and memory) -- we don't actually need the url field, if we've successfully parsed the map, so let's avoid storing it.

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
});
}
}
Expand All @@ -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) {
bcoe marked this conversation as resolved.
Show resolved Hide resolved
// 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');
Expand Down Expand Up @@ -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) {
Expand All @@ -230,8 +203,8 @@ function findSourceMap(uri, error) {
}

module.exports = {
findSourceMap,
maybeCacheSourceMap,
prepareStackTrace,
rekeySourceMap,
sourceMapCacheToObject,
};
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
30 changes: 30 additions & 0 deletions test/parallel/test-source-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down