Skip to content

Commit 35f723b

Browse files
nbbeekennbbeeken
committed
feat(NODE-6451): retry DNS timeout on SRV and TXT lookup
1 parent 41b066b commit 35f723b

File tree

3 files changed

+194
-4
lines changed

3 files changed

+194
-4
lines changed

‎src/connection_string.ts

+24-4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,27 @@ const LB_REPLICA_SET_ERROR = 'loadBalanced option not supported with a replicaSe
5252
const LB_DIRECT_CONNECTION_ERROR =
5353
'loadBalanced option not supported when directConnection is provided';
5454

55+
function retryDNSTimeoutFor(api: 'resolveSrv'): (a: string) => Promise<dns.SrvRecord[]>;
56+
function retryDNSTimeoutFor(api: 'resolveTxt'): (a: string) => Promise<string[][]>;
57+
function retryDNSTimeoutFor(
58+
api: 'resolveSrv' | 'resolveTxt'
59+
): (a: string) => Promise<dns.SrvRecord[] | string[][]> {
60+
return async function dnsReqRetryTimeout(lookupAddress: string) {
61+
try {
62+
return await dns.promises[api](lookupAddress);
63+
} catch (firstDNSError) {
64+
if (firstDNSError.code === dns.TIMEOUT) {
65+
return await dns.promises[api](lookupAddress);
66+
} else {
67+
throw firstDNSError;
68+
}
69+
}
70+
};
71+
}
72+
73+
const resolveSrv = retryDNSTimeoutFor('resolveSrv');
74+
const resolveTxt = retryDNSTimeoutFor('resolveTxt');
75+
5576
/**
5677
* Lookup a `mongodb+srv` connection string, combine the parts and reparse it as a normal
5778
* connection string.
@@ -67,14 +88,13 @@ export async function resolveSRVRecord(options: MongoOptions): Promise<HostAddre
6788
// Asynchronously start TXT resolution so that we do not have to wait until
6889
// the SRV record is resolved before starting a second DNS query.
6990
const lookupAddress = options.srvHost;
70-
const txtResolutionPromise = dns.promises.resolveTxt(lookupAddress);
91+
const txtResolutionPromise = resolveTxt(lookupAddress);
7192

7293
txtResolutionPromise.then(undefined, squashError); // rejections will be handled later
7394

95+
const hostname = `_${options.srvServiceName}._tcp.${lookupAddress}`;
7496
// Resolve the SRV record and use the result as the list of hosts to connect to.
75-
const addresses = await dns.promises.resolveSrv(
76-
`_${options.srvServiceName}._tcp.${lookupAddress}`
77-
);
97+
const addresses = await resolveSrv(hostname);
7898

7999
if (addresses.length === 0) {
80100
throw new MongoAPIError('No addresses found at host');

‎src/mongo_client.ts

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

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

+162
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import { expect } from 'chai';
2+
import * as dns from 'dns';
3+
import * as sinon from 'sinon';
4+
5+
import { MongoClient } from '../../mongodb';
6+
7+
const metadata: MongoDBMetadataUI = { requires: { topology: '!single' } };
8+
9+
// This serves as a placeholder for _whatever_ node.js may throw. We only rely upon `.code`
10+
class DNSTimeoutError extends Error {
11+
code = 'ETIMEOUT';
12+
}
13+
// This serves as a placeholder for _whatever_ node.js may throw. We only rely upon `.code`
14+
class DNSSomethingError extends Error {
15+
code = undefined;
16+
}
17+
18+
const CONNECTION_STRING = `mongodb+srv://test1.test.build.10gen.cc`;
19+
// 27018 localhost.test.build.10gen.cc.
20+
// 27017 localhost.test.build.10gen.cc.
21+
22+
describe('DNS timeout errors', () => {
23+
let client: MongoClient;
24+
25+
beforeEach(async function () {
26+
client = new MongoClient(CONNECTION_STRING, { serverSelectionTimeoutMS: 2000, tls: false });
27+
});
28+
29+
afterEach(async function () {
30+
sinon.restore();
31+
await client.close();
32+
});
33+
34+
const restoreDNS =
35+
api =>
36+
async (...args) => {
37+
sinon.restore();
38+
return await dns.promises[api](...args);
39+
};
40+
41+
describe('when SRV record look up times out', () => {
42+
beforeEach(() => {
43+
sinon
44+
.stub(dns.promises, 'resolveSrv')
45+
.onFirstCall()
46+
.rejects(new DNSTimeoutError())
47+
.onSecondCall()
48+
.callsFake(restoreDNS('resolveSrv'));
49+
});
50+
51+
afterEach(async function () {
52+
sinon.restore();
53+
});
54+
55+
it('retries timeout error', metadata, async () => {
56+
await client.connect();
57+
});
58+
});
59+
60+
describe('when TXT record look up times out', () => {
61+
beforeEach(() => {
62+
sinon
63+
.stub(dns.promises, 'resolveTxt')
64+
.onFirstCall()
65+
.rejects(new DNSTimeoutError())
66+
.onSecondCall()
67+
.callsFake(restoreDNS('resolveTxt'));
68+
});
69+
70+
afterEach(async function () {
71+
sinon.restore();
72+
});
73+
74+
it('retries timeout error', metadata, async () => {
75+
await client.connect();
76+
});
77+
});
78+
79+
describe('when SRV record look up times out twice', () => {
80+
beforeEach(() => {
81+
sinon
82+
.stub(dns.promises, 'resolveSrv')
83+
.onFirstCall()
84+
.rejects(new DNSTimeoutError())
85+
.onSecondCall()
86+
.rejects(new DNSTimeoutError())
87+
.onThirdCall()
88+
.callsFake(restoreDNS('resolveSrv'));
89+
});
90+
91+
afterEach(async function () {
92+
sinon.restore();
93+
});
94+
95+
it('throws timeout error', metadata, async () => {
96+
const error = await client.connect().catch(error => error);
97+
expect(error).to.be.instanceOf(DNSTimeoutError);
98+
});
99+
});
100+
101+
describe('when TXT record look up times out twice', () => {
102+
beforeEach(() => {
103+
sinon
104+
.stub(dns.promises, 'resolveTxt')
105+
.onFirstCall()
106+
.rejects(new DNSTimeoutError())
107+
.onSecondCall()
108+
.rejects(new DNSTimeoutError())
109+
.onThirdCall()
110+
.callsFake(restoreDNS('resolveTxt'));
111+
});
112+
113+
afterEach(async function () {
114+
sinon.restore();
115+
});
116+
117+
it('throws timeout error', metadata, async () => {
118+
const error = await client.connect().catch(error => error);
119+
expect(error).to.be.instanceOf(DNSTimeoutError);
120+
});
121+
});
122+
123+
describe('when SRV record look up throws a non-timeout error', () => {
124+
beforeEach(() => {
125+
sinon
126+
.stub(dns.promises, 'resolveSrv')
127+
.onFirstCall()
128+
.rejects(new DNSSomethingError())
129+
.onSecondCall()
130+
.callsFake(restoreDNS('resolveSrv'));
131+
});
132+
133+
afterEach(async function () {
134+
sinon.restore();
135+
});
136+
137+
it('throws that error', metadata, async () => {
138+
const error = await client.connect().catch(error => error);
139+
expect(error).to.be.instanceOf(DNSSomethingError);
140+
});
141+
});
142+
143+
describe('when TXT record look up throws a non-timeout error', () => {
144+
beforeEach(() => {
145+
sinon
146+
.stub(dns.promises, 'resolveTxt')
147+
.onFirstCall()
148+
.rejects(new DNSSomethingError())
149+
.onSecondCall()
150+
.callsFake(restoreDNS('resolveTxt'));
151+
});
152+
153+
afterEach(async function () {
154+
sinon.restore();
155+
});
156+
157+
it('throws that error', metadata, async () => {
158+
const error = await client.connect().catch(error => error);
159+
expect(error).to.be.instanceOf(DNSSomethingError);
160+
});
161+
});
162+
});

0 commit comments

Comments
 (0)