Skip to content

src: Handle NULL env scenario - #31899

Closed
HarshithaKP wants to merge 2 commits into
nodejs:masterfrom
HarshithaKP:module_wrap_handle_null_ptr
Closed

src: Handle NULL env scenario#31899
HarshithaKP wants to merge 2 commits into
nodejs:masterfrom
HarshithaKP:module_wrap_handle_null_ptr

Conversation

@HarshithaKP

@HarshithaKPHarshithaKP commented Feb 21, 2020

Copy link
Copy Markdown
Member

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
Comment threadsrc/module_wrap.cc Outdated
@HarshithaKP
HarshithaKPforce-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
Comment threadsrc/node_errors.h Outdated
@HarshithaKP

HarshithaKP commented Feb 21, 2020

Copy link
Copy Markdown
MemberAuthor

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

@HarshithaKP

HarshithaKP commented Feb 21, 2020

Copy link
Copy Markdown
MemberAuthor

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

@addaleaxaddaleax 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.

LGTM with docs added

Comment threadsrc/node_errors.h Outdated
addaleax
addaleax reviewed Mar 4, 2020
View reviewed changes
Comment threaddoc/api/errors.md Outdated
Comment threadsrc/node_errors.h Outdated
@HarshithaKP
HarshithaKPforce-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
@HarshithaKP
HarshithaKPforce-pushed the module_wrap_handle_null_ptr branch from 8594430 to 8069174CompareMarch 5, 2020 07:22
@HarshithaKP

HarshithaKP commented Mar 5, 2020

Copy link
Copy Markdown
MemberAuthor

I rebased and commits squashed

@nodejs-github-bot

nodejs-github-bot commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

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

addaleax
addaleax approved these changes Mar 5, 2020
View reviewed changes
Comment threadsrc/node_errors.h Outdated
@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

nodejs-github-bot commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

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

@addaleax

addaleax commented Mar 9, 2020

Copy link
Copy Markdown
Member

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 <anna@addaleax.net>
@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 <anna@addaleax.net>
@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 <anna@addaleax.net>
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 <anna@addaleax.net>
@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