FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: expose default prepareStackTrace by legendecas · Pull Request #50827 · nodejs/node · GitHub

/ node Public

lib: expose default prepareStackTrace - #50827

Closed
legendecas wants to merge 3 commits into
nodejs:mainfrom
legendecas:source_maps/prepare_stack_trace
Closed

lib: expose default prepareStackTrace#50827
legendecas wants to merge 3 commits into
nodejs:mainfrom
legendecas:source_maps/prepare_stack_trace

Conversation

legendecas commented Nov 20, 2023
edited
Loading

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/startup

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

Copy link
Copy Markdown
Member

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

Comment thread doc/api/module.md Outdated
Comment thread doc/api/module.md Outdated
Comment thread doc/api/module.md Outdated
Comment thread lib/internal/errors.js Outdated
legendecas force-pushed the source_maps/prepare_stack_trace branch 4 times, most recently from 2a5b908 to 5bbcf91 Compare November 21, 2023 18:37

Ethan-Arrowood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread doc/api/cli.md Outdated
Comment thread lib/repl.js Outdated
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 force-pushed the source_maps/prepare_stack_trace branch from 5bbcf91 to f281bc4 Compare November 27, 2023 15:47
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

Copy link
Copy Markdown
Member Author

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

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2023

Copy link
Copy Markdown
Collaborator

legendecas commented Dec 10, 2023
edited
Loading

Copy link
Copy Markdown
Member Author

@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

Copy link
Copy Markdown
Collaborator

Comment thread lib/internal/errors.js
Comment thread lib/internal/errors.js Outdated
Comment thread doc/api/cli.md
legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2023

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Collaborator

GeoffreyBooth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks great, thanks! 👍

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
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 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 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 mentioned this pull request Mar 25, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL