Skip to content

Commit

Permalink
feat(NODE-6451): retry SRV and TXT lookup for DNS timeout errors (#4375)
Browse files Browse the repository at this point in the history
Co-authored-by: Aditi Khare <[email protected]>
  • Loading branch information
nbbeeken and aditi-khare-mongoDB authored Jan 24, 2025
1 parent 70d476a commit fd902d3
Show file tree
Hide file tree
Showing 3 changed files with 196 additions and 4 deletions.
28 changes: 24 additions & 4 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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');
Expand Down
8 changes: 8 additions & 0 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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> {
Expand Down
164 changes: 164 additions & 0 deletions test/integration/initial-dns-seedlist-discovery/dns_seedlist.test.ts
Original file line number Diff line number Diff line change
@@ -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;
});
});
});

0 comments on commit fd902d3

Please sign in to comment.