lib: expose default prepareStackTrace

Expose the default prepareStackTrace implementation as
`Error.prepareStackTrace` so that userland can chain up formatting of
stack traces with built-in source maps support.

PR-URL: https://github.com/nodejs/node/pull/50827
Fixes: https://github.com/nodejs/node/issues/50733
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

doc/api/cli.md

@@ -598,8 +598,19 @@ application reference the transpiled code, not the original source position.
 `--enable-source-maps` enables caching of Source Maps and makes a best
 effort to report stack traces relative to the original source file.
 
-Overriding `Error.prepareStackTrace` prevents `--enable-source-maps` from
-modifying the stack trace.
+Overriding `Error.prepareStackTrace` may prevent `--enable-source-maps` from
+modifying the stack trace. Call and return the results of the original
+`Error.prepareStackTrace` in the overriding function to modify the stack trace
+with source maps.
+
+```js
+const originalPrepareStackTrace = Error.prepareStackTrace;
+Error.prepareStackTrace = (error, trace) => {
+  // Modify error and trace and format stack trace with
+  // original Error.prepareStackTrace.
+  return originalPrepareStackTrace(error, trace);
+};
+```
 
 Note, enabling source maps can introduce latency to your application
 when `Error.stack` is accessed. If you access `Error.stack` frequently

lib/.eslintrc.yaml

@@ -23,7 +23,7 @@ rules:
       message: Use an error exported by the internal/errors module.
     - selector: CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']
       message: Please use `require('internal/errors').hideStackFrames()` instead.
-    - selector: AssignmentExpression:matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])
+    - selector: AssignmentExpression:matches([left.object.name='Error']):matches([left.name='prepareStackTrace'], [left.property.name='prepareStackTrace'])
       message: Use 'overrideStackTrace' from 'lib/internal/errors.js' instead of 'Error.prepareStackTrace'.
     - selector: ThrowStatement > NewExpression[callee.name=/^ERR_[A-Z_]+$/] > ObjectExpression:first-child:not(:has([key.name='message']):has([key.name='code']):has([key.name='syscall']))
       message: The context passed into SystemError constructor must have .code, .syscall and .message.

lib/internal/bootstrap/realm.js

@@ -445,7 +445,8 @@ function setupPrepareStackTrace() {
     setPrepareStackTraceCallback,
   } = internalBinding('errors');
   const {
-    prepareStackTrace,
+    prepareStackTraceCallback,
+    ErrorPrepareStackTrace,
     fatalExceptionStackEnhancers: {
       beforeInspector,
       afterInspector,
@@ -453,9 +454,17 @@ function setupPrepareStackTrace() {
   } = requireBuiltin('internal/errors');
   // Tell our PrepareStackTraceCallback passed to the V8 API
   // to call prepareStackTrace().
-  setPrepareStackTraceCallback(prepareStackTrace);
+  setPrepareStackTraceCallback(prepareStackTraceCallback);
   // Set the function used to enhance the error stack for printing
   setEnhanceStackForFatalException(beforeInspector, afterInspector);
+  // Setup the default Error.prepareStackTrace.
+  ObjectDefineProperty(Error, 'prepareStackTrace', {
+    __proto__: null,
+    writable: true,
+    enumerable: false,
+    configurable: true,
+    value: ErrorPrepareStackTrace,
+  });
 }
 
 // Store the internal loaders in C++.

lib/internal/errors.js

@@ -85,21 +85,14 @@ const kTypes = [
 
 const MainContextError = Error;
 const overrideStackTrace = new SafeWeakMap();
-const kNoOverride = Symbol('kNoOverride');
-
-const prepareStackTrace = (globalThis, error, trace) => {
-  // API for node internals to override error stack formatting
-  // without interfering with userland code.
-  if (overrideStackTrace.has(error)) {
-    const f = overrideStackTrace.get(error);
-    overrideStackTrace.delete(error);
-    return f(error, trace);
-  }
-
-  const globalOverride =
-    maybeOverridePrepareStackTrace(globalThis, error, trace);
-  if (globalOverride !== kNoOverride) return globalOverride;
+let internalPrepareStackTrace = defaultPrepareStackTrace;
 
+/**
+ * The default implementation of `Error.prepareStackTrace` with simple
+ * concatenation of stack frames.
+ * Read more about `Error.prepareStackTrace` at https://v8.dev/docs/stack-trace-api#customizing-stack-traces.
+ */
+function defaultPrepareStackTrace(error, trace) {
   // Normal error formatting:
   //
   // Error: Message
@@ -115,9 +108,35 @@ const prepareStackTrace = (globalThis, error, trace) => {
     return errorString;
   }
   return `${errorString}\n    at ${ArrayPrototypeJoin(trace, '\n    at ')}`;
-};
+}
+
+function setInternalPrepareStackTrace(callback) {
+  internalPrepareStackTrace = callback;
+}
+
+/**
+ * Every realm has its own prepareStackTraceCallback. When `error.stack` is
+ * accessed, if the error is created in a shadow realm, the shadow realm's
+ * prepareStackTraceCallback is invoked. Otherwise, the principal realm's
+ * prepareStackTraceCallback is invoked. Note that accessing `error.stack`
+ * of error objects created in a VM Context will always invoke the
+ * prepareStackTraceCallback of the principal realm.
+ * @param {object} globalThis The global object of the realm that the error was
+ *   created in. When the error object is created in a VM Context, this is the
+ *   global object of that VM Context.
+ * @param {object} error The error object.
+ * @param {CallSite[]} trace An array of CallSite objects, read more at https://v8.dev/docs/stack-trace-api#customizing-stack-traces.
+ * @returns {string}
+ */
+function prepareStackTraceCallback(globalThis, error, trace) {
+  // API for node internals to override error stack formatting
+  // without interfering with userland code.
+  if (overrideStackTrace.has(error)) {
+    const f = overrideStackTrace.get(error);
+    overrideStackTrace.delete(error);
+    return f(error, trace);
+  }
 
-const maybeOverridePrepareStackTrace = (globalThis, error, trace) => {
   // Polyfill of V8's Error.prepareStackTrace API.
   // https://crbug.com/v8/7848
   // `globalThis` is the global that contains the constructor which
@@ -132,8 +151,17 @@ const maybeOverridePrepareStackTrace = (globalThis, error, trace) => {
     return MainContextError.prepareStackTrace(error, trace);
   }
 
-  return kNoOverride;
-};
+  // If the Error.prepareStackTrace was not a function, fallback to the
+  // internal implementation.
+  return internalPrepareStackTrace(error, trace);
+}
+
+/**
+ * The default Error.prepareStackTrace implementation.
+ */
+function ErrorPrepareStackTrace(error, trace) {
+  return internalPrepareStackTrace(error, trace);
+}
 
 const aggregateTwoErrors = (innerError, outerError) => {
   if (innerError && outerError && innerError !== outerError) {
@@ -1055,10 +1083,11 @@ module.exports = {
   isStackOverflowError,
   kEnhanceStackBeforeInspector,
   kIsNodeError,
-  kNoOverride,
-  maybeOverridePrepareStackTrace,
+  defaultPrepareStackTrace,
+  setInternalPrepareStackTrace,
   overrideStackTrace,
-  prepareStackTrace,
+  prepareStackTraceCallback,
+  ErrorPrepareStackTrace,
   setArrowMessage,
   SystemError,
   uvErrmapGet,

lib/internal/source_map/prepare_stack_trace.js

@@ -19,9 +19,6 @@ const { getStringWidth } = require('internal/util/inspect');
 const { readFileSync } = require('fs');
 const { findSourceMap } = require('internal/source_map/source_map_cache');
 const {
-  kNoOverride,
-  overrideStackTrace,
-  maybeOverridePrepareStackTrace,
   kIsNodeError,
 } = require('internal/errors');
 const { fileURLToPath } = require('internal/url');
@@ -29,20 +26,7 @@ const { setGetSourceMapErrorSource } = internalBinding('errors');
 
 // Create a prettified stacktrace, inserting context from source maps
 // if possible.
-const prepareStackTrace = (globalThis, error, trace) => {
-  // API for node internals to override error stack formatting
-  // without interfering with userland code.
-  // TODO(bcoe): add support for source-maps to repl.
-  if (overrideStackTrace.has(error)) {
-    const f = overrideStackTrace.get(error);
-    overrideStackTrace.delete(error);
-    return f(error, trace);
-  }
-
-  const globalOverride =
-    maybeOverridePrepareStackTrace(globalThis, error, trace);
-  if (globalOverride !== kNoOverride) return globalOverride;
-
+function prepareStackTraceWithSourceMaps(error, trace) {
   let errorString;
   if (kIsNodeError in error) {
     errorString = `${error.name} [${error.code}]: ${error.message}`;
@@ -57,7 +41,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
   let lastSourceMap;
   let lastFileName;
   const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => {
-    const str = i !== 0 ? '\n    at ' : '';
+    const str = '\n    at ';
     try {
       // A stack trace will often have several call sites in a row within the
       // same file, cache the source map and file content accordingly:
@@ -106,8 +90,8 @@ const prepareStackTrace = (globalThis, error, trace) => {
     }
     return `${str}${t}`;
   }), '');
-  return `${errorString}\n    at ${preparedTrace}`;
-};
+  return `${errorString}${preparedTrace}`;
+}
 
 // Transpilers may have removed the original symbol name used in the stack
 // trace, if possible restore it from the names field of the source map:
@@ -210,5 +194,5 @@ function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
 setGetSourceMapErrorSource(getSourceMapErrorSource);
 
 module.exports = {
-  prepareStackTrace,
+  prepareStackTraceWithSourceMaps,
 };

lib/internal/source_map/source_map_cache.js

@@ -19,8 +19,10 @@ let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
 const { validateBoolean } = require('internal/validators');
 const {
   setSourceMapsEnabled: setSourceMapsNative,
-  setPrepareStackTraceCallback,
 } = internalBinding('errors');
+const {
+  setInternalPrepareStackTrace,
+} = require('internal/errors');
 const { getLazy } = require('internal/util');
 
 // Since the CJS module cache is mutable, which leads to memory leaks when
@@ -56,15 +58,15 @@ function setSourceMapsEnabled(val) {
   setSourceMapsNative(val);
   if (val) {
     const {
-      prepareStackTrace,
+      prepareStackTraceWithSourceMaps,
     } = require('internal/source_map/prepare_stack_trace');
-    setPrepareStackTraceCallback(prepareStackTrace);
+    setInternalPrepareStackTrace(prepareStackTraceWithSourceMaps);
   } else if (sourceMapsEnabled !== undefined) {
     // Reset prepare stack trace callback only when disabling source maps.
     const {
-      prepareStackTrace,
+      defaultPrepareStackTrace,
     } = require('internal/errors');
-    setPrepareStackTraceCallback(prepareStackTrace);
+    setInternalPrepareStackTrace(defaultPrepareStackTrace);
   }
 
   sourceMapsEnabled = val;

lib/repl.js

@@ -151,6 +151,7 @@ const {
   },
   isErrorStackTraceLimitWritable,
   overrideStackTrace,
+  ErrorPrepareStackTrace,
 } = require('internal/errors');
 const { sendInspectorCommand } = require('internal/util/inspector');
 const { getOptionValue } = require('internal/options');
@@ -692,8 +693,7 @@ function REPLServer(prompt,
         if (typeof MainContextError.prepareStackTrace === 'function') {
           return MainContextError.prepareStackTrace(error, frames);
         }
-        ArrayPrototypeUnshift(frames, error);
-        return ArrayPrototypeJoin(frames, '\n    at ');
+        return ErrorPrepareStackTrace(error, frames);
       });
       decorateErrorStack(e);
 

test/fixtures/source-map/output/source_map_prepare_stack_trace.js

@@ -0,0 +1,34 @@
+// Flags: --enable-source-maps
+
+'use strict';
+require('../../../common');
+const assert = require('assert');
+Error.stackTraceLimit = 5;
+
+assert.strictEqual(typeof Error.prepareStackTrace, 'function');
+const defaultPrepareStackTrace = Error.prepareStackTrace;
+Error.prepareStackTrace = (error, trace) => {
+  trace = trace.filter(it => {
+    return it.getFunctionName() !== 'functionC';
+  });
+  return defaultPrepareStackTrace(error, trace);
+};
+
+try {
+  require('../enclosing-call-site-min.js');
+} catch (e) {
+  console.log(e);
+}
+
+delete require.cache[require
+  .resolve('../enclosing-call-site-min.js')];
+
+// Disable
+process.setSourceMapsEnabled(false);
+assert.strictEqual(process.sourceMapsEnabled, false);
+
+try {
+  require('../enclosing-call-site-min.js');
+} catch (e) {
+  console.log(e);
+}

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

@@ -0,0 +1,10 @@
+Error: an error!
+    at functionD (*enclosing-call-site.js:16:17)
+    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)
+Error: an error!
+    at functionD (*enclosing-call-site-min.js:1:156)
+    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)

test/parallel/test-error-prepare-stack-trace.js

@@ -1,10 +1,13 @@
-// Flags: --enable-source-maps
 'use strict';
 
 require('../common');
+const { spawnSyncAndExitWithoutError } = require('../common/child_process');
 const assert = require('assert');
 
-// Error.prepareStackTrace() can be overridden with source maps enabled.
+// Verify that the default Error.prepareStackTrace is present.
+assert.strictEqual(typeof Error.prepareStackTrace, 'function');
+
+// Error.prepareStackTrace() can be overridden.
 {
   let prepareCalled = false;
   Error.prepareStackTrace = (_error, trace) => {
@@ -17,3 +20,12 @@ const assert = require('assert');
   }
   assert(prepareCalled);
 }
+
+if (process.argv[2] !== 'child') {
+  // Verify that the above test still passes when source-maps support is
+  // enabled.
+  spawnSyncAndExitWithoutError(
+    process.execPath,
+    ['--enable-source-maps', __filename, 'child'],
+    {});
+}

test/parallel/test-node-output-sourcemaps.mjs

@@ -31,6 +31,7 @@ describe('sourcemaps output', { concurrency: true }, () => {
     { name: 'source-map/output/source_map_enclosing_function.js' },
     { name: 'source-map/output/source_map_eval.js' },
     { name: 'source-map/output/source_map_no_source_file.js' },
+    { name: 'source-map/output/source_map_prepare_stack_trace.js' },
     { name: 'source-map/output/source_map_reference_error_tabs.js' },
     { name: 'source-map/output/source_map_sourcemapping_url_string.js' },
     { name: 'source-map/output/source_map_throw_catch.js' },