Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 4249276

Browse filesBrowse files
legendecasruyadorno
legendecas
authored and
ruyadorno
committed
src,lib: print source map error source on demand
The source context is not prepended to the value of the `stack` property when the source map is not enabled. Rather than prepending the error source context to the value of the `stack` property unconditionally, this patch aligns the behavior and only prints the source context when the error is not handled by userland (e.g. fatal errors). Also, this patch fixes that when source-map support is enabled, the error source context is not pointing to where the error was thrown. PR-URL: #43875 Fixes: #43186 Fixes: #41541 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 374b776 commit 4249276
Copy full SHA for 4249276
Expand file treeCollapse file tree

18 files changed

+221
-87
lines changed
Open diff view settings
Collapse file

‎.eslintignore‎

Copy file name to clipboardExpand all lines: .eslintignore
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ test/message/esm_display_syntax_error.mjs
66
tools/icu
77
tools/lint-md/lint-md.mjs
88
benchmark/tmp
9+
benchmark/fixtures
910
doc/**/*.js
1011
!doc/api_assets/*.js
1112
!.eslintrc.js
Collapse file

‎benchmark/es/error-stack.js‎

Copy file name to clipboard
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
const modPath = require.resolve('../fixtures/simple-error-stack.js');
5+
6+
const bench = common.createBenchmark(main, {
7+
method: ['without-sourcemap', 'sourcemap'],
8+
n: [1e5],
9+
});
10+
11+
function runN(n) {
12+
delete require.cache[modPath];
13+
const mod = require(modPath);
14+
bench.start();
15+
for (let i = 0; i < n; i++) {
16+
mod.simpleErrorStack();
17+
}
18+
bench.end(n);
19+
}
20+
21+
function main({ n, method }) {
22+
switch (method) {
23+
case 'without-sourcemap':
24+
process.setSourceMapsEnabled(false);
25+
runN(n);
26+
break;
27+
case 'sourcemap':
28+
process.setSourceMapsEnabled(true);
29+
runN(n);
30+
break;
31+
default:
32+
throw new Error(`Unexpected method "${method}"`);
33+
}
34+
}
Collapse file

‎benchmark/fixtures/simple-error-stack.js‎

Copy file name to clipboardExpand all lines: benchmark/fixtures/simple-error-stack.js
+15Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎benchmark/fixtures/simple-error-stack.ts‎

Copy file name to clipboard
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
3+
// Compile with `tsc --inlineSourceMap benchmark/fixtures/simple-error-stack.ts`.
4+
5+
const lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.';
6+
7+
function simpleErrorStack() {
8+
try {
9+
(lorem as any).BANG();
10+
} catch (e) {
11+
return e.stack;
12+
}
13+
}
14+
15+
export {
16+
simpleErrorStack,
17+
};
Collapse file

‎lib/internal/source_map/prepare_stack_trace.js‎

Copy file name to clipboardExpand all lines: lib/internal/source_map/prepare_stack_trace.js
+26-18Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525
kIsNodeError,
2626
} = require('internal/errors');
2727
const { fileURLToPath } = require('internal/url');
28+
const { setGetSourceMapErrorSource } = internalBinding('errors');
2829

2930
// Create a prettified stacktrace, inserting context from source maps
3031
// if possible.
@@ -53,7 +54,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
5354
return errorString;
5455
}
5556

56-
let errorSource = '';
5757
let lastSourceMap;
5858
let lastFileName;
5959
const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => {
@@ -62,14 +62,12 @@ const prepareStackTrace = (globalThis, error, trace) => {
6262
// A stack trace will often have several call sites in a row within the
6363
// same file, cache the source map and file content accordingly:
6464
let fileName = t.getFileName();
65-
let generated = false;
6665
if (fileName === undefined) {
6766
fileName = t.getEvalOrigin();
68-
generated = true;
6967
}
7068
const sm = fileName === lastFileName ?
7169
lastSourceMap :
72-
findSourceMap(fileName, generated);
70+
findSourceMap(fileName);
7371
lastSourceMap = sm;
7472
lastFileName = fileName;
7573
if (sm) {
@@ -83,14 +81,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
8381
if (originalSource && originalLine !== undefined &&
8482
originalColumn !== undefined) {
8583
const name = getOriginalSymbolName(sm, trace, i);
86-
if (i === 0) {
87-
errorSource = getErrorSource(
88-
sm,
89-
originalSource,
90-
originalLine,
91-
originalColumn
92-
);
93-
}
9484
// Construct call site name based on: v8.dev/docs/stack-trace-api:
9585
const fnName = t.getFunctionName() ?? t.getMethodName();
9686
const typeName = t.getTypeName();
@@ -116,7 +106,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
116106
}
117107
return `${str}${t}`;
118108
}), '');
119-
return `${errorSource}${errorString}\n at ${preparedTrace}`;
109+
return `${errorString}\n at ${preparedTrace}`;
120110
};
121111

122112
// Transpilers may have removed the original symbol name used in the stack
@@ -155,7 +145,7 @@ function getErrorSource(
155145
fileURLToPath(originalSourcePath) : originalSourcePath;
156146
const source = getOriginalSource(
157147
sourceMap.payload,
158-
originalSourcePathNoScheme
148+
originalSourcePath
159149
);
160150
const lines = RegExpPrototypeSymbolSplit(/\r?\n/, source, originalLine + 1);
161151
const line = lines[originalLine];
@@ -178,28 +168,46 @@ function getErrorSource(
178168

179169
function getOriginalSource(payload, originalSourcePath) {
180170
let source;
181-
const originalSourcePathNoScheme =
182-
StringPrototypeStartsWith(originalSourcePath, 'file://') ?
183-
fileURLToPath(originalSourcePath) : originalSourcePath;
171+
// payload.sources has been normalized to be an array of absolute urls.
184172
const sourceContentIndex =
185173
ArrayPrototypeIndexOf(payload.sources, originalSourcePath);
186174
if (payload.sourcesContent?.[sourceContentIndex]) {
187175
// First we check if the original source content was provided in the
188176
// source map itself:
189177
source = payload.sourcesContent[sourceContentIndex];
190-
} else {
178+
} else if (StringPrototypeStartsWith(originalSourcePath, 'file://')) {
191179
// If no sourcesContent was found, attempt to load the original source
192180
// from disk:
181+
debug(`read source of ${originalSourcePath} from filesystem`);
182+
const originalSourcePathNoScheme = fileURLToPath(originalSourcePath);
193183
try {
194184
source = readFileSync(originalSourcePathNoScheme, 'utf8');
195185
} catch (err) {
196186
debug(err);
197187
source = '';
198188
}
189+
} else {
190+
source = '';
199191
}
200192
return source;
201193
}
202194

195+
function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
196+
const sm = findSourceMap(fileName);
197+
if (sm === null) {
198+
return;
199+
}
200+
const {
201+
originalLine,
202+
originalColumn,
203+
originalSource,
204+
} = sm.findEntry(lineNumber - 1, columnNumber);
205+
const errorSource = getErrorSource(sm, originalSource, originalLine, originalColumn);
206+
return errorSource;
207+
}
208+
209+
setGetSourceMapErrorSource(getSourceMapErrorSource);
210+
203211
module.exports = {
204212
prepareStackTrace,
205213
};
Collapse file

‎lib/internal/source_map/source_map_cache.js‎

Copy file name to clipboardExpand all lines: lib/internal/source_map/source_map_cache.js
+54-37Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ const esmSourceMapCache = new SafeMap();
4141
// The generated sources is not mutable, so we can use a Map without memory concerns:
4242
const generatedSourceMapCache = new SafeMap();
4343
const kLeadingProtocol = /^\w+:\/\//;
44+
const kSourceMappingURLMagicComment = /\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/;
45+
const kSourceURLMagicComment = /\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/;
4446

4547
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
4648
let SourceMap;
@@ -77,7 +79,22 @@ function setSourceMapsEnabled(val) {
7779
sourceMapsEnabled = val;
7880
}
7981

80-
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource) {
82+
function extractSourceURLMagicComment(content) {
83+
const matchSourceURL = RegExpPrototypeExec(
84+
kSourceURLMagicComment,
85+
content
86+
);
87+
if (matchSourceURL === null) {
88+
return null;
89+
}
90+
let sourceURL = matchSourceURL.groups.sourceURL;
91+
if (sourceURL != null && RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
92+
sourceURL = pathToFileURL(sourceURL).href;
93+
}
94+
return sourceURL;
95+
}
96+
97+
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL) {
8198
const sourceMapsEnabled = getSourceMapsEnabled();
8299
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
83100
try {
@@ -87,10 +104,10 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
87104
debug(err);
88105
return;
89106
}
90-
const match = RegExpPrototypeExec(
91-
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/,
92-
content,
93-
);
107+
const match = RegExpPrototypeExec(kSourceMappingURLMagicComment, content);
108+
if (sourceURL === undefined) {
109+
sourceURL = extractSourceURLMagicComment(content);
110+
}
94111
if (match) {
95112
const data = dataFromUrl(filename, match.groups.sourceMappingURL);
96113
const url = data ? null : match.groups.sourceMappingURL;
@@ -99,22 +116,33 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
99116
filename,
100117
lineLengths: lineLengths(content),
101118
data,
102-
url
119+
url,
120+
sourceURL,
103121
});
104122
} else if (isGeneratedSource) {
105-
generatedSourceMapCache.set(filename, {
123+
const entry = {
106124
lineLengths: lineLengths(content),
107125
data,
108-
url
109-
});
126+
url,
127+
sourceURL
128+
};
129+
generatedSourceMapCache.set(filename, entry);
130+
if (sourceURL) {
131+
generatedSourceMapCache.set(sourceURL, entry);
132+
}
110133
} else {
111134
// If there is no cjsModuleInstance and is not generated source assume we are in a
112135
// "modules/esm" context.
113-
esmSourceMapCache.set(filename, {
136+
const entry = {
114137
lineLengths: lineLengths(content),
115138
data,
116-
url
117-
});
139+
url,
140+
sourceURL,
141+
};
142+
esmSourceMapCache.set(filename, entry);
143+
if (sourceURL) {
144+
esmSourceMapCache.set(sourceURL, entry);
145+
}
118146
}
119147
}
120148
}
@@ -123,19 +151,12 @@ function maybeCacheGeneratedSourceMap(content) {
123151
const sourceMapsEnabled = getSourceMapsEnabled();
124152
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
125153

126-
const matchSourceURL = RegExpPrototypeExec(
127-
/\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/,
128-
content
129-
);
130-
if (matchSourceURL == null) {
154+
const sourceURL = extractSourceURLMagicComment(content);
155+
if (sourceURL === null) {
131156
return;
132157
}
133-
let sourceURL = matchSourceURL.groups.sourceURL;
134-
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
135-
sourceURL = pathToFileURL(sourceURL).href;
136-
}
137158
try {
138-
maybeCacheSourceMap(sourceURL, content, null, true);
159+
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL);
139160
} catch (err) {
140161
// This can happen if the filename is not a valid URL.
141162
// If we fail to cache the source map, we should not fail the whole process.
@@ -254,33 +275,29 @@ function appendCJSCache(obj) {
254275
}
255276
}
256277

257-
function findSourceMap(sourceURL, isGenerated) {
278+
function findSourceMap(sourceURL) {
258279
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
259280
sourceURL = pathToFileURL(sourceURL).href;
260281
}
261282
if (!SourceMap) {
262283
SourceMap = require('internal/source_map/source_map').SourceMap;
263284
}
264-
let sourceMap;
265-
if (isGenerated) {
266-
sourceMap = generatedSourceMapCache.get(sourceURL);
267-
} else {
268-
sourceMap = esmSourceMapCache.get(sourceURL);
269-
if (sourceMap === undefined) {
270-
for (const value of cjsSourceMapCache) {
271-
const filename = ObjectGetValueSafe(value, 'filename');
272-
if (sourceURL === filename) {
273-
sourceMap = {
274-
data: ObjectGetValueSafe(value, 'data')
275-
};
276-
}
285+
let sourceMap = esmSourceMapCache.get(sourceURL) ?? generatedSourceMapCache.get(sourceURL);
286+
if (sourceMap === undefined) {
287+
for (const value of cjsSourceMapCache) {
288+
const filename = ObjectGetValueSafe(value, 'filename');
289+
const cachedSourceURL = ObjectGetValueSafe(value, 'sourceURL');
290+
if (sourceURL === filename || sourceURL === cachedSourceURL) {
291+
sourceMap = {
292+
data: ObjectGetValueSafe(value, 'data')
293+
};
277294
}
278295
}
279296
}
280297
if (sourceMap && sourceMap.data) {
281298
return new SourceMap(sourceMap.data);
282299
}
283-
return undefined;
300+
return null;
284301
}
285302

286303
module.exports = {
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,7 @@ class NoArrayBufferZeroFillScope {
525525
V(enhance_fatal_stack_after_inspector, v8::Function) \
526526
V(enhance_fatal_stack_before_inspector, v8::Function) \
527527
V(fs_use_promises_symbol, v8::Symbol) \
528+
V(get_source_map_error_source, v8::Function) \
528529
V(host_import_module_dynamically_callback, v8::Function) \
529530
V(host_initialize_import_meta_object_callback, v8::Function) \
530531
V(http2session_on_altsvc_function, v8::Function) \

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.