Skip to content

Commit

Permalink
src,lib: print source map error source on demand
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas committed Jul 17, 2022
1 parent 71ca6d7 commit 6a97b7f
Show file tree
Hide file tree
Showing 14 changed files with 149 additions and 82 deletions.
44 changes: 26 additions & 18 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
kIsNodeError,
} = require('internal/errors');
const { fileURLToPath } = require('internal/url');
const { setGetSourceMapErrorSource } = internalBinding('errors');

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

let errorSource = '';
let lastSourceMap;
let lastFileName;
const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => {
Expand All @@ -62,14 +62,12 @@ const prepareStackTrace = (globalThis, error, trace) => {
// A stack trace will often have several call sites in a row within the
// same file, cache the source map and file content accordingly:
let fileName = t.getFileName();
let generated = false;
if (fileName === undefined) {
fileName = t.getEvalOrigin();
generated = true;
}
const sm = fileName === lastFileName ?
lastSourceMap :
findSourceMap(fileName, generated);
findSourceMap(fileName);
lastSourceMap = sm;
lastFileName = fileName;
if (sm) {
Expand All @@ -83,14 +81,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
if (originalSource && originalLine !== undefined &&
originalColumn !== undefined) {
const name = getOriginalSymbolName(sm, trace, i);
if (i === 0) {
errorSource = getErrorSource(
sm,
originalSource,
originalLine,
originalColumn
);
}
// Construct call site name based on: v8.dev/docs/stack-trace-api:
const fnName = t.getFunctionName() ?? t.getMethodName();
const typeName = t.getTypeName();
Expand All @@ -116,7 +106,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
}
return `${str}${t}`;
}), '');
return `${errorSource}${errorString}\n at ${preparedTrace}`;
return `${errorString}\n at ${preparedTrace}`;
};

// Transpilers may have removed the original symbol name used in the stack
Expand Down Expand Up @@ -155,7 +145,7 @@ function getErrorSource(
fileURLToPath(originalSourcePath) : originalSourcePath;
const source = getOriginalSource(
sourceMap.payload,
originalSourcePathNoScheme
originalSourcePath
);
const lines = RegExpPrototypeSymbolSplit(/\r?\n/, source, originalLine + 1);
const line = lines[originalLine];
Expand All @@ -178,28 +168,46 @@ function getErrorSource(

function getOriginalSource(payload, originalSourcePath) {
let source;
const originalSourcePathNoScheme =
StringPrototypeStartsWith(originalSourcePath, 'file://') ?
fileURLToPath(originalSourcePath) : originalSourcePath;
// payload.sources has been normalized to be an array of absolute urls.
const sourceContentIndex =
ArrayPrototypeIndexOf(payload.sources, originalSourcePath);
if (payload.sourcesContent?.[sourceContentIndex]) {
// First we check if the original source content was provided in the
// source map itself:
source = payload.sourcesContent[sourceContentIndex];
} else {
} else if (StringPrototypeStartsWith(originalSourcePath, 'file://')) {
// If no sourcesContent was found, attempt to load the original source
// from disk:
debug(`read source of ${originalSourcePath} from filesystem`);
const originalSourcePathNoScheme = fileURLToPath(originalSourcePath);
try {
source = readFileSync(originalSourcePathNoScheme, 'utf8');
} catch (err) {
debug(err);
source = '';
}
} else {
source = '';
}
return source;
}

function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
const sm = findSourceMap(fileName);
if (sm == null) {
return;
}
const {
originalLine,
originalColumn,
originalSource,
} = sm.findEntry(lineNumber - 1, columnNumber);
const errorSource = getErrorSource(sm, originalSource, originalLine, originalColumn);
return errorSource;
}

setGetSourceMapErrorSource(getSourceMapErrorSource);

module.exports = {
prepareStackTrace,
};
83 changes: 51 additions & 32 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const esmSourceMapCache = new SafeMap();
// The generated sources is not mutable, so we can use a Map without memory concerns:
const generatedSourceMapCache = new SafeMap();
const kLeadingProtocol = /^\w+:\/\//;
const kSourceURLMagicComment = /\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/;

const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
let SourceMap;
Expand Down Expand Up @@ -77,7 +78,22 @@ function setSourceMapsEnabled(val) {
sourceMapsEnabled = val;
}

function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource) {
function extractSourceURLMagicComment(content) {
const matchSourceURL = RegExpPrototypeExec(
kSourceURLMagicComment,
content
);
if (matchSourceURL == null) {
return null;
}
let sourceURL = matchSourceURL.groups.sourceURL;
if (sourceURL != null && RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
sourceURL = pathToFileURL(sourceURL).href;
}
return sourceURL;
}

function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL) {
const sourceMapsEnabled = getSourceMapsEnabled();
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
try {
Expand All @@ -91,6 +107,9 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/,
content,
);
if (sourceURL == null) {
sourceURL = extractSourceURLMagicComment(content);
}
if (match) {
const data = dataFromUrl(filename, match.groups.sourceMappingURL);
const url = data ? null : match.groups.sourceMappingURL;
Expand All @@ -99,22 +118,33 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
filename,
lineLengths: lineLengths(content),
data,
url
url,
sourceURL,
});
} else if (isGeneratedSource) {
generatedSourceMapCache.set(filename, {
const entry = {
lineLengths: lineLengths(content),
data,
url
});
url,
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.
esmSourceMapCache.set(filename, {
const entry = {
lineLengths: lineLengths(content),
data,
url
});
url,
sourceURL,
};
esmSourceMapCache.set(filename, entry);
if (sourceURL) {
esmSourceMapCache.set(sourceURL, entry);
}
}
}
}
Expand All @@ -123,19 +153,12 @@ function maybeCacheGeneratedSourceMap(content) {
const sourceMapsEnabled = getSourceMapsEnabled();
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;

const matchSourceURL = RegExpPrototypeExec(
/\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/,
content
);
if (matchSourceURL == null) {
const sourceURL = extractSourceURLMagicComment(content);
if (sourceURL == null) {
return;
}
let sourceURL = matchSourceURL.groups.sourceURL;
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
sourceURL = pathToFileURL(sourceURL).href;
}
try {
maybeCacheSourceMap(sourceURL, content, null, true);
maybeCacheSourceMap(sourceURL, content, null, true, sourceURL);
} 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 Expand Up @@ -254,26 +277,22 @@ function appendCJSCache(obj) {
}
}

