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

src: don't abort when c-ares initialization fails by bnoordhuis · Pull Request #8710 · nodejs/node · GitHub

/ node Public

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

Merged
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:fix8699
Sep 23, 2016
ConversationCommits (1)ChecksFiles changed
Merged

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

Conversation

bnoordhuis 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

bnoordhuis 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-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 22, 2016

targos commented Sep 22, 2016

Copy link
Copy Markdown
Member

Is there a way to simulate failure in a test ?

bnoordhuis commented Sep 22, 2016

Copy link
Copy Markdown
Member Author

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 approved these changes Sep 22, 2016
View reviewed changes

cjihrig 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

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

fhinkel commented Sep 22, 2016

Copy link
Copy Markdown
Contributor

LGTM

imyller approved these changes Sep 22, 2016
View reviewed changes

imyller 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

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 closed this Sep 23, 2016
bnoordhuis deleted the fix8699 branch September 23, 2016 21:56
bnoordhuis merged commit 5cb5b1f into nodejs:master Sep 23, 2016
jasnell pushed a commit that referenced this pull request Sep 29, 2016
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
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
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>
MylesBorins mentioned this pull request Nov 22, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
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
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
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
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
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>
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

cjihrig cjihrig approved these changes
+1 more reviewer
imyller imyller 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++. cares Issues 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

Back | FazBrowse Home | New Git URL