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: #43875Fixes: #43186Fixes: #41541
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: legendecas

.eslintignore

@@ -6,6 +6,7 @@ test/message/esm_display_syntax_error.mjs
 tools/icu
 tools/lint-md/lint-md.mjs
 benchmark/tmp
+benchmark/fixtures
 doc/**/*.js
 !doc/api_assets/*.js
 !.eslintrc.js

benchmark/es/error-stack.js

@@ -0,0 +1,34 @@
+'use strict';
+
+constcommon=require('../common.js');
+constmodPath=require.resolve('../fixtures/simple-error-stack.js');
+
+constbench=common.createBenchmark(main,{
+method: ['without-sourcemap','sourcemap'],
+n: [1e5],
+});
+
+functionrunN(n){
+deleterequire.cache[modPath];
+constmod=require(modPath);
+bench.start();
+for(leti=0;i<n;i++){
+mod.simpleErrorStack();
+}
+bench.end(n);
+}
+
+functionmain({ n, method }){
+switch(method){
+case'without-sourcemap':
+process.setSourceMapsEnabled(false);
+runN(n);
+break;
+case'sourcemap':
+process.setSourceMapsEnabled(true);
+runN(n);
+break;
+default:
+thrownewError(`Unexpected method "${method}"`);
+}
+}

benchmark/fixtures/simple-error-stack.js

@@ -0,0 +1,17 @@
+'use strict';
+
+// Compile with `tsc --inlineSourceMap benchmark/fixtures/simple-error-stack.ts`.
+
+constlorem='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.';
+
+functionsimpleErrorStack(){
+try{
+(loremasany).BANG();
+}catch(e){
+returne.stack;
+}
+}
+
+export{
+simpleErrorStack,
+};

benchmark/fixtures/simple-error-stack.ts

@@ -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.
@@ -53,7 +54,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
 returnerrorString;
 }
 
-leterrorSource='';
 letlastSourceMap;
 letlastFileName;
 constpreparedTrace=ArrayPrototypeJoin(ArrayPrototypeMap(trace,(t,i)=>{
@@ -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:
 letfileName=t.getFileName();
-letgenerated=false;
 if(fileName===undefined){
 fileName=t.getEvalOrigin();
-generated=true;
 }
 constsm=fileName===lastFileName ?
 lastSourceMap :
-findSourceMap(fileName,generated);
+findSourceMap(fileName);
 lastSourceMap=sm;
 lastFileName=fileName;
 if(sm){
@@ -83,14 +81,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
 if(originalSource&&originalLine!==undefined&&
 originalColumn!==undefined){
 constname=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:
 constfnName=t.getFunctionName()??t.getMethodName();
 consttypeName=t.getTypeName();
@@ -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
@@ -155,7 +145,7 @@ function getErrorSource(
 fileURLToPath(originalSourcePath) : originalSourcePath;
 constsource=getOriginalSource(
 sourceMap.payload,
-originalSourcePathNoScheme
+originalSourcePath
 );
 constlines=RegExpPrototypeSymbolSplit(/\r?\n/,source,originalLine+1);
 constline=lines[originalLine];
@@ -178,28 +168,46 @@ function getErrorSource(
 
 functiongetOriginalSource(payload,originalSourcePath){
 letsource;
-constoriginalSourcePathNoScheme=
-StringPrototypeStartsWith(originalSourcePath,'file://') ?
-fileURLToPath(originalSourcePath) : originalSourcePath;
+// payload.sources has been normalized to be an array of absolute urls.
 constsourceContentIndex=
 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{
+}elseif(StringPrototypeStartsWith(originalSourcePath,'file://')){
 // If no sourcesContent was found, attempt to load the original source
 // from disk:
+debug(`read source of ${originalSourcePath} from filesystem`);
+constoriginalSourcePathNoScheme=fileURLToPath(originalSourcePath);
 try{
 source=readFileSync(originalSourcePathNoScheme,'utf8');
 }catch(err){
 debug(err);
 source='';
 }
+}else{
+source='';
 }
 returnsource;
 }
 
+functiongetSourceMapErrorSource(fileName,lineNumber,columnNumber){
+constsm=findSourceMap(fileName);
+if(sm===null){
+return;
+}
+const{
+ originalLine,
+ originalColumn,
+ originalSource,
+}=sm.findEntry(lineNumber-1,columnNumber);
+consterrorSource=getErrorSource(sm,originalSource,originalLine,originalColumn);
+returnerrorSource;
+}
+
+setGetSourceMapErrorSource(getSourceMapErrorSource);
+
 module.exports={
  prepareStackTrace,
 };

lib/internal/source_map/prepare_stack_trace.js

@@ -41,6 +41,8 @@ const esmSourceMapCache = new SafeMap();
 // The generated sources is not mutable, so we can use a Map without memory concerns:
 constgeneratedSourceMapCache=newSafeMap();
 constkLeadingProtocol=/^\w+:\/\//;
+constkSourceMappingURLMagicComment=/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/;
+constkSourceURLMagicComment=/\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/;
 
 const{ fileURLToPath, pathToFileURL,URL}=require('internal/url');
 letSourceMap;
@@ -77,7 +79,22 @@ function setSourceMapsEnabled(val) {
 sourceMapsEnabled=val;
 }
 
-functionmaybeCacheSourceMap(filename,content,cjsModuleInstance,isGeneratedSource){
+functionextractSourceURLMagicComment(content){
+constmatchSourceURL=RegExpPrototypeExec(
+kSourceURLMagicComment,
+content
+);
+if(matchSourceURL===null){
+returnnull;
+}
+letsourceURL=matchSourceURL.groups.sourceURL;
+if(sourceURL!=null&&RegExpPrototypeExec(kLeadingProtocol,sourceURL)===null){
+sourceURL=pathToFileURL(sourceURL).href;
+}
+returnsourceURL;
+}
+
+functionmaybeCacheSourceMap(filename,content,cjsModuleInstance,isGeneratedSource,sourceURL){
 constsourceMapsEnabled=getSourceMapsEnabled();
 if(!(process.env.NODE_V8_COVERAGE||sourceMapsEnabled))return;
 try{
@@ -87,10 +104,10 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
 debug(err);
 return;
 }
-constmatch=RegExpPrototypeExec(
-/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/,
-content,
-);
+constmatch=RegExpPrototypeExec(kSourceMappingURLMagicComment,content);
+if(sourceURL===undefined){
+sourceURL=extractSourceURLMagicComment(content);
+}
 if(match){
 constdata=dataFromUrl(filename,match.groups.sourceMappingURL);
 consturl=data ? null : match.groups.sourceMappingURL;
@@ -99,22 +116,33 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSo
  filename,
 lineLengths: lineLengths(content),
  data,
- url
+ url,
+ sourceURL,
 });
 }elseif(isGeneratedSource){
-generatedSourceMapCache.set(filename,{
+constentry={
 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,{
+constentry={
 lineLengths: lineLengths(content),
  data,
- url
-});
+ url,
+ sourceURL,
+};
+esmSourceMapCache.set(filename,entry);
+if(sourceURL){
+esmSourceMapCache.set(sourceURL,entry);
+}
 }
 }
 }
@@ -123,19 +151,12 @@ function maybeCacheGeneratedSourceMap(content) {
 constsourceMapsEnabled=getSourceMapsEnabled();
 if(!(process.env.NODE_V8_COVERAGE||sourceMapsEnabled))return;
 
-constmatchSourceURL=RegExpPrototypeExec(
-/\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/,
-content
-);
-if(matchSourceURL==null){
+constsourceURL=extractSourceURLMagicComment(content);
+if(sourceURL===null){
 return;
 }
-letsourceURL=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.
@@ -254,33 +275,29 @@ function appendCJSCache(obj) {
 }
 }
 
-functionfindSourceMap(sourceURL,isGenerated){
+functionfindSourceMap(sourceURL){
 if(RegExpPrototypeExec(kLeadingProtocol,sourceURL)===null){
 sourceURL=pathToFileURL(sourceURL).href;
 }
 if(!SourceMap){
 SourceMap=require('internal/source_map/source_map').SourceMap;
 }
-letsourceMap;
-if(isGenerated){
-sourceMap=generatedSourceMapCache.get(sourceURL);
-}else{
-sourceMap=esmSourceMapCache.get(sourceURL);
-if(sourceMap===undefined){
-for(constvalueofcjsSourceMapCache){
-constfilename=ObjectGetValueSafe(value,'filename');
-if(sourceURL===filename){
-sourceMap={
-data: ObjectGetValueSafe(value,'data')
-};
-}
+letsourceMap=esmSourceMapCache.get(sourceURL)??generatedSourceMapCache.get(sourceURL);
+if(sourceMap===undefined){
+for(constvalueofcjsSourceMapCache){
+constfilename=ObjectGetValueSafe(value,'filename');
+constcachedSourceURL=ObjectGetValueSafe(value,'sourceURL');
+if(sourceURL===filename||sourceURL===cachedSourceURL){
+sourceMap={
+data: ObjectGetValueSafe(value,'data')
+};
 }
 }
 }
 if(sourceMap&&sourceMap.data){
 returnnewSourceMap(sourceMap.data);
 }
-returnundefined;
+returnnull;
 }
 
 module.exports={

lib/internal/source_map/source_map_cache.js

@@ -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) \