dns: fix `resolve` failed starts without network by XadillaX · Pull Request #13076 · nodejs/node · GitHub

Fix the bug that you start process without network at first, but it connected lately, dns.resolve will stay failed with ECONNREFUSED because c-ares servers fallback to 127.0.0.1 at the very beginning.

If c-ares servers "127.0.0.1" is detected and its not set by user self, and last query is not OK, recreating ares_channel operation will be triggered to reload servers.

You can test with this script:

const dns = require("dns");

function test() {
    dns.resolve("google.com", function(err, ret) {
        console.log(err, ret, dns.getServers());
        setTimeout(test, 10000);
    });
}

test();

Turn off your network at first before starting this script. Then try to open the network and then turn off. Make the steps above in a loop and you will get the result.

Fixes: #1644

Solutaion

DNS server matches follow conditions is considered as a fallback server of c-ares:

caused last query ECONNREFUSED; (the fallback server very likely to have this error)
is exactly [ "127.0.0.1" ]; (the fallback server is exactly this value)
haven't set by users or developers by dns.setServers.

Because users or developers may do dns.setServers([ "127.0.0.1" ]) or theirs /etc/resolv.conf is 127.0.0.1.

As a result, I gathered those 3 conditions.

I created two flags at the process start up.

cares_query_last_ok_ indicates that is last dns query succeeded or not;
cares_is_servers_use_default_ indicates that whether current dns servers is the fallback 127.0.0.1 that c-ares initialized at the beginning.

If cares_query_last_ok_ is true or cares_is_servers_use_default_ is false, everything will be OK.

If not, the code will check current dns servers to see if it's 127.0.0.1 (maybe c-ares' fallback server). When it's 127.0.0.1, old ares_channel will be released and a new one will be created.

Once one query succeeded or not ECONNREFUSED, cares_query_last_ok_ will be marked as true. And once user set dns servers via dns.setServers, cares_is_servers_use_default_ will be marked as false. And what's more, once the first check found the dns servers is not 127.0.0.1, cares_is_servers_use_default_ also will be marked as false.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

dns

Anyone who has time to review this code?

/cc @trevnorris @indutny @addaleax @danbev

Any chance you could name the setters set_FLAG() and the getters just FLAG() (compare with e.g. using_domains)?

Extremely tiny nit: I’d put these on separate lines

I realize the code around this doesn’t do it either, but once more, would you mind aligning the arguments vertically? 😄

tiny nit: Check sounds a bit like it’s just testing things, maybe Ensure is better?

(ditto for aligning)

Can we somehow share this code with Initialize, to make it more obviously correct? It looks like there’s quite a bit that could be shared

I'v extracted these code to SetupCaresChannel().

CI: https://ci.nodejs.org/job/node-test-commit/9955/

One of Windows CI Machines failed. I wonder why.

not ok 2 parallel/test-async-wrap-getasyncid
  ---
  duration_ms: 0.248
  severity: fail
  stack: |-
    Mismatched <anonymous> function calls. Expected 1, actual 0.
        at Object.<anonymous> (c:\workspace\node-test-binary-windows\RUN_SUBSET\0\VS_VERSION\vs2015-x86\label\win2008r2\test\parallel\test-async-wrap-getasyncid.js:155:42)
        at Module._compile (module.js:569:30)
        at Object.Module._extensions..js (module.js:580:10)
        at Module.load (module.js:503:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3)
        at Function.Module.runMain (module.js:605:10)
        at startup (bootstrap_node.js:144:16)
        at bootstrap_node.js:561:3
    (node:5140) [DEP0064] DeprecationWarning: tls.createSecurePair() is deprecated. Please use tls.Socket instead.
  ...

I don't know why, maybe it's the bug of CI? I'm not sure. @addaleax

Yes, the CI failure is nothing to worry about. 👍 (It’s not a bug in the CI but it’s a known failure that’s not related to this PR.)

Hey @addaleax I've already rebased for some conflicts with master.

CI: https://ci.nodejs.org/job/node-test-commit/10008/

CI is green. Maybe @bnoordhuis or @silverwind could give this a look, too?

LGTM modulo comments.

This fits on one line now.

Likewise. I'll stop pointing it out from here on.

@XadillaX Actually, one more thing: your author name in this commit is given as “XadillaX”. Is that intended or do you prefer to be listed (changelog, git log, AUTHORS file) with some other name? People typically prefer their full name, but ultimately it’s up to you. (Edit: Just to be clear, that name doesn’t have to be ASCII-only – whatever you prefer, really.)

Fix the bug that you start process without network at first, but it connected lately, `dns.resolve` will stay failed with ECONNREFUSED because c-ares servers fallback to 127.0.0.1 at the very beginning. If c-ares servers "127.0.0.1" is detected and its not set by user self, and last query is not OK, recreating `ares_channel` operation will be triggered to reload servers. Fixes: nodejs#1644

@addaleax I think 'XadillaX - Khaidi Chu' would be fine to me.

@addaleax I mean I'd like to use XadillaX when in git log, changelog and authors file. And if someday I have the honor of being a collaborator, I'd like to use 'XadillaX - Khaidi Chu' in the list.

Landed in 2b54147

