feat(NODE-6451): retry SRV and TXT lookup for DNS timeout errors (#4375)

Co-authored-by: Aditi Khare <[email protected]>

Author: nbbeeken

src/connection_string.ts

@@ -52,6 +52,27 @@ const LB_REPLICA_SET_ERROR = 'loadBalanced option not supported with a replicaSe
 const LB_DIRECT_CONNECTION_ERROR =
   'loadBalanced option not supported when directConnection is provided';
 
+function retryDNSTimeoutFor(api: 'resolveSrv'): (a: string) => Promise<dns.SrvRecord[]>;
+function retryDNSTimeoutFor(api: 'resolveTxt'): (a: string) => Promise<string[][]>;
+function retryDNSTimeoutFor(
+  api: 'resolveSrv' | 'resolveTxt'
+): (a: string) => Promise<dns.SrvRecord[] | string[][]> {
+  return async function dnsReqRetryTimeout(lookupAddress: string) {
+    try {
+      return await dns.promises[api](lookupAddress);
+    } catch (firstDNSError) {
+      if (firstDNSError.code === dns.TIMEOUT) {
+        return await dns.promises[api](lookupAddress);
+      } else {
+        throw firstDNSError;
+      }
+    }
+  };
+}
+
+const resolveSrv = retryDNSTimeoutFor('resolveSrv');
+const resolveTxt = retryDNSTimeoutFor('resolveTxt');
+
 /**
  * Lookup a `mongodb+srv` connection string, combine the parts and reparse it as a normal
  * connection string.
@@ -67,14 +88,13 @@ export async function resolveSRVRecord(options: MongoOptions): Promise<HostAddre
   // Asynchronously start TXT resolution so that we do not have to wait until
   // the SRV record is resolved before starting a second DNS query.
   const lookupAddress = options.srvHost;
-  const txtResolutionPromise = dns.promises.resolveTxt(lookupAddress);
+  const txtResolutionPromise = resolveTxt(lookupAddress);
 
   txtResolutionPromise.then(undefined, squashError); // rejections will be handled later
 
+  const hostname = `_${options.srvServiceName}._tcp.${lookupAddress}`;
   // Resolve the SRV record and use the result as the list of hosts to connect to.
-  const addresses = await dns.promises.resolveSrv(
-    `_${options.srvServiceName}._tcp.${lookupAddress}`
-  );
+  const addresses = await resolveSrv(hostname);
 
   if (addresses.length === 0) {
     throw new MongoAPIError('No addresses found at host');

src/mongo_client.ts

@@ -521,6 +521,10 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
    * This means the time to setup the `MongoClient` does not count against `timeoutMS`.
    * If you are using `timeoutMS` we recommend connecting your client explicitly in advance of any operation to avoid this inconsistent execution time.
    *
+   * @remarks
+   * The driver will look up corresponding SRV and TXT records if the connection string starts with `mongodb+srv://`.
+   * If those look ups throw a DNS Timeout error, the driver will retry the look up once.
+   *
    * @see docs.mongodb.org/manual/reference/connection-string/
    */
   async connect(): Promise<this> {
@@ -727,6 +731,10 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
    * @remarks
    * The programmatically provided options take precedence over the URI options.
    *
+   * @remarks
+   * The driver will look up corresponding SRV and TXT records if the connection string starts with `mongodb+srv://`.
+   * If those look ups throw a DNS Timeout error, the driver will retry the look up once.
+   *
    * @see https://www.mongodb.com/docs/manual/reference/connection-string/
    */
   static async connect(url: string, options?: MongoClientOptions): Promise<MongoClient> {

test/integration/initial-dns-seedlist-discovery/dns_seedlist.test.ts

@@ -0,0 +1,164 @@
+import { expect } from 'chai';
+import * as dns from 'dns';
+import * as sinon from 'sinon';
+
+import { MongoClient } from '../../mongodb';
+
+const metadata: MongoDBMetadataUI = { requires: { topology: '!single' } };
+
+// This serves as a placeholder for _whatever_ node.js may throw. We only rely upon `.code`
+class DNSTimeoutError extends Error {
+  code = 'ETIMEOUT';
+}
+// This serves as a placeholder for _whatever_ node.js may throw. We only rely upon `.code`
+class DNSSomethingError extends Error {
+  code = undefined;
+}
+
+const CONNECTION_STRING = `mongodb+srv://test1.test.build.10gen.cc`;
+
+describe('DNS timeout errors', () => {
+  let client: MongoClient;
+  let stub;
+
+  beforeEach(async function () {
+    client = new MongoClient(CONNECTION_STRING, { serverSelectionTimeoutMS: 2000, tls: false });
+  });
+
+  afterEach(async function () {
+    stub = undefined;
+    sinon.restore();
+    await client.close();
+  });
+
+  const restoreDNS =
+    api =>
+    async (...args) => {
+      sinon.restore();
+      return await dns.promises[api](...args);
+    };
+
+  describe('when SRV record look up times out', () => {
+    beforeEach(() => {
+      stub = sinon
+        .stub(dns.promises, 'resolveSrv')
+        .onFirstCall()
+        .rejects(new DNSTimeoutError())
+        .onSecondCall()
+        .callsFake(restoreDNS('resolveSrv'));
+    });
+
+    afterEach(async function () {
+      sinon.restore();
+    });
+
+    it('retries timeout error', metadata, async () => {
+      await client.connect();
+      expect(stub).to.have.been.calledTwice;
+    });
+  });
+
+  describe('when TXT record look up times out', () => {
+    beforeEach(() => {
+      stub = sinon
+        .stub(dns.promises, 'resolveTxt')
+        .onFirstCall()
+        .rejects(new DNSTimeoutError())
+        .onSecondCall()
+        .callsFake(restoreDNS('resolveTxt'));
+    });
+
+    afterEach(async function () {
+      sinon.restore();
+    });
+
+    it('retries timeout error', metadata, async () => {
+      await client.connect();
+      expect(stub).to.have.been.calledTwice;
+    });
+  });
+
+  describe('when SRV record look up times out twice', () => {
+    beforeEach(() => {
+      stub = sinon
+        .stub(dns.promises, 'resolveSrv')
+        .onFirstCall()
+        .rejects(new DNSTimeoutError())
+        .onSecondCall()
+        .rejects(new DNSTimeoutError());
+    });
+
+    afterEach(async function () {
+      sinon.restore();
+    });
+
+    it('throws timeout error', metadata, async () => {
+      const error = await client.connect().catch(error => error);
+      expect(error).to.be.instanceOf(DNSTimeoutError);
+      expect(stub).to.have.been.calledTwice;
+    });
+  });
+
+  describe('when TXT record look up times out twice', () => {
+    beforeEach(() => {
+      stub = sinon
+        .stub(dns.promises, 'resolveTxt')
+        .onFirstCall()
+        .rejects(new DNSTimeoutError())
+        .onSecondCall()
+        .rejects(new DNSTimeoutError());
+    });
+
+    afterEach(async function () {
+      sinon.restore();
+    });
+
+    it('throws timeout error', metadata, async () => {
+      const error = await client.connect().catch(error => error);
+      expect(error).to.be.instanceOf(DNSTimeoutError);
+      expect(stub).to.have.been.calledTwice;
+    });
+  });
+
+  describe('when SRV record look up throws a non-timeout error', () => {
+    beforeEach(() => {
+      stub = sinon
+        .stub(dns.promises, 'resolveSrv')
+        .onFirstCall()
+        .rejects(new DNSSomethingError())
+        .onSecondCall()
+        .callsFake(restoreDNS('resolveSrv'));
+    });
+
+    afterEach(async function () {
+      sinon.restore();
+    });
+
+    it('throws that error', metadata, async () => {
+      const error = await client.connect().catch(error => error);
+      expect(error).to.be.instanceOf(DNSSomethingError);
+      expect(stub).to.have.been.calledOnce;
+    });
+  });
+
+  describe('when TXT record look up throws a non-timeout error', () => {
+    beforeEach(() => {
+      stub = sinon
+        .stub(dns.promises, 'resolveTxt')
+        .onFirstCall()
+        .rejects(new DNSSomethingError())
+        .onSecondCall()
+        .callsFake(restoreDNS('resolveTxt'));
+    });
+
+    afterEach(async function () {
+      sinon.restore();
+    });
+
+    it('throws that error', metadata, async () => {
+      const error = await client.connect().catch(error => error);
+      expect(error).to.be.instanceOf(DNSSomethingError);
+      expect(stub).to.have.been.calledOnce;
+    });
+  });
+});