Repository: nodejs/node
Pull request: dns: support max timeout

When dns retries the query, the `tiemout` will keep increasing, `max_timeout` caps the `timeout`.

- [x] `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes
- [x] tests and/or benchmarks are included
- [x] documentation is changed or added
- [x] commit message follows [commit guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines)


Changed files:
filename: doc/api/dns.md
patch: @@ -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()`
 

filename: lib/internal/dns/utils.js
patch: @@ -25,6 +25,7 @@ const {
   validateInt32,
   validateOneOf,
   validateString,
+  validateUint32,
 } = require('internal/validators');
 let binding;
 function lazyBinding() {
@@ -49,6 +50,12 @@ function validateTimeout(options) {
   return timeout;
 }
 
+function validateMaxTimeout(options) {
+  const { maxTimeout = 0 } = { ...options };
+  validateUint32(maxTimeout, 'options.maxTimeout');
+  return maxTimeout;
+}
+
 function validateTries(options) {
   const { tries = 4 } = { ...options };
   validateInt32(tries, 'options.tries', 1);
@@ -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() {
@@ -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];
+    this[kInitializeHandle](timeout, tries, maxTimeout);
     if (localAddress) {
       const { ipv4, ipv6 } = localAddress;
       this._handle.setLocalAddress(ipv4, ipv6);

filename: src/cares_wrap.cc
patch: @@ -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();
@@ -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,
@@ -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 |
-                      ARES_OPT_QUERY_CACHE;
+  int optmask = ARES_OPT_FLAGS | ARES_OPT_TIMEOUTMS | ARES_OPT_SOCK_STATE_CB |
+                ARES_OPT_TRIES | ARES_OPT_QUERY_CACHE;
+
+  if (max_timeout_ > 0) {
+    options.maxtimeout = max_timeout_;
+    optmask |= ARES_OPT_MAXTIMEOUTMS;
+  }
+
   r = ares_init_options(&channel_, &options, optmask);
 
   if (r != ARES_SUCCESS) {

filename: src/cares_wrap.h
patch: @@ -152,11 +152,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);
@@ -191,6 +191,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_;
 };

filename: test/parallel/test-dns-resolver-max-timeout.js
patch: @@ -0,0 +1,77 @@
+'use strict';
+const common = require('../common');
+const dnstools = require('../common/dns');
+const dns = require('dns');
+const assert = require('assert');
+const dgram = require('dgram');
+
+[
+  -1,
+  1.1,
+  NaN,
+  undefined,
+  {},
+  [],
+  null,
+  function() {},
+  Symbol(),
+  true,
+  Infinity,
+].forEach((maxTimeout) => {
+  try {
+    new dns.Resolver({ maxTimeout });
+  } catch (e) {
+    assert.ok(/ERR_OUT_OF_RANGE|ERR_INVALID_ARG_TYPE/i.test(e.code));
+  }
+});
+
+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, maxTimeout: 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;
+  }
+}