refactor: refactor approch based on research

nodejs · bcoe · Nov 8, 2020 · Nov 27, 2020 · Nov 27, 2020 · Nov 27, 2020
commit f46eb0c57e07537478add3e9f4b287924ad71073
diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js
@@ -47,37 +47,37 @@ const prepareStackTrace = (globalThis, error, trace) => {
   }
 
   let errorSource = '';
-  let firstLine;
-  let firstColumn;
+  let lastSourceMap;
+  let lastFileName;
   const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => {
-    if (i === 0) {
-      firstLine = t.getLineNumber();
-      firstColumn = t.getColumnNumber();
-    }
     let str = i !== 0 ? '\n    at ' : '';
     str = `${str}${t}`;
     try {
-      const sm = findSourceMap(t.getFileName());
+      // A stack trace will often have several call sites in a row within the
+      // same file, cache the source map and file content accordingly:
+      const fileName = t.getFileName();
+      const sm = fileName === lastFileName ?
+        lastSourceMap :
+        findSourceMap(fileName);
+      lastSourceMap = sm;
+      lastFileName = fileName;
       if (sm) {
         // Source Map V3 lines/columns use zero-based offsets whereas, in
         // stack traces, they start at 1/1.
         const {
           originalLine,
           originalColumn,
           originalSource,
-          name
         } = sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
         if (originalSource && originalLine !== undefined &&
             originalColumn !== undefined) {
+          const name = getOriginalSymbolName(sm, trace, i);
           if (i === 0) {
-            firstLine = originalLine + 1;
-            firstColumn = originalColumn + 1;
-            // Show error in original source context to help user pinpoint it:
             errorSource = getErrorSource(
-              sm.payload,
+              sm,
               originalSource,
-              firstLine,
-              firstColumn
+              originalLine,
+              originalColumn
             );
           }
           // Show both original and transpiled stack trace information:
@@ -97,18 +97,69 @@ const prepareStackTrace = (globalThis, error, trace) => {
   return `${errorSource}${errorString}\n    at ${preparedTrace}`;
 };
 
+// Transpilers may have removed the original symbol name used in the stack
+// trace, if possible restore it from the source map:
+function getOriginalSymbolName(sourceMap, trace, curIndex) {
+  // First check for a symbol name associated with the enclosing function:
+  const enclosingEntry = sourceMap.findEntry(
+    trace[curIndex].getEnclosingLineNumber() - 1,
+    trace[curIndex].getEnclosingColumnNumber() - 1
+  );
+  if (enclosingEntry.name) return enclosingEntry.name;
+  // Fallback to using the symbol name attached to the next stack frame:
+  const currentFileName = trace[curIndex].getFileName();
+  const nextCallSite = trace[curIndex + 1];
+  if (nextCallSite && currentFileName === nextCallSite.getFileName()) {
+    const { name } = sourceMap.findEntry(
+      nextCallSite.getLineNumber() - 1,
+      nextCallSite.getColumnNumber() - 1
+    );
+    return name;
+  }
+}
+
 // Places a snippet of code from where the exception was originally thrown
 // above the stack trace. This logic is modeled after GetErrorSource in
 // node_errors.cc.
-function getErrorSource(payload, originalSource, firstLine, firstColumn) {
+function getErrorSource(
+  sourceMap,
+  originalSourcePath,
+  originalLine,
+  originalColumn
+) {
   let exceptionLine = '';
-  const originalSourceNoScheme =
-    StringPrototypeStartsWith(originalSource, 'file://') ?
-      fileURLToPath(originalSource) : originalSource;
+  const originalSourcePathNoScheme =
+    StringPrototypeStartsWith(originalSourcePath, 'file://') ?
+      fileURLToPath(originalSourcePath) : originalSourcePath;
+  const source = getOriginalSource(
+    sourceMap.payload,
+    originalSourcePathNoScheme
+  );
+  const lines = StringPrototypeSplit(source, /\r?\n/, originalLine + 1);
+  const line = lines[originalLine];
+  if (!line) return exceptionLine;
+
+  // Display ^ in appropriate position, regardless of whether tabs or
+  // spaces are used:
+  let prefix = '';
+  for (const character of StringPrototypeSlice(line, 0, originalColumn + 1)) {
+    prefix += (character === '\t') ? '\t' :
+      StringPrototypeRepeat(' ', getStringWidth(character));
+  }
+  prefix = StringPrototypeSlice(prefix, 0, -1); // The last character is '^'.
+
+  exceptionLine =
+   `${originalSourcePathNoScheme}:${originalLine + 1}\n${line}\n${prefix}^\n\n`;
+  return exceptionLine;
+}
 
+function getOriginalSource(payload, originalSourcePath) {
   let source;
+  const originalSourcePathNoScheme =
+    StringPrototypeStartsWith(originalSourcePath, 'file://') ?
+      fileURLToPath(originalSourcePath) : originalSourcePath;
   const sourceContentIndex =
-    ArrayPrototypeIndexOf(payload.sources, originalSource);
+    ArrayPrototypeIndexOf(payload.sources, originalSourcePath);
   if (payload.sourcesContent?.[sourceContentIndex]) {
     // First we check if the original source content was provided in the
     // source map itself:
@@ -117,29 +168,13 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
     // If no sourcesContent was found, attempt to load the original source
     // from disk:
     try {
-      source = readFileSync(originalSourceNoScheme, 'utf8');
+      source = readFileSync(originalSourcePathNoScheme, 'utf8');
     } catch (err) {
       debug(err);
-      return '';
+      source = '';
     }
   }
-
-  const lines = StringPrototypeSplit(source, /\r?\n/, firstLine);
-  const line = lines[firstLine - 1];
-  if (!line) return exceptionLine;
-
-  // Display ^ in appropriate position, regardless of whether tabs or
-  // spaces are used:
-  let prefix = '';
-  for (const character of StringPrototypeSlice(line, 0, firstColumn)) {
-    prefix += (character === '\t') ? '\t' :
-      StringPrototypeRepeat(' ', getStringWidth(character));
-  }
-  prefix = StringPrototypeSlice(prefix, 0, -1); // The last character is '^'.
-
-  exceptionLine =
-              `${originalSourceNoScheme}:${firstLine}\n${line}\n${prefix}^\n\n`;
-  return exceptionLine;
+  return source;
 }
 
 module.exports = {

diff --git a/lib/internal/source_map/source_map.js b/lib/internal/source_map/source_map.js
@@ -231,8 +231,6 @@ class SourceMap {
 
     const stringCharIterator = new StringCharIterator(map.mappings);
     let sourceURL = sources[sourceIndex];
-    let name = map.names?.[nameIndex];
-
     while (true) {
       if (stringCharIterator.peek() === ',')
         stringCharIterator.next();
@@ -259,6 +257,8 @@ class SourceMap {
       }
       sourceLineNumber += decodeVLQ(stringCharIterator);
       sourceColumnNumber += decodeVLQ(stringCharIterator);
+
+      let name;
       if (!isSeparator(stringCharIterator.peek())) {
         nameIndex += decodeVLQ(stringCharIterator);
         name = map.names?.[nameIndex];