Skip to content

Commit ccc60bf

Browse files
legendecaslegendecas
committed
lib: cache source maps in vm sources
Cache source maps found in sources parsed with `vm.Script`, `vm.compileFunction`, and `vm.SourceTextModule`. Also, retrieve source url from V8 parsing results. Not like filenames returned by `CallSite.getFileName()` in translating stack traces, when generating source lines prepended to exceptions, only resource names can be used as an index to find source maps, which can be source url magic comments instead. Source url magic comments can be either a file path or a URL. To verify that source urls with absolute file paths in the source lines are correctly translated, snapshots should include the full snapshot urls rather than neutralizing all the path strings in the stack traces.
1 parent 7450332 commit ccc60bf

37 files changed

+289
-103
lines changed

‎lib/internal/modules/cjs/loader.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,9 +1471,9 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
14711471
);
14721472

14731473
// Cache the source map for the module if present.
1474-
const { sourceMapURL } = script;
1474+
const { sourceMapURL, sourceURL } = script;
14751475
if (sourceMapURL) {
1476-
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, sourceMapURL);
1476+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, sourceURL, sourceMapURL);
14771477
}
14781478

14791479
return {
@@ -1498,7 +1498,7 @@ function wrapSafe(filename, content, cjsModuleInstance, format) {
14981498

14991499
// Cache the source map for the module if present.
15001500
if (result.sourceMapURL) {
1501-
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, undefined, result.sourceMapURL);
1501+
maybeCacheSourceMap(filename, content, cjsModuleInstance, false, result.sourceURL, result.sourceMapURL);
15021502
}
15031503

15041504
return result;

‎lib/internal/modules/esm/translators.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ translators.set('module', function moduleStrategy(url, source, isMain) {
118118
function loadCJSModule(module, source, url, filename, isMain) {
119119
const compileResult = compileFunctionForCJSLoader(source, filename, false /* is_sea_main */, false);
120120

121-
const { function: compiledWrapper, sourceMapURL } = compileResult;
121+
const { function: compiledWrapper, sourceMapURL, sourceURL } = compileResult;
122122
// Cache the source map for the cjs module if present.
123123
if (sourceMapURL) {
124-
maybeCacheSourceMap(url, source, module, false, undefined, sourceMapURL);
124+
maybeCacheSourceMap(url, source, module, false, sourceURL, sourceMapURL);
125125
}
126126

127127
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();

‎lib/internal/modules/esm/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ function compileSourceTextModule(url, source, cascadedLoader) {
342342
}
343343
// Cache the source map for the module if present.
344344
if (wrap.sourceMapURL) {
345-
maybeCacheSourceMap(url, source, wrap, false, undefined, wrap.sourceMapURL);
345+
maybeCacheSourceMap(url, source, wrap, false, wrap.sourceURL, wrap.sourceMapURL);
346346
}
347347
return wrap;
348348
}

‎lib/internal/source_map/source_map_cache.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,10 @@ function maybeCacheSourceMap(filename, content, moduleInstance, isGeneratedSourc
148148
return;
149149
}
150150

151-
// FIXME: callers should obtain sourceURL from v8 and pass it
152-
// rather than leaving it undefined and extract by regex.
153-
if (sourceURL === undefined) {
154-
sourceURL = extractSourceURLMagicComment(content);
151+
if (sourceURL !== undefined) {
152+
// SourceURL magic comment content might be a file path or URL.
153+
// Normalize the sourceURL to be a file URL if it is a file path.
154+
sourceURL = normalizeReferrerURL(sourceURL);
155155
}
156156

157157
const data = dataFromUrl(filename, sourceMapURL);
@@ -190,7 +190,7 @@ function maybeCacheGeneratedSourceMap(content) {
190190
return;
191191
}
192192
try {
193-
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL);
193+
maybeCacheSourceMap(sourceURL, content, null, true);
194194
} catch (err) {
195195
// This can happen if the filename is not a valid URL.
196196
// If we fail to cache the source map, we should not fail the whole process.

‎lib/internal/vm.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525
const {
2626
getOptionValue,
2727
} = require('internal/options');
28+
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
2829
const {
2930
privateSymbols: {
3031
contextify_context_private_symbol,
@@ -151,6 +152,7 @@ function internalCompileFunction(
151152
}
152153

153154
registerImportModuleDynamically(result.function, importModuleDynamically);
155+
maybeCacheSourceMap(filename, code, result.function, false, result.sourceURL, result.sourceMapURL);
154156

155157
return result;
156158
}

‎lib/internal/vm/module.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ const kPerContextModuleId = Symbol('kPerContextModuleId');
8181
const kLink = Symbol('kLink');
8282

8383
const { isContext } = require('internal/vm');
84+
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
8485

8586
function isModule(object) {
8687
if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, kWrap)) {
@@ -150,6 +151,7 @@ class Module {
150151
registry.callbackReferrer = this;
151152
const { registerModule } = require('internal/modules/esm/utils');
152153
registerModule(this[kWrap], registry);
154+
maybeCacheSourceMap(identifier, sourceText, this, false, this[kWrap].sourceURL, this[kWrap].sourceMapURL);
153155
} else {
154156
assert(syntheticEvaluationSteps);
155157
this[kWrap] = new ModuleWrap(identifier, context,

‎lib/vm.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ const {
6363
isContext: _isContext,
6464
registerImportModuleDynamically,
6565
} = require('internal/vm');
66+
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
6667
const {
6768
vm_dynamic_import_main_context_default,
6869
vm_context_no_contextify,
@@ -127,6 +128,7 @@ class Script extends ContextifyScript {
127128
}
128129

129130
registerImportModuleDynamically(this, importModuleDynamically);
131+
maybeCacheSourceMap(filename, code, this, false, this.sourceURL, this.sourceMapURL);
130132
}
131133

132134
runInThisContext(options) {

‎src/env_properties.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@
332332
V(sni_context_string, "sni_context") \
333333
V(source_string, "source") \
334334
V(source_map_url_string, "sourceMapURL") \
335+
V(source_url_string, "sourceURL") \
335336
V(specifier_string, "specifier") \
336337
V(stack_string, "stack") \
337338
V(standard_name_string, "standardName") \

‎src/module_wrap.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
291291
return;
292292
}
293293

294+
if (that->Set(context,
295+
realm->env()->source_url_string(),
296+
module->GetUnboundModuleScript()->GetSourceURL())
297+
.IsNothing()) {
298+
return;
299+
}
300+
294301
if (that->Set(context,
295302
realm->env()->source_map_url_string(),
296303
module->GetUnboundModuleScript()->GetSourceMappingURL())

‎src/node_contextify.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,13 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
10981098
return;
10991099
}
11001100

1101+
if (args.This()
1102+
->Set(env->context(),
1103+
env->source_url_string(),
1104+
v8_script->GetSourceURL())
1105+
.IsNothing())
1106+
return;
1107+
11011108
if (args.This()
11021109
->Set(env->context(),
11031110
env->source_map_url_string(),
@@ -1556,6 +1563,15 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
15561563
Local<Object> result = Object::New(isolate);
15571564
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
15581565
return Object::New(env->isolate());
1566+
1567+
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
1568+
// present.
1569+
if (result
1570+
->Set(parsing_context,
1571+
env->source_url_string(),
1572+
fn->GetScriptOrigin().ResourceName())
1573+
.IsNothing())
1574+
return Object::New(env->isolate());
15591575
if (result
15601576
->Set(parsing_context,
15611577
env->source_map_url_string(),
@@ -1793,12 +1809,16 @@ static void CompileFunctionForCJSLoader(
17931809
std::vector<Local<Name>> names = {
17941810
env->cached_data_rejected_string(),
17951811
env->source_map_url_string(),
1812+
env->source_url_string(),
17961813
env->function_string(),
17971814
FIXED_ONE_BYTE_STRING(isolate, "canParseAsESM"),
17981815
};
17991816
std::vector<Local<Value>> values = {
18001817
Boolean::New(isolate, cache_rejected),
18011818
fn.IsEmpty() ? undefined : fn->GetScriptOrigin().SourceMapUrl(),
1819+
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
1820+
// present.
1821+
fn.IsEmpty() ? undefined : fn->GetScriptOrigin().ResourceName(),
18021822
fn.IsEmpty() ? undefined : fn.As<Value>(),
18031823
Boolean::New(isolate, can_parse_as_esm),
18041824
};

‎src/node_errors.cc

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,20 @@ static std::string GetSourceMapErrorSource(Isolate* isolate,
5858
bool* added_exception_line) {
5959
v8::TryCatch try_catch(isolate);
6060
HandleScope handle_scope(isolate);
61-
Environment* env = Environment::GetCurrent(context);
61+
Realm* realm = Realm::GetCurrent(context);
62+
63+
Local<Function> get_source;
64+
if (realm != nullptr) {
65+
// If we are in a Realm, call the realm specific getSourceMapErrorSource
66+
// callback to avoid passing the JS objects (the exception and trace) across
67+
// the realm boundary.
68+
get_source = realm->get_source_map_error_source();
69+
} else {
70+
Environment* env = Environment::GetCurrent(context);
71+
// The context is created with ContextifyContext, call the principal
72+
// realm's getSourceMapErrorSource callback.
73+
get_source = env->principal_realm()->get_source_map_error_source();
74+
}
6275

6376
// The ScriptResourceName of the message may be different from the one we use
6477
// to compile the script. V8 replaces it when it detects magic comments in
@@ -70,8 +83,8 @@ static std::string GetSourceMapErrorSource(Isolate* isolate,
7083
Local<Value> argv[] = {script_resource_name,
7184
v8::Int32::New(isolate, linenum),
7285
v8::Int32::New(isolate, columnum)};
73-
MaybeLocal<Value> maybe_ret = env->get_source_map_error_source()->Call(
74-
context, Undefined(isolate), arraysize(argv), argv);
86+
MaybeLocal<Value> maybe_ret =
87+
get_source->Call(context, Undefined(isolate), arraysize(argv), argv);
7588
Local<Value> ret;
7689
if (!maybe_ret.ToLocal(&ret)) {
7790
// Ignore the caught exceptions.

‎test/common/assertSnapshot.js

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ const assert = require('node:assert/strict');
88
const stackFramesRegexp = /(?<=\n)(\s+)((.+?)\s+\()?(?:\(?(.+?):(\d+)(?::(\d+))?)\)?(\s+\{)?(\[\d+m)?(\n|$)/g;
99
const windowNewlineRegexp = /\r/g;
1010

11+
function replaceExperimentalWarning(str) {
12+
return str.replace(/\(node:\d+\) ExperimentalWarning: (.*)\n/g, '')
13+
.replace('(Use `node --trace-warnings ...` to show where the warning was created)\n', '');
14+
}
15+
1116
function replaceNodeVersion(str) {
1217
return str.replaceAll(process.version, '*');
1318
}
@@ -16,6 +21,10 @@ function replaceStackTrace(str, replacement = '$1*$7$8\n') {
1621
return str.replace(stackFramesRegexp, replacement);
1722
}
1823

24+
function replaceInternalStackTrace(str) {
25+
return str.replaceAll(/(\W+).*node:internal.*/g, '$1*');
26+
}
27+
1928
function replaceWindowsLineEndings(str) {
2029
return str.replace(windowNewlineRegexp, '');
2130
}
@@ -24,8 +33,20 @@ function replaceWindowsPaths(str) {
2433
return common.isWindows ? str.replaceAll(path.win32.sep, path.posix.sep) : str;
2534
}
2635

27-
function replaceFullPaths(str) {
28-
return str.replaceAll('\\\'', "'").replaceAll(path.resolve(__dirname, '../..'), '');
36+
function replaceWindowsDriveLetter(str) {
37+
if (!common.isWindows) {
38+
return str;
39+
}
40+
const currentDriveLetter = path.parse(process.cwd()).root.substring(0, 1).toLowerCase();
41+
const regex = new RegExp(`${currentDriveLetter}:`, 'gi');
42+
return str.replaceAll('\\\'', "'").replaceAll(regex, '');
43+
}
44+
45+
function transformCwd(replacement = '') {
46+
const cwd = process.cwd();
47+
return (str) => {
48+
return str.replaceAll('\\\'', "'").replaceAll(cwd, replacement);
49+
};
2950
}
3051

3152
function transform(...args) {
@@ -94,11 +115,14 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
94115
module.exports = {
95116
assertSnapshot,
96117
getSnapshotPath,
97-
replaceFullPaths,
118+
replaceExperimentalWarning,
98119
replaceNodeVersion,
99120
replaceStackTrace,
121+
replaceInternalStackTrace,
100122
replaceWindowsLineEndings,
101123
replaceWindowsPaths,
124+
replaceWindowsDriveLetter,
102125
spawnAndAssert,
103126
transform,
127+
transformCwd,
104128
};

‎test/fixtures/source-map/output/source_map_disabled_by_api.snapshot

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site-min.js:1:156)
3-
at functionC (*enclosing-call-site-min.js:1:97)
4-
at functionB (*enclosing-call-site-min.js:1:60)
5-
at functionA (*enclosing-call-site-min.js:1:26)
6-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
3+
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
4+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
5+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
6+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
77
Error: an error!
8-
at functionD (*enclosing-call-site.js:16:17)
9-
at functionC (*enclosing-call-site.js:10:3)
10-
at functionB (*enclosing-call-site.js:6:3)
11-
at functionA (*enclosing-call-site.js:2:3)
12-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
8+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
9+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
10+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
11+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
12+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)

‎test/fixtures/source-map/output/source_map_enabled_by_api.snapshot

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site.js:16:17)
3-
at functionC (*enclosing-call-site.js:10:3)
4-
at functionB (*enclosing-call-site.js:6:3)
5-
at functionA (*enclosing-call-site.js:2:3)
6-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
3+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
4+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
5+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
6+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
77
Error: an error!
8-
at functionD (*enclosing-call-site-min.js:1:156)
9-
at functionC (*enclosing-call-site-min.js:1:97)
10-
at functionB (*enclosing-call-site-min.js:1:60)
11-
at functionA (*enclosing-call-site-min.js:1:26)
12-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
8+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
9+
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
10+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
11+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
12+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)

‎test/fixtures/source-map/output/source_map_enclosing_function.snapshot

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
*enclosing-call-site.js:26
1+
*/test/fixtures/source-map/enclosing-call-site.js:26
22
throw err
33
^
44

55

66
Error: an error!
7-
at functionD (*enclosing-call-site.js:16:17)
8-
at functionC (*enclosing-call-site.js:10:3)
9-
at functionB (*enclosing-call-site.js:6:3)
10-
at functionA (*enclosing-call-site.js:2:3)
11-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
7+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
8+
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
9+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
10+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
11+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
1212

1313
Node.js *

‎test/fixtures/source-map/output/source_map_eval.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ Error.stackTraceLimit = 3;
77
const fs = require('fs');
88

99
const content = fs.readFileSync(require.resolve('../tabs.js'), 'utf8');
10+
// SourceURL magic comment is hardcoded in the source content.
1011
eval(content);

‎test/fixtures/source-map/output/source_map_eval.snapshot

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
*tabs.coffee:26
1+
/synthethized/workspace/tabs.coffee:26
22
alert "I knew it!"
33
^
44

55

66
ReferenceError: alert is not defined
7-
at Object.eval (*tabs.coffee:26:2)
8-
at eval (*tabs.coffee:1:14)
9-
at Object.<anonymous> (*output*source_map_eval.js:10:1)
7+
at Object.eval (/synthethized/workspace/tabs.coffee:26:2)
8+
at eval (/synthethized/workspace/tabs.coffee:1:14)
9+
at Object.<anonymous> (*/test/fixtures/source-map/output/source_map_eval.js:11:1)
1010

1111
Node.js *

‎test/fixtures/source-map/output/source_map_no_source_file.snapshot

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
*no-source.js:2
1+
*/test/fixtures/source-map/no-source.js:2
22
throw new Error('foo');
33
^
44

55
Error: foo
6-
at Throw (*file-not-exists.ts:2:9)
7-
at Object.<anonymous> (*file-not-exists.ts:5:1)
6+
at Throw (*/test/fixtures/source-map/file-not-exists.ts:2:9)
7+
at Object.<anonymous> (*/test/fixtures/source-map/file-not-exists.ts:5:1)
88

99
Node.js *

‎test/fixtures/source-map/output/source_map_prepare_stack_trace.snapshot

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
Error: an error!
2-
at functionD (*enclosing-call-site.js:16:17)
3-
at functionB (*enclosing-call-site.js:6:3)
4-
at functionA (*enclosing-call-site.js:2:3)
5-
at Object.<anonymous> (*enclosing-call-site.js:24:3)
2+
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
3+
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
4+
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
5+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
66
Error: an error!
7-
at functionD (*enclosing-call-site-min.js:1:156)
8-
at functionB (*enclosing-call-site-min.js:1:60)
9-
at functionA (*enclosing-call-site-min.js:1:26)
10-
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
7+
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
8+
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
9+
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
10+
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)

0 commit comments

Comments
 (0)