Skip to content

dns: support max timeout #58440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ Create a new resolver.
default timeout.
* `tries` {integer} The number of tries the resolver will try contacting
each name server before giving up. **Default:** `4`
* `maxTimeout` {integer} The max retry timeout, in milliseconds.
**Default:** `0`, disabled.

### `resolver.cancel()`

Expand Down
20 changes: 14 additions & 6 deletions lib/internal/dns/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
validateInt32,
validateOneOf,
validateString,
validateUint32,
} = require('internal/validators');
let binding;
function lazyBinding() {
Expand All @@ -49,6 +50,12 @@ function validateTimeout(options) {
return timeout;
}

function validateMaxTimeout(options) {
const { maxTimeout = 0 } = { ...options };
validateUint32(maxTimeout, 'options.maxTimeout', 0);
return maxTimeout;
}

function validateTries(options) {
const { tries = 4 } = { ...options };
validateInt32(tries, 'options.tries', 1);
Expand All @@ -67,17 +74,18 @@ class ResolverBase {
constructor(options = undefined) {
const timeout = validateTimeout(options);
const tries = validateTries(options);
const maxTimeout = validateMaxTimeout(options);
// If we are building snapshot, save the states of the resolver along
// the way.
if (isBuildingSnapshot()) {
this[kSnapshotStates] = { timeout, tries };
this[kSnapshotStates] = { timeout, tries, maxTimeout };
}
this[kInitializeHandle](timeout, tries);
this[kInitializeHandle](timeout, tries, maxTimeout);
}

[kInitializeHandle](timeout, tries) {
[kInitializeHandle](timeout, tries, maxTimeout) {
const { ChannelWrap } = lazyBinding();
this._handle = new ChannelWrap(timeout, tries);
this._handle = new ChannelWrap(timeout, tries, maxTimeout);
}

cancel() {
Expand Down Expand Up @@ -187,8 +195,8 @@ class ResolverBase {
}

[kDeserializeResolver]() {
const { timeout, tries, localAddress, servers } = this[kSnapshotStates];
this[kInitializeHandle](timeout, tries);
const { timeout, tries, maxTimeout, localAddress, servers } = this[kSnapshotStates];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joyeecheung Hi ! Can I make a snapshot in node version 1 and then use it in version 2? Do I need to use maxTimeout=0 here ?

this[kInitializeHandle](timeout, tries, maxTimeout);
if (localAddress) {
const { ipv4, ipv6 } = localAddress;
this._handle.setLocalAddress(ipv4, ipv6);
Expand Down
30 changes: 19 additions & 11 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -787,14 +787,15 @@ Maybe<int> ParseSoaReply(Environment* env,
}
} // anonymous namespace

ChannelWrap::ChannelWrap(
Environment* env,
Local<Object> object,
int timeout,
int tries)
ChannelWrap::ChannelWrap(Environment* env,
Local<Object> object,
int timeout,
int tries,
int max_timeout)
: AsyncWrap(env, object, PROVIDER_DNSCHANNEL),
timeout_(timeout),
tries_(tries) {
tries_(tries),
max_timeout_(max_timeout) {
MakeWeak();

Setup();
Expand All @@ -808,13 +809,15 @@ void ChannelWrap::MemoryInfo(MemoryTracker* tracker) const {

void ChannelWrap::New(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
CHECK_EQ(args.Length(), 2);
CHECK_EQ(args.Length(), 3);
CHECK(args[0]->IsInt32());
CHECK(args[1]->IsInt32());
CHECK(args[2]->IsInt32());
const int timeout = args[0].As<Int32>()->Value();
const int tries = args[1].As<Int32>()->Value();
const int max_timeout = args[2].As<Int32>()->Value();
Environment* env = Environment::GetCurrent(args);
new ChannelWrap(env, args.This(), timeout, tries);
new ChannelWrap(env, args.This(), timeout, tries, max_timeout);
}

GetAddrInfoReqWrap::GetAddrInfoReqWrap(Environment* env,
Expand Down Expand Up @@ -879,9 +882,14 @@ void ChannelWrap::Setup() {
}

/* We do the call to ares_init_option for caller. */
const int optmask =
ARES_OPT_FLAGS | ARES_OPT_TIMEOUTMS |
ARES_OPT_SOCK_STATE_CB | ARES_OPT_TRIES;
int optmask = ARES_OPT_FLAGS | ARES_OPT_TIMEOUTMS | ARES_OPT_SOCK_STATE_CB |
ARES_OPT_TRIES;

if (max_timeout_ > 0) {
options.maxtimeout = max_timeout_;
optmask |= ARES_OPT_MAXTIMEOUTMS;
}

r = ares_init_options(&channel_, &options, optmask);

if (r != ARES_SUCCESS) {
Expand Down
11 changes: 6 additions & 5 deletions src/cares_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ struct NodeAresTask final : public MemoryRetainer {

class ChannelWrap final : public AsyncWrap {
public:
ChannelWrap(
Environment* env,
v8::Local<v8::Object> object,
int timeout,
int tries);
ChannelWrap(Environment* env,
v8::Local<v8::Object> object,
int timeout,
int tries,
int max_timeout);
~ChannelWrap() override;

static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down Expand Up @@ -190,6 +190,7 @@ class ChannelWrap final : public AsyncWrap {
bool library_inited_ = false;
int timeout_;
int tries_;
int max_timeout_;
int active_query_count_ = 0;
NodeAresTask::List task_list_;
};
Expand Down
57 changes: 57 additions & 0 deletions test/parallel/test-dns-resolver-max-timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict';
const common = require('../common');
const dnstools = require('../common/dns');
const dns = require('dns');
const assert = require('assert');
const dgram = require('dgram');

const server = dgram.createSocket('udp4');
const nxdomain = 'nxdomain.org';
const domain = 'example.org';
const answers = [{ type: 'A', address: '1.2.3.4', ttl: 123, domain }];

server.on('message', common.mustCallAtLeast((msg, { address, port }) => {
const parsed = dnstools.parseDNSPacket(msg);
if (parsed.questions[0].domain === nxdomain) {
return;
}
assert.strictEqual(parsed.questions[0].domain, domain);
server.send(dnstools.writeDNSPacket({
id: parsed.id,
questions: parsed.questions,
answers: answers,
}), port, address);
}), 1);

server.bind(0, common.mustCall(async () => {
const address = server.address();
// Test if the Resolver works as before.
const resolver = new dns.promises.Resolver({ timeout: 1000, tries: 1, maxTimeouts: 1000 });
resolver.setServers([`127.0.0.1:${address.port}`]);
const res = await resolver.resolveAny('example.org');
assert.strictEqual(res.length, 1);
assert.strictEqual(res.length, answers.length);
assert.strictEqual(res[0].address, answers[0].address);

// Test that maxTimeout is effective.
// Without maxTimeout, the timeout will keep increasing when retrying.
const timeout1 = await timeout(address, { timeout: 500, tries: 3 });
// With maxTimeout, the timeout will always be 500 when retrying.
const timeout2 = await timeout(address, { timeout: 500, tries: 3, maxTimeout: 500 });
console.log(`timeout1: ${timeout1}, timeout2: ${timeout2}`);
assert.strictEqual(timeout1 !== undefined && timeout2 !== undefined, true);
assert.strictEqual(timeout1 > timeout2, true);
server.close();
}));

async function timeout(address, options) {
const start = Date.now();
const resolver = new dns.promises.Resolver(options);
resolver.setServers([`127.0.0.1:${address.port}`]);
try {
await resolver.resolveAny(nxdomain);
} catch (e) {
assert.strictEqual(e.code, 'ETIMEOUT');
return Date.now() - start;
}
}
Loading