GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: don't abort when c-ares initialization fails by bnoordhuis · Pull Request #8710 · nodejs/node

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bnoordhuis added lib / src

Issues and PRs involving general changes in the lib/ or src/ directories.

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 ?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cjihrig approved these changes

Sep 22, 2016

View reviewed changes

cjihrig left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mscdex removed the lib / src Issues and PRs involving general changes in the lib/ or src/ directories. label

Sep 22, 2016

fhinkel commented Sep 22, 2016

Copy link

Copy Markdown

Contributor

LGTM

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

imyller approved these changes

Sep 22, 2016

View reviewed changes

imyller left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          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 <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>

bnoordhuis force-pushed the fix8699 branch from e65b051 to 5cb5b1f Compare

September 23, 2016 21:56

bnoordhuis closed this

Sep 23, 2016

bnoordhuis deleted the fix8699 branch

September 23, 2016 21:56

bnoordhuis added the lts-watch-v4.x label

Sep 23, 2016

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 <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>

This was referenced Oct 7, 2016

Assertion failed: (r) == (0), file src\cares_wrap.cc, line 1262 #5588

Closed

dns: don't skip getaddrino/getnameinfo when c-ares init fails #8966

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 <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>

MylesBorins added land-on-v4.x and removed lts-watch-v4.x labels

Nov 18, 2016

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 <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>

MylesBorins mentioned this pull request

Nov 22, 2016

v4.7.0 proposal #9736

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 <[email protected]>

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 <[email protected]>

Reviewers

cjihrig

cjihrig approved these changes

+1 more reviewer

imyller

imyller approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.