Skip to content

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

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

Merged
merged 4 commits into from
Jan 24, 2025
Merged
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
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;
});
});
});
Loading