Skip to content

Conversation

@legendecas
Copy link
Member

@legendecas legendecas commented Nov 20, 2023

repl: fix prepareStackTrace frames array order

The second parameter of Error.prepareStackTrace is an array of
reversed call site frames.

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.

Fixes: #50733

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Nov 20, 2023
@GeoffreyBooth
Copy link
Member

Can people call this function as a replacement for Error.prepareStackTrace? If not then we should maybe name it defaultPrepareStackTrace.

@legendecas legendecas force-pushed the source_maps/prepare_stack_trace branch 4 times, most recently from 2a5b908 to 5bbcf91 Compare November 21, 2023 18:37
Copy link
Contributor

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good. can you make the new test file ESM? I believe we are trying to make all net-new tests ESM now

legendecas and others added 2 commits November 27, 2023 23:46
The second parameter of `Error.prepareStackTrace` is an array of
reversed call site frames.
Expose the default prepareStackTrace implementation as
`Error.prepareStackTrace` so that userland can chain up formatting of
stack traces with built-in source maps support.
@legendecas legendecas force-pushed the source_maps/prepare_stack_trace branch from 5bbcf91 to f281bc4 Compare November 27, 2023 15:47
@legendecas legendecas added commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. request-ci Add this label to start a Jenkins CI on a PR. labels Dec 10, 2023
@legendecas
Copy link
Member Author

@joyeecheung would you mind taking a look at this PR again? Thank you very much!

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2023
@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member Author

legendecas commented Dec 10, 2023

@GeoffreyBooth: Can people call this function as a replacement for Error.prepareStackTrace? If not then we should maybe name it defaultPrepareStackTrace.

Updated the PR to expose the default implementation as Error.prepareStackTrace instead. Would you mind taking a look again? Thank you

@nodejs-github-bot
Copy link
Collaborator

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2023
@legendecas
Copy link
Member Author

@GeoffreyBooth would you mind taking a look again? Thank you!

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! 👍

@nodejs-github-bot
Copy link
Collaborator

@legendecas legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 21, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 21, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in fe918f5...147abb9

nodejs-github-bot pushed a commit that referenced this pull request Dec 21, 2023
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>
nodejs-github-bot pushed a commit that referenced this pull request 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>
@legendecas legendecas deleted the source_maps/prepare_stack_trace branch December 21, 2023 16:59
RafaelGSS pushed a commit that referenced this pull request 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 pull request 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>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
richardlau pushed a commit that referenced this pull request 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 pull request 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>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

6 participants