Skip to content
New issue

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

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

Already on GitHub? # to your account

src: fail CommonEnvironmentSetup if environment bootstrapping failed #46797

Closed
wants to merge 1 commit into from

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Feb 23, 2023

If the Environment is failed to bootstrap at development time (e.g. syntax errors or uncaught exceptions in the JavaScript bootstrapping codes), the CommonEnvironmentSetup::CreateForSnapshotting should return a nullptr instead of a CommonEnvironmentSetup with a nullptr env member, which would lead to invalid access.

Unfortunately, I'm not aware of a method to run an unhappy path in bootstrapping to add a test case for this behavior.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 23, 2023
@addaleax
Copy link
Member

Mh this might be a duplicate of #46533? The approach there is a bit more complete because it passes along the actual error message (but also does a quite a bit of work in order to be able to do so)

@legendecas
Copy link
Member Author

Ah, thanks. Closing as duplicated.

@legendecas legendecas closed this Feb 23, 2023
@addaleax addaleax added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Feb 23, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. duplicate Issues and PRs that are duplicates of other issues or PRs. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants