Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using --enable-source-maps together with custom Error.prepareStackTrace #50733

Closed
nicolo-ribaudo opened this issue Nov 14, 2023 · 3 comments
Labels
feature request Issues that request new features to be added to Node.js. source maps Issues and PRs related to source map support.

Comments

@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Nov 14, 2023

What is the problem this feature will solve?

Some libraries use Error.prepareStackTrace to make their errors more user-friendly. One example is Babel: when we throw an error due to user configs, we hide most of our internal stack frames to that our users can more easily find where they originally called Babel.

To make sure that our implementation works well together with other libraries that use Error.prepareStackTrace, we do something like this:

const { prepareStackTrace = defaultPrepareStackTrace } = Error;

Error.prepareStackTrace = function (error, frames) {
  updateTheFrames(frames);
  // Call the original stack trace formatter
  return prepareStackTrace(error, frames);
};

As the default defaultPrepareStackTrace we use the following, which matches Node.js' default behavior:

function defaultPrepareStackTrace(err: Error, trace: CallSite[]) {
  if (trace.length === 0) return ErrorToString(err);
  return `${ErrorToString(err)}\n    at ${trace.join("\n    at ")}`;
}

I am currently using Babel together with the source-map-support package, so that errors show their original source files in the stack traces rather than the compiled ones. This works well, because source-map-support simply installs a custom Error.prepareStackTrace that then Babel's Error.prepareStackTrace will use to format the error.

A couple days ago @joyeecheung introduced me to Node.js's --enable-source-maps flag (https://nodejs.org/api/cli.html#--enable-source-maps), which basically replaces the source-map-support package. I tried to use it, but I cannot because I'm using defaultPrepareStackTrace that mimics Node.js' built-in behavior, rather than it being what Node.js does.

What is the feature you are proposing to solve the problem?

Node.js should expose its default error formatter, maybe as require("util").prepareStackTrace or as a pre-defined Error.prepareStackTrace function.

This would allow some chaining similar to how ESM loaders can compose.

What alternatives have you considered?

I'm very open to any other solution that would allow me to write an Error.prepareStackTrace that works well together with --enable-source-maps.

@nicolo-ribaudo nicolo-ribaudo added the feature request Issues that request new features to be added to Node.js. label Nov 14, 2023
@legendecas legendecas added the source maps Issues and PRs related to source map support. label Nov 20, 2023
@legendecas
Copy link
Member

Thanks for reporting. I believe it is worthwhile to expose the default prepareStackTrace as well, as mentioned in the OP that the mimic is not what exactly Node.js' default prepareStackTrace is doing.

Created #50827 to expose module.prepareStackTrace which supports both the default plain and source maps.

@nicolo-ribaudo
Copy link
Contributor Author

Q: why on module?

@legendecas
Copy link
Member

My first impression was that this could be related to other functions exposed in node:module. But as pointed out at #50827 (comment), I believe that it can be more intuitive to expose with Error.prepareStackTrace instead.

nodejs-github-bot pushed a commit that referenced this issue Dec 21, 2023
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>
RafaelGSS pushed a commit that referenced this issue Jan 2, 2024
The second parameter of `Error.prepareStackTrace` is an array of
reversed call site frames.

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>
RafaelGSS pushed a commit that referenced this issue Jan 2, 2024
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>
richardlau pushed a commit that referenced this issue Mar 25, 2024
The second parameter of `Error.prepareStackTrace` is an array of
reversed call site frames.

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>
richardlau pushed a commit that referenced this issue Mar 25, 2024
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>
@RedYetiDev RedYetiDev moved this from Awaiting Triage to Done in Node.js feature requests Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. source maps Issues and PRs related to source map support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants