Skip to content

src: don't abort when c-ares initialization fails - #8710

Merged
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:fix8699
Sep 23, 2016
Merged

src: don't abort when c-ares initialization fails#8710
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:fix8699

Conversation

@bnoordhuis

@bnoordhuisbnoordhuis commented Sep 22, 2016

Copy link
Copy Markdown
Member

Throw a JS exception instead of aborting so the user at least has a
fighting chance of understanding what went wrong.

Fixes: #8699

@bnoordhuisbnoordhuis added lib / src Issues and PRs related to general changes in the lib or src directory. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. labels Sep 22, 2016
@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 22, 2016
@targos

targos commented Sep 22, 2016

Copy link
Copy Markdown
Member

Is there a way to simulate failure in a test ?

@bnoordhuis

bnoordhuis commented Sep 22, 2016

Copy link
Copy Markdown
MemberAuthor

Not easily and not cross-platform. c-ares fails when it can't do things like read /etc/resolv.conf or load the right DLL on Windows. Hard to control from a test.

cjihrig
cjihrig approved these changes Sep 22, 2016
View reviewed changes

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

LGTM

@mscdexmscdex removed the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 22, 2016
@fhinkel

fhinkel commented Sep 22, 2016

Copy link
Copy Markdown
Member

LGTM

imyller
imyller approved these changes Sep 22, 2016
View reviewed changes

@imyllerimyller 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

@bnoordhuis
src: don't abort when c-ares initialization fails
5cb5b1f
Throw a JS exception instead of aborting so the user at least has a
fighting chance of understanding what went wrong.
Fixes: nodejs#8699
PR-URL: nodejs#8710
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
@bnoordhuis
bnoordhuis deleted the fix8699 branch September 23, 2016 21:56
@bnoordhuis
bnoordhuis merged commit 5cb5b1f into nodejs:masterSep 23, 2016
jasnell pushed a commit that referenced this pull request Sep 29, 2016
@bnoordhuis@jasnell
src: don't abort when c-ares initialization fails
eb972b0
Throw a JS exception instead of aborting so the user at least has a
fighting chance of understanding what went wrong.
Fixes: #8699
PR-URL: #8710
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
This was referenced Oct 7, 2016
Closed
Closed
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
@bnoordhuis@Fishrock123
src: don't abort when c-ares initialization fails
9205edc
Throw a JS exception instead of aborting so the user at least has a
fighting chance of understanding what went wrong.
Fixes: #8699
PR-URL: #8710
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
@bnoordhuisMyles Borins
src: don't abort when c-ares initialization fails
f0192ec
Throw a JS exception instead of aborting so the user at least has a
fighting chance of understanding what went wrong.
Fixes: #8699
PR-URL: #8710
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
@MylesBorinsMylesBorins mentioned this pull request Nov 22, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
Myles Borins@MylesBorins
2016-12-06, Version 4.7.0 'Argon' (LTS)
f4f2bce
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.
Notable Changes:
The SEMVER-MINOR changes include:
* build:
- export openssl symbols on Windows making it possible to build
addons linked against the bundled version of openssl (Alex Hultman)
#7576
* debugger:
- make listen address configurable in the debugger server
(Ben Noordhuis) #3316
* dgram:
- generalized send queue to handle close fixing a potential throw
when dgram socket is closed in the listening event handler.
(Matteo Collina) #7066
* http:
- Introduce the 451 status code "Unavailable For Legal Reasons"
(Max Barinov) #4377
* tls:
- introduce `secureContext` for `tls.connect` which is useful for
caching client certificates, key, and CA certificates.
(Fedor Indutny) #4246
Notable SEMVER-PATCH changes include:
* build:
- introduce the configure --shared option for embedders (sxa555)
#6994
* gtest:
- the test reporter now outputs tap comments as yamlish
(Johan Bergström) #9262
* src:
- node no longer aborts when c-ares initialization fails
(Ben Noordhuis) #8710
* tls:
- fix memory leak when writing data to TLSWrap instance during
handshake (Fedor Indutny)
#9586
PR-URL: #9736
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
Myles Borins@MylesBorins
2016-12-06, Version 4.7.0 'Argon' (LTS)
4e73e75
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.
Notable Changes:
The SEMVER-MINOR changes include:
* build:
- export openssl symbols on Windows making it possible to build
addons linked against the bundled version of openssl (Alex Hultman)
#7576
* debugger:
- make listen address configurable in the debugger server
(Ben Noordhuis) #3316
* dgram:
- generalized send queue to handle close fixing a potential throw
when dgram socket is closed in the listening event handler.
(Matteo Collina) #7066
* http:
- Introduce the 451 status code "Unavailable For Legal Reasons"
(Max Barinov) #4377
* tls:
- introduce `secureContext` for `tls.connect` which is useful for
caching client certificates, key, and CA certificates.
(Fedor Indutny) #4246
Notable SEMVER-PATCH changes include:
* build:
- introduce the configure --shared option for embedders (sxa555)
#6994
* gtest:
- the test reporter now outputs tap comments as yamlish
(Johan Bergström) #9262
* src:
- node no longer aborts when c-ares initialization fails
(Ben Noordhuis) #8710
* tls:
- fix memory leak when writing data to TLSWrap instance during
handshake (Fedor Indutny)
#9586
PR-URL: #9736
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
Myles Borins@MylesBorins
2016-12-06, Version 4.7.0 'Argon' (LTS)
6ade6de
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.
Notable Changes:
The SEMVER-MINOR changes include:
* build:
- export openssl symbols on Windows making it possible to build
addons linked against the bundled version of openssl (Alex Hultman)
#7576
* debugger:
- make listen address configurable in the debugger server
(Ben Noordhuis) #3316
* dgram:
- generalized send queue to handle close fixing a potential throw
when dgram socket is closed in the listening event handler.
(Matteo Collina) #7066
* http:
- Introduce the 451 status code "Unavailable For Legal Reasons"
(Max Barinov) #4377
* tls:
- introduce `secureContext` for `tls.connect` which is useful for
caching client certificates, key, and CA certificates.
(Fedor Indutny) #4246
Notable SEMVER-PATCH changes include:
* build:
- introduce the configure --shared option for embedders (sxa555)
#6994
* gtest:
- the test reporter now outputs tap comments as yamlish
(Johan Bergström) #9262
* src:
- node no longer aborts when c-ares initialization fails
(Ben Noordhuis) #8710
* tls:
- fix memory leak when writing data to TLSWrap instance during
handshake (Fedor Indutny)
#9586
PR-URL: #9736
imyller added a commit to imyller/meta-nodejs that referenced this pull request Dec 7, 2016
@imyller
2016-12-06, Version 4.7.0 'Argon' (LTS)
72bbc64
 This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.