@XadillaX I think you’ve already figured out that you can set this information using git config [--global] user.name and … user.email, but if you need any help, just let us know. :)

Fix the bug that you start process without network at first, but it connected lately, `dns.resolve` will stay failed with ECONNREFUSED because c-ares servers fallback to 127.0.0.1 at the very beginning. If c-ares servers "127.0.0.1" is detected and its not set by user self, and last query is not OK, recreating `ares_channel` operation will be triggered to reload servers. Fixes: #1644 PR-URL: #13076 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@addaleax Thanks. I wondered if this PR passed the coverity? Shall I trigger that by myself?

@XadillaX I am not sure how to do that. I think the tool gets run periodically without being triggered, but really, I’m not sure.

@addaleax Thanks. I wondered if this PR passed the coverity? Shall I trigger that by myself?

see #13117 (comment)

Should this be backported to v6.x? If so it will require to be done manually

Fix the bug that you start process without network at first, but it connected lately, `dns.resolve` will stay failed with ECONNREFUSED because c-ares servers fallback to 127.0.0.1 at the very beginning. If c-ares servers "127.0.0.1" is detected and its not set by user self, and last query is not OK, recreating `ares_channel` operation will be triggered to reload servers. Fixes: nodejs#1644 PR-URL: nodejs#13076 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Should this be backported to v6.x? If so it will require to be done manually

#14434

Fix the bug that you start process without network at first, but it connected lately, `dns.resolve` will stay failed with ECONNREFUSED because c-ares servers fallback to 127.0.0.1 at the very beginning. If c-ares servers "127.0.0.1" is detected and its not set by user self, and last query is not OK, recreating `ares_channel` operation will be triggered to reload servers. Fixes: #1644 Backport-PR-URL: #14434 PR-URL: #13076 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

nodejs-github-bot added 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. labels May 17, 2017

XadillaX force-pushed the fix/resolve-fails-when-start-without-network branch from 8662937 to 57f04fd Compare May 17, 2017 09:44

addaleax approved these changes May 17, 2017

View reviewed changes

XadillaX force-pushed the fix/resolve-fails-when-start-without-network branch from 57f04fd to 51caf53 Compare May 18, 2017 03:38

XadillaX force-pushed the fix/resolve-fails-when-start-without-network branch from 51caf53 to 490672c Compare May 19, 2017 02:52

bnoordhuis approved these changes May 19, 2017

View reviewed changes

XadillaX force-pushed the fix/resolve-fails-when-start-without-network branch from 490672c to 355a33b Compare May 20, 2017 04:48

addaleax closed this May 21, 2017

jasnell mentioned this pull request May 28, 2017

8.0.0 Release Proposal #12220

Closed

gibfahn mentioned this pull request Jun 15, 2017

Auditing for 6.11.1 nodejs/Release#230

Closed

3 tasks

MylesBorins added the lts-watch-v6.x label Jul 17, 2017

XadillaX mentioned this pull request Jul 23, 2017

(v6.x backport) dns: fix resolve failed starts without network #14434

Closed

2 tasks

MylesBorins added backported-to-v6.x and removed lts-watch-v6.x labels Aug 14, 2017

MylesBorins mentioned this pull request Aug 16, 2017

v6.11.3-proposal #14852

Merged

robobun mentioned this pull request Jul 20, 2026

dns: re-read system nameservers when network config changes oven-sh/bun#34781

Open

coderabbitai Bot mentioned this pull request Jul 23, 2026

net.connect: report resolver failures with a negative errno instead of hanging oven-sh/bun#35221

Open

dns: fix `resolve` failed starts without network by XadillaX · Pull Request #13076 · nodejs/node · GitHub

Uh oh!

Uh oh!

dns: fix resolve failed starts without network - #13076

dns: fix resolve failed starts without network#13076XadillaX wants to merge 1 commit intonodejs:masternodejs/node:masterfrom XadillaX:fix/resolve-fails-when-start-without-networkXadillaX/node:fix/resolve-fails-when-start-without-networkCopy head branch name to clipboard

Conversation

XadillaX commented May 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Solutaion

Checklist

Affected core subsystem(s)

Uh oh!

XadillaX commented May 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax commented May 17, 2017

Uh oh!

pmq20 commented May 18, 2017

Uh oh!

XadillaX commented May 18, 2017

Uh oh!

addaleax commented May 18, 2017

Uh oh!

XadillaX commented May 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax commented May 19, 2017

Uh oh!

addaleax commented May 19, 2017

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax commented May 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XadillaX commented May 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XadillaX commented May 21, 2017

Uh oh!

addaleax commented May 21, 2017

Uh oh!

XadillaX commented May 21, 2017

Uh oh!

addaleax commented May 21, 2017

Uh oh!

gibfahn commented May 22, 2017

Uh oh!

MylesBorins commented Jul 17, 2017

Uh oh!

XadillaX commented Jul 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dns: fix resolve failed starts without network#13076
XadillaX wants to merge 1 commit into
nodejs:masterfrom
XadillaX:fix/resolve-fails-when-start-without-network

XadillaX commented May 17, 2017 •
edited
Loading

XadillaX commented May 19, 2017 •
edited
Loading

addaleax commented May 19, 2017 •
edited
Loading

XadillaX commented May 20, 2017 •
edited
Loading