Skip to content

Commit

Permalink
lib: cache source maps in vm sources
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
legendecas committed Mar 24, 2024
1 parent bae14b7 commit 44aa272
Show file tree
Hide file tree
Showing 35 changed files with 284 additions and 101 deletions.
6 changes: 3 additions & 3 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1308,9 +1308,9 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
);

// Cache the source map for the module if present.
const { sourceMapURL } = script;
const { sourceMapURL, sourceURL } = script;
if (sourceMapURL) {
maybeCacheSourceMap(filename, content, this, false, undefined, sourceMapURL);
maybeCacheSourceMap(filename, content, this, false, sourceURL, sourceMapURL);
}

return {
Expand All @@ -1332,7 +1332,7 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {

// Cache the source map for the module if present.
if (result.sourceMapURL) {
maybeCacheSourceMap(filename, content, this, false, undefined, result.sourceMapURL);
maybeCacheSourceMap(filename, content, this, false, result.sourceURL, result.sourceMapURL);
}

return result;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class ModuleLoader {
const wrap = new ModuleWrap(url, undefined, source, 0, 0);
// Cache the source map for the module if present.
if (wrap.sourceMapURL) {
maybeCacheSourceMap(url, source, null, false, undefined, wrap.sourceMapURL);
maybeCacheSourceMap(url, source, null, false, wrap.sourceURL, wrap.sourceMapURL);
}
const { registerModule } = require('internal/modules/esm/utils');
// TODO(joyeecheung): refactor so that the default options are shared across
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ translators.set('module', function moduleStrategy(url, source, isMain) {
const module = new ModuleWrap(url, undefined, source, 0, 0);
// Cache the source map for the module if present.
if (module.sourceMapURL) {
maybeCacheSourceMap(url, source, null, false, undefined, module.sourceMapURL);
maybeCacheSourceMap(url, source, null, false, module.sourceURL, module.sourceMapURL);
}
const { registerModule } = require('internal/modules/esm/utils');
registerModule(module, {
Expand Down Expand Up @@ -227,7 +227,7 @@ function loadCJSModule(module, source, url, filename) {
}
// Cache the source map for the cjs module if present.
if (compileResult.sourceMapURL) {
maybeCacheSourceMap(url, source, null, false, undefined, compileResult.sourceMapURL);
maybeCacheSourceMap(url, source, null, false, compileResult.sourceURL, compileResult.sourceMapURL);
}

const compiledWrapper = compileResult.function;
Expand Down
13 changes: 5 additions & 8 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
return;
}

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

const data = dataFromUrl(filename, sourceMapURL);
Expand All @@ -149,9 +149,6 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
sourceURL,
};
generatedSourceMapCache.set(filename, entry);
if (sourceURL) {
generatedSourceMapCache.set(sourceURL, entry);
}
} else {
// If there is no cjsModuleInstance and is not generated source assume we are in a
// "modules/esm" context.
Expand All @@ -178,7 +175,7 @@ function maybeCacheGeneratedSourceMap(content) {
return;
}
try {
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL);
maybeCacheSourceMap(sourceURL, content, null, true);
} catch (err) {
// This can happen if the filename is not a valid URL.
// If we fail to cache the source map, we should not fail the whole process.
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
const {
getOptionValue,
} = require('internal/options');
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
const {
privateSymbols: {
contextify_context_private_symbol,
Expand Down Expand Up @@ -151,6 +152,7 @@ function internalCompileFunction(
}

registerImportModuleDynamically(result.function, importModuleDynamically);
maybeCacheSourceMap(filename, code, result.function, false, result.sourceURL, result.sourceMapURL);

return result;
}
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/vm/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const kPerContextModuleId = Symbol('kPerContextModuleId');
const kLink = Symbol('kLink');

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

function isModule(object) {
if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, kWrap)) {
Expand Down Expand Up @@ -141,6 +142,7 @@ class Module {
importModuleDynamicallyWrap(options.importModuleDynamically) :
undefined,
};
maybeCacheSourceMap(identifier, sourceText, this, false, this[kWrap].sourceURL, this[kWrap].sourceMapURL);
} else {
assert(syntheticEvaluationSteps);
this[kWrap] = new ModuleWrap(identifier, context,
Expand Down
2 changes: 2 additions & 0 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const {
isContext: _isContext,
registerImportModuleDynamically,
} = require('internal/vm');
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
const {
vm_dynamic_import_main_context_default,
} = internalBinding('symbols');
Expand Down Expand Up @@ -126,6 +127,7 @@ class Script extends ContextifyScript {
}

registerImportModuleDynamically(this, importModuleDynamically);
maybeCacheSourceMap(filename, code, this, false, this.sourceURL, this.sourceMapURL);
}

runInThisContext(options) {
Expand Down
1 change: 1 addition & 0 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@
V(sni_context_string, "sni_context") \
V(source_string, "source") \
V(source_map_url_string, "sourceMapURL") \
V(source_url_string, "sourceURL") \
V(stack_string, "stack") \
V(standard_name_string, "standardName") \
V(start_time_string, "startTime") \
Expand Down
7 changes: 7 additions & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,13 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
return;
}

if (that->Set(context,
realm->env()->source_url_string(),
module->GetUnboundModuleScript()->GetSourceURL())
.IsNothing()) {
return;
}

if (that->Set(context,
realm->env()->source_map_url_string(),
module->GetUnboundModuleScript()->GetSourceMappingURL())
Expand Down
20 changes: 20 additions & 0 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,13 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
return;
}

if (args.This()
->Set(env->context(),
env->source_url_string(),
v8_script->GetSourceURL())
.IsNothing())
return;

if (args.This()
->Set(env->context(),
env->source_map_url_string(),
Expand Down Expand Up @@ -1373,6 +1380,15 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
Local<Object> result = Object::New(isolate);
if (result->Set(parsing_context, env->function_string(), fn).IsNothing())
return Object::New(env->isolate());

// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
// present.
if (result
->Set(parsing_context,
env->source_url_string(),
fn->GetScriptOrigin().ResourceName())
.IsNothing())
return Object::New(env->isolate());
if (result
->Set(parsing_context,
env->source_map_url_string(),
Expand Down Expand Up @@ -1650,11 +1666,15 @@ static void CompileFunctionForCJSLoader(
std::vector<Local<Name>> names = {
env->cached_data_rejected_string(),
env->source_map_url_string(),
env->source_url_string(),
env->function_string(),
};
std::vector<Local<Value>> values = {
Boolean::New(isolate, cache_rejected),
fn->GetScriptOrigin().SourceMapUrl(),
// ScriptOrigin::ResourceName() returns SourceURL magic comment content if
// present.
fn->GetScriptOrigin().ResourceName(),
fn,
};
Local<Object> result = Object::New(
Expand Down
19 changes: 16 additions & 3 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,20 @@ static std::string GetSourceMapErrorSource(Isolate* isolate,
bool* added_exception_line) {
v8::TryCatch try_catch(isolate);
HandleScope handle_scope(isolate);
Environment* env = Environment::GetCurrent(context);
Realm* realm = Realm::GetCurrent(context);

Local<Function> get_source;
if (realm != nullptr) {
// If we are in a Realm, call the realm specific getSourceMapErrorSource
// callback to avoid passing the JS objects (the exception and trace) across
// the realm boundary.
get_source = realm->get_source_map_error_source();
} else {
Environment* env = Environment::GetCurrent(context);
// The context is created with ContextifyContext, call the principal
// realm's getSourceMapErrorSource callback.
get_source = env->principal_realm()->get_source_map_error_source();
}

// The ScriptResourceName of the message may be different from the one we use
// to compile the script. V8 replaces it when it detects magic comments in
Expand All @@ -69,8 +82,8 @@ static std::string GetSourceMapErrorSource(Isolate* isolate,
Local<Value> argv[] = {script_resource_name,
v8::Int32::New(isolate, linenum),
v8::Int32::New(isolate, columnum)};
MaybeLocal<Value> maybe_ret = env->get_source_map_error_source()->Call(
context, Undefined(isolate), arraysize(argv), argv);
MaybeLocal<Value> maybe_ret =
get_source->Call(context, Undefined(isolate), arraysize(argv), argv);
Local<Value> ret;
if (!maybe_ret.ToLocal(&ret)) {
// Ignore the caught exceptions.
Expand Down
30 changes: 27 additions & 3 deletions test/common/assertSnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ const assert = require('node:assert/strict');
const stackFramesRegexp = /(?<=\n)(\s+)((.+?)\s+\()?(?:\(?(.+?):(\d+)(?::(\d+))?)\)?(\s+\{)?(\[\d+m)?(\n|$)/g;
const windowNewlineRegexp = /\r/g;

function replaceExperimentalWarning(str) {
return str.replace(/\(node:\d+\) ExperimentalWarning: (.*)\n/g, '')
.replace('(Use `node --trace-warnings ...` to show where the warning was created)\n', '');
}

function replaceNodeVersion(str) {
return str.replaceAll(process.version, '*');
}
Expand All @@ -16,6 +21,10 @@ function replaceStackTrace(str, replacement = '$1*$7$8\n') {
return str.replace(stackFramesRegexp, replacement);
}

function replaceInternalStackTrace(str) {
return str.replaceAll(/(\W+).*node:internal.*/g, '$1*');
}

function replaceWindowsLineEndings(str) {
return str.replace(windowNewlineRegexp, '');
}
Expand All @@ -24,8 +33,20 @@ function replaceWindowsPaths(str) {
return common.isWindows ? str.replaceAll(path.win32.sep, path.posix.sep) : str;
}

function replaceFullPaths(str) {
return str.replaceAll(process.cwd(), '');
function replaceWindowsDriveLetter(str) {
if (!common.isWindows) {
return str;
}
const currentDriveLetter = path.parse(process.cwd()).root.substring(0, 1).toLowerCase();
const regex = new RegExp(`${currentDriveLetter}:`, 'gi');
return str.replaceAll(regex, '');
}

function transformCwd(replacement = '') {
const cwd = process.cwd();
return (str) => {
return str.replaceAll(cwd, replacement);
};
}

function transform(...args) {
Expand Down Expand Up @@ -87,11 +108,14 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
module.exports = {
assertSnapshot,
getSnapshotPath,
replaceFullPaths,
replaceExperimentalWarning,
replaceNodeVersion,
replaceStackTrace,
replaceInternalStackTrace,
replaceWindowsLineEndings,
replaceWindowsPaths,
replaceWindowsDriveLetter,
spawnAndAssert,
transform,
transformCwd,
};
20 changes: 10 additions & 10 deletions test/fixtures/source-map/output/source_map_disabled_by_api.snapshot
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
Error: an error!
at functionD (*enclosing-call-site-min.js:1:156)
at functionC (*enclosing-call-site-min.js:1:97)
at functionB (*enclosing-call-site-min.js:1:60)
at functionA (*enclosing-call-site-min.js:1:26)
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
Error: an error!
at functionD (*enclosing-call-site.js:16:17)
at functionC (*enclosing-call-site.js:10:3)
at functionB (*enclosing-call-site.js:6:3)
at functionA (*enclosing-call-site.js:2:3)
at Object.<anonymous> (*enclosing-call-site.js:24:3)
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
20 changes: 10 additions & 10 deletions test/fixtures/source-map/output/source_map_enabled_by_api.snapshot
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
Error: an error!
at functionD (*enclosing-call-site.js:16:17)
at functionC (*enclosing-call-site.js:10:3)
at functionB (*enclosing-call-site.js:6:3)
at functionA (*enclosing-call-site.js:2:3)
at Object.<anonymous> (*enclosing-call-site.js:24:3)
at functionD (*/test/fixtures/source-map/enclosing-call-site.js:16:17)
at functionC (*/test/fixtures/source-map/enclosing-call-site.js:10:3)
at functionB (*/test/fixtures/source-map/enclosing-call-site.js:6:3)
at functionA (*/test/fixtures/source-map/enclosing-call-site.js:2:3)
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site.js:24:3)
Error: an error!
at functionD (*enclosing-call-site-min.js:1:156)
at functionC (*enclosing-call-site-min.js:1:97)
at functionB (*enclosing-call-site-min.js:1:60)
at functionA (*enclosing-call-site-min.js:1:26)
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
at functionD (*/test/fixtures/source-map/enclosing-call-site-min.js:1:156)
at functionC (*/test/fixtures/source-map/enclosing-call-site-min.js:1:97)
at functionB (*/test/fixtures/source-map/enclosing-call-site-min.js:1:60)
at functionA (*/test/fixtures/source-map/enclosing-call-site-min.js:1:26)
at Object.<anonymous> (*/test/fixtures/source-map/enclosing-call-site-min.js:1:199)
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
*enclosing-call-site.js:26
*/test/fixtures/source-map/enclosing-call-site.js:26
throw err
^


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

Node.js *
1 change: 1 addition & 0 deletions test/fixtures/source-map/output/source_map_eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ Error.stackTraceLimit = 3;
const fs = require('fs');

const content = fs.readFileSync(require.resolve('../tabs.js'), 'utf8');
// SourceURL magic comment is hardcoded in the source content.
eval(content);
8 changes: 4 additions & 4 deletions test/fixtures/source-map/output/source_map_eval.snapshot
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
*tabs.coffee:26
/synthethized/workspace/tabs.coffee:26
alert "I knew it!"
^


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

Node.js *
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
*no-source.js:2
*/test/fixtures/source-map/no-source.js:2
throw new Error('foo');
^

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

Node.js *
Loading

0 comments on commit 44aa272

Please sign in to comment.