function findSourceMap(sourceURL, isGenerated) {
function findSourceMap(sourceURL) {
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
sourceURL = pathToFileURL(sourceURL).href;
}
if (!SourceMap) {
SourceMap = require('internal/source_map/source_map').SourceMap;
}
let sourceMap;
if (isGenerated) {
sourceMap = generatedSourceMapCache.get(sourceURL);
} else {
sourceMap = esmSourceMapCache.get(sourceURL);
if (sourceMap === undefined) {
for (const value of cjsSourceMapCache) {
const filename = ObjectGetValueSafe(value, 'filename');
if (sourceURL === filename) {
sourceMap = {
data: ObjectGetValueSafe(value, 'data')
};
}
let sourceMap = esmSourceMapCache.get(sourceURL) ?? generatedSourceMapCache.get(sourceURL);
if (sourceMap === undefined) {
for (const value of cjsSourceMapCache) {
const filename = ObjectGetValueSafe(value, 'filename');
const cachedSourceURL = ObjectGetValueSafe(value, 'sourceURL');
if (sourceURL === filename || sourceURL === cachedSourceURL) {
sourceMap = {
data: ObjectGetValueSafe(value, 'data')
};
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ class NoArrayBufferZeroFillScope {
V(enhance_fatal_stack_after_inspector, v8::Function) \
V(enhance_fatal_stack_before_inspector, v8::Function) \
V(fs_use_promises_symbol, v8::Symbol) \
V(get_source_map_error_source, v8::Function) \
V(host_import_module_dynamically_callback, v8::Function) \
V(host_initialize_import_meta_object_callback, v8::Function) \
V(http2session_on_altsvc_function, v8::Function) \
Expand Down
56 changes: 51 additions & 5 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,40 @@ namespace per_process {
static Mutex tty_mutex;
} // namespace per_process

static std::string GetSourceMapErrorSource(Isolate* isolate,
Local<Context> context,
Local<Message> message,
bool* added_exception_line) {
v8::TryCatch try_catch(isolate);
HandleScope handle_scope(isolate);
Environment* env = Environment::GetCurrent(context);

// 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
// the source texts.
Local<Value> script_resource_name = message->GetScriptResourceName();
int linenum = message->GetLineNumber(context).FromJust();
int columnum = message->GetStartColumn(context).FromJust();

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);
Local<Value> ret;
if (!maybe_ret.ToLocal(&ret)) {
// Ignore the caught exceptions.
DCHECK(try_catch.HasCaught());
return std::string();
}
if (!ret->IsString()) {
return std::string();
}
*added_exception_line = true;
node::Utf8Value error_source_utf8(isolate, ret.As<String>());
return *error_source_utf8;
}

static std::string GetErrorSource(Isolate* isolate,
Local<Context> context,
Local<Message> message,
Expand All @@ -58,17 +92,19 @@ static std::string GetErrorSource(Isolate* isolate,
std::string sourceline(*encoded_source, encoded_source.length());
*added_exception_line = false;

if (sourceline.find("node-do-not-add-exception-line") != std::string::npos) {
return sourceline;
}

// If source maps have been enabled, the exception line will instead be
// added in the JavaScript context:
Environment* env = Environment::GetCurrent(isolate);
const bool has_source_map_url =
!message->GetScriptOrigin().SourceMapUrl().IsEmpty();
if (has_source_map_url && env != nullptr && env->source_maps_enabled()) {
return sourceline;
}

if (sourceline.find("node-do-not-add-exception-line") != std::string::npos) {
return sourceline;
std::string source = GetSourceMapErrorSource(
isolate, context, message, added_exception_line);
return added_exception_line ? source : sourceline;
}

// Because of how node modules work, all scripts are wrapped with a
Expand Down Expand Up @@ -868,6 +904,13 @@ static void SetSourceMapsEnabled(const FunctionCallbackInfo<Value>& args) {
env->set_source_maps_enabled(args[0].As<Boolean>()->Value());
}

static void SetGetSourceMapErrorSource(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK(args[0]->IsFunction());
env->set_get_source_map_error_source(args[0].As<Function>());
}

static void SetMaybeCacheGeneratedSourceMap(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand Down Expand Up @@ -908,6 +951,7 @@ static void TriggerUncaughtException(const FunctionCallbackInfo<Value>& args) {

void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(SetPrepareStackTraceCallback);
registry->Register(SetGetSourceMapErrorSource);
registry->Register(SetSourceMapsEnabled);
registry->Register(SetMaybeCacheGeneratedSourceMap);
registry->Register(SetEnhanceStackForFatalException);
Expand All @@ -922,6 +966,8 @@ void Initialize(Local<Object> target,
Environment* env = Environment::GetCurrent(context);
env->SetMethod(
target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback);
env->SetMethod(
target, "setGetSourceMapErrorSource", SetGetSourceMapErrorSource);
env->SetMethod(target, "setSourceMapsEnabled", SetSourceMapsEnabled);
env->SetMethod(target,
"setMaybeCacheGeneratedSourceMap",
Expand Down
6 changes: 5 additions & 1 deletion test/message/source_map_disabled_by_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,8 @@ delete require.cache[require
// Re-enable.
process.setSourceMapsEnabled(true);

require('../fixtures/source-map/enclosing-call-site-min.js');
try {
require('../fixtures/source-map/enclosing-call-site-min.js');
} catch (e) {
console.log(e);
}
6 changes: 0 additions & 6 deletions test/message/source_map_disabled_by_api.out
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ Error: an error!
at Module.load (node:internal/modules/cjs/loader:*)
at Module._load (node:internal/modules/cjs/loader:*)
at Module.require (node:internal/modules/cjs/loader:*)
*enclosing-call-site.js:16
throw new Error('an error!')
^

Error: an error!
at functionD (*enclosing-call-site.js:16:17)
at functionC (*enclosing-call-site.js:10:3)
Expand All @@ -24,5 +20,3 @@ Error: an error!
at Module.load (node:internal/modules/cjs/loader:*)
at Module._load (node:internal/modules/cjs/loader:*)
at Module.require (node:internal/modules/cjs/loader:*)

Node.js *

0 comments on commit 6a97b7f

Please sign in to comment.