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

bootstrap: mksnapshot should show JS error summaries by bmeck · Pull Request #38174 · nodejs/node · GitHub

/ node Public

bootstrap: mksnapshot should show JS error summaries - #38174

Closed
bmeck wants to merge 6 commits into
nodejs:masterfrom
bmeck:mksnapshot-show-js-error
Closed

bootstrap: mksnapshot should show JS error summaries#38174
bmeck wants to merge 6 commits into
nodejs:masterfrom
bmeck:mksnapshot-show-js-error

Conversation

bmeck commented Apr 9, 2021

Copy link
Copy Markdown
Member

This came up while trying to debug some stuff going on in the snapshot, makes debugging a lot easier. IDK if we want to do something other than use abort() here.

bmeck requested review from Trott and joyeecheung April 9, 2021 20:33
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 Apr 9, 2021
bmeck added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. labels Apr 9, 2021
bmeck force-pushed the mksnapshot-show-js-error branch from 8f5e7f7 to 2ffc11a Compare April 9, 2021 20:36

This comment has been minimized.

bmeck added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 9, 2021
Comment thread tools/snapshot/snapshot_builder.cc Outdated
Comment thread tools/snapshot/snapshot_builder.cc Outdated
bmeck and others added 2 commits April 12, 2021 13:04
Co-authored-by: Michaël Zasso <targos@protonmail.com>
Co-authored-by: Michaël Zasso <targos@protonmail.com>

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

bmeck commented Apr 13, 2021

Copy link
Copy Markdown
Member Author

CI is green except a timeout

bmeck added a commit that referenced this pull request Apr 13, 2021
PR-URL: #38174
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

bmeck commented Apr 13, 2021

Copy link
Copy Markdown
Member Author

Landed in 837f7e2f71ce

bmeck closed this Apr 13, 2021
BethGriggs pushed a commit that referenced this pull request Apr 15, 2021
PR-URL: #38174
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Copy link
Copy Markdown
Member

Sorry for missing the ping: just FYI SetIsolateErrorHandlers() can install error handlers that print uncaught exceptions and it's used by the ordinary Node.js instances - I don't quite remember why it wasn't used by mksnapshot, though. Also there are some routines in node_errors.cc that can be reused to print these errors.

bmeck deleted the mksnapshot-show-js-error branch February 3, 2022 21:46
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

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. 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.

6 participants


Back | FazBrowse Home | New Git URL