Skip to content

Commit

Permalink
lib: expose default prepareStackTrace
Browse files Browse the repository at this point in the history
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: #50827
Fixes: #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>
  • Loading branch information
legendecas authored and nodejs-github-bot committed Dec 21, 2023
1 parent 2cc6a0c commit 147abb9
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 56 deletions.
15 changes: 13 additions & 2 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 11 additions & 2 deletions lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,17 +445,26 @@ function setupPrepareStackTrace() {
setPrepareStackTraceCallback,
} = internalBinding('errors');
const {
prepareStackTrace,
prepareStackTraceCallback,
ErrorPrepareStackTrace,
fatalExceptionStackEnhancers: {
beforeInspector,
afterInspector,
},
} = 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++.
Expand Down
71 changes: 50 additions & 21 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -1055,10 +1083,11 @@ module.exports = {
isStackOverflowError,
kEnhanceStackBeforeInspector,
kIsNodeError,
kNoOverride,
maybeOverridePrepareStackTrace,
defaultPrepareStackTrace,
setInternalPrepareStackTrace,
overrideStackTrace,
prepareStackTrace,
prepareStackTraceCallback,
ErrorPrepareStackTrace,
setArrowMessage,
SystemError,
uvErrmapGet,
Expand Down
26 changes: 5 additions & 21 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,14 @@ 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');
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}`;
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -210,5 +194,5 @@ function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
setGetSourceMapErrorSource(getSourceMapErrorSource);

module.exports = {
prepareStackTrace,
prepareStackTraceWithSourceMaps,
};
12 changes: 7 additions & 5 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ const {
},
isErrorStackTraceLimitWritable,
overrideStackTrace,
ErrorPrepareStackTrace,
} = require('internal/errors');
const { sendInspectorCommand } = require('internal/util/inspector');
const { getOptionValue } = require('internal/options');
Expand Down Expand Up @@ -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);

Expand Down
34 changes: 34 additions & 0 deletions test/fixtures/source-map/output/source_map_prepare_stack_trace.js
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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)
16 changes: 14 additions & 2 deletions test/parallel/test-error-prepare-stack-trace.js
Original file line number Diff line number Diff line change
@@ -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) => {
Expand All @@ -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'],
{});
}
1 change: 1 addition & 0 deletions test/parallel/test-node-output-sourcemaps.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down

0 comments on commit 147abb9

Please sign in to comment.