Notable Changes:
The SEMVER-MINOR changes include:
* build:
- export openssl symbols on Windows making it possible to build
addons linked against the bundled version of openssl (Alex Hultman)
nodejs/node#7576
* debugger:
- make listen address configurable in the debugger server
(Ben Noordhuis) nodejs/node#3316
* dgram:
- generalized send queue to handle close fixing a potential throw
when dgram socket is closed in the listening event handler.
(Matteo Collina) nodejs/node#7066
* http:
- Introduce the 451 status code "Unavailable For Legal Reasons"
(Max Barinov) nodejs/node#4377
* tls:
- introduce `secureContext` for `tls.connect` which is useful for
caching client certificates, key, and CA certificates.
(Fedor Indutny) nodejs/node#4246
Notable SEMVER-PATCH changes include:
* build:
- introduce the configure --shared option for embedders (sxa555)
nodejs/node#6994
* gtest:
- the test reporter now outputs tap comments as yamlish
(Johan Bergstrom) nodejs/node#9262
* src:
- node no longer aborts when c-ares initialization fails
(Ben Noordhuis) nodejs/node#8710
* tls:
- fix memory leak when writing data to TLSWrap instance during
handshake (Fedor Indutny)
nodejs/node#9586
PR-URL: nodejs/node#9736
Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Dec 7, 2016
@imyller
2016-12-06, Version 4.7.0 'Argon' (LTS)
3a81f72
 This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.
Notable Changes:
The SEMVER-MINOR changes include:
* build:
- export openssl symbols on Windows making it possible to build
addons linked against the bundled version of openssl (Alex Hultman)
nodejs/node#7576
* debugger:
- make listen address configurable in the debugger server
(Ben Noordhuis) nodejs/node#3316
* dgram:
- generalized send queue to handle close fixing a potential throw
when dgram socket is closed in the listening event handler.
(Matteo Collina) nodejs/node#7066
* http:
- Introduce the 451 status code "Unavailable For Legal Reasons"
(Max Barinov) nodejs/node#4377
* tls:
- introduce `secureContext` for `tls.connect` which is useful for
caching client certificates, key, and CA certificates.
(Fedor Indutny) nodejs/node#4246
Notable SEMVER-PATCH changes include:
* build:
- introduce the configure --shared option for embedders (sxa555)
nodejs/node#6994
* gtest:
- the test reporter now outputs tap comments as yamlish
(Johan Bergstrom) nodejs/node#9262
* src:
- node no longer aborts when c-ares initialization fails
(Ben Noordhuis) nodejs/node#8710
* tls:
- fix memory leak when writing data to TLSWrap instance during
handshake (Fedor Indutny)
nodejs/node#9586
PR-URL: nodejs/node#9736
Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjihrigcjihrigcjihrig approved these changes
+1 more reviewer
@imyllerimyllerimyller approved these changes
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++.caresIssues and PRs related to the c-ares dependency or the cares_wrap binding.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@bnoordhuis@targos@fhinkel@imyller@cjihrig@mscdex@MylesBorins@nodejs-github-bot