Skip to content

src: Handle NULL env scenario#31899

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.

Sign up for GitHub

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

Jump to bottom
Closed
HarshithaKP wants to merge 2 commits into from
Closed

src: Handle NULL env scenario #31899

HarshithaKP wants to merge 2 commits into from

Conversation

@HarshithaKP
Copy link
Member

@HarshithaKPHarshithaKP commented Feb 21, 2020

Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Feb 21, 2020
addaleax
addaleax reviewed Feb 21, 2020
View reviewed changes
@HarshithaKPHarshithaKPforce-pushed the module_wrap_handle_null_ptr branch from c06e445 to baa5f1aCompareFebruary 21, 2020 08:39
mscdex
mscdex reviewed Feb 21, 2020
View reviewed changes
@HarshithaKP
Copy link
MemberAuthor

HarshithaKP commented Feb 21, 2020

While going through the file found one more similar location, fixed it.

@HarshithaKP
Copy link
MemberAuthor

HarshithaKP commented Feb 21, 2020

Surprisingly make format-cpp did not fix the error. Fixed it in one more commit.

addaleax
addaleax approved these changes Feb 22, 2020
View reviewed changes
Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

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

LGTM with docs added

addaleax
addaleax reviewed Mar 4, 2020
View reviewed changes
@HarshithaKPHarshithaKPforce-pushed the module_wrap_handle_null_ptr branch from 870a3cb to 53c0050CompareMarch 5, 2020 07:17
@HarshithaKP
src: Handle NULL env scenario
8069174
Convert hard assertion into a throw with a useful error message in src/module_wrap.cc
@HarshithaKPHarshithaKPforce-pushed the module_wrap_handle_null_ptr branch from 8594430 to 8069174CompareMarch 5, 2020 07:22
@HarshithaKP
Copy link
MemberAuthor

HarshithaKP commented Mar 5, 2020

I rebased and commits squashed

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 5, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29562/

addaleax
addaleax approved these changes Mar 5, 2020
View reviewed changes
@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 5, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 5, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29563/

@addaleax
Copy link
Member

addaleax commented Mar 9, 2020

Landed in e9fa5ae

addaleax pushed a commit that referenced this pull request Mar 9, 2020
@HarshithaKP@addaleax
src: handle NULL env scenario
e9fa5ae
Convert hard assertion into a throw with a useful error message in src/module_wrap.cc. PR-URL: #31899 Reviewed-By: Anna Henningsen <[email protected]>
@addaleaxaddaleax closed this Mar 9, 2020
MylesBorins pushed a commit that referenced this pull request Mar 9, 2020
@HarshithaKP@MylesBorins
src: handle NULL env scenario
cba75c5
Convert hard assertion into a throw with a useful error message in src/module_wrap.cc. PR-URL: #31899 Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Mar 10, 2020
Merged
@targostargos added backport-blocked-v12.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 20, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@HarshithaKP@targos
src: handle NULL env scenario
b3fb4f3
Convert hard assertion into a throw with a useful error message in src/module_wrap.cc. PR-URL: nodejs#31899 Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
@HarshithaKP@targos
src: handle NULL env scenario
51a3456
Convert hard assertion into a throw with a useful error message in src/module_wrap.cc. PR-URL: #31899 Reviewed-By: Anna Henningsen <[email protected]>
@targostargos mentioned this pull request May 2, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleaxaddaleaxaddaleax approved these changes

+1 more reviewer

@mscdexmscdexmscdex left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@HarshithaKP@nodejs-github-bot@addaleax@mscdex@targos