Skip to content

refactor(NODE-7313): only use dns.resolve for all types #4846

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.

Sign up for GitHub

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
PavelSafronov merged 15 commits into from
Mar 16, 2026
Merged

refactor(NODE-7313): only use dns.resolve for all types #4846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4e89a87
refactor(NODE-7313): only use dns.resolve for all types
durran Jan 22, 2026
e024ec6
test: stub only resolve
durran Feb 4, 2026
3ade98f
test: fix gssapi tests
durran Feb 4, 2026
38b46c1
test: fix initial dns seedlist test
durran Feb 4, 2026
9b5b0a8
update sinon stubs
tadjik1 Feb 6, 2026
e7e43d3
make sure to check stubs with arguments
tadjik1 Feb 6, 2026
52aa2e9
run kerberos tests only
tadjik1 Feb 6, 2026
68008ba
fix kerberos tests
tadjik1 Feb 6, 2026
bb16669
run all tests
tadjik1 Feb 9, 2026
481b7b1
address typescript type comment
tadjik1 Feb 17, 2026
9a631da
fix typo in test
tadjik1 Feb 18, 2026
c342496
avoid ts casting
tadjik1 Feb 18, 2026
d992590
Merge branch 'main' into NODE-7313
PavelSafronov Feb 26, 2026
9635d93
Apply suggestions from code review
tadjik1 Mar 13, 2026
dc90501
make tests more robust
tadjik1 Mar 13, 2026
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
4 changes: 2 additions & 2 deletions src/cmap/auth/gssapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export async function performGSSAPICanonicalizeHostName(

try {
// Perform a reverse ptr lookup on the ip address.
const results = await dns.promises.resolvePtr(address);
const results = await dns.promises.resolve(address, 'PTR');
// If the ptr did not error but had no results, return the host.
return results.length > 0 ? results[0] : host;
} catch {
Expand All @@ -188,7 +188,7 @@ export async function performGSSAPICanonicalizeHostName(
export async function resolveCname(host: string): Promise<string> {
// Attempt to resolve the host name
try {
const results = await dns.promises.resolveCname(host);
const results = await dns.promises.resolve(host, 'CNAME');
// Get the first resolved host id
return results.length > 0 ? results[0] : host;
} catch {
Expand Down
20 changes: 12 additions & 8 deletions src/connection_string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,26 +42,30 @@ 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(rrtype: 'SRV'): (lookupAddress: string) => Promise<dns.SrvRecord[]>;
function retryDNSTimeoutFor(rrtype: 'TXT'): (lookupAddress: string) => Promise<string[][]>;
function retryDNSTimeoutFor(
api: 'resolveSrv' | 'resolveTxt'
): (a: string) => Promise<dns.SrvRecord[] | string[][]> {
rrtype: 'SRV' | 'TXT'
): (lookupAddress: string) => Promise<dns.SrvRecord[] | string[][]> {
const resolve =
rrtype === 'SRV'
? (address: string) => dns.promises.resolve(address, 'SRV')
: (address: string) => dns.promises.resolve(address, 'TXT');
return async function dnsReqRetryTimeout(lookupAddress: string) {
try {
return await dns.promises[api](lookupAddress);
return await resolve(lookupAddress);
} catch (firstDNSError) {
if (firstDNSError.code === dns.TIMEOUT) {
return await dns.promises[api](lookupAddress);
return await resolve(lookupAddress);
} else {
throw firstDNSError;
}
}
};
}

const resolveSrv = retryDNSTimeoutFor('resolveSrv');
const resolveTxt = retryDNSTimeoutFor('resolveTxt');
const resolveSrv = retryDNSTimeoutFor('SRV');
const resolveTxt = retryDNSTimeoutFor('TXT');

/**
* Lookup a `mongodb+srv` connection string, combine the parts and reparse it as a normal
Expand Down
2 changes: 1 addition & 1 deletion src/sdam/srv_polling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class SrvPoller extends TypedEventEmitter<SrvPollerEvents> {
let srvRecords;

try {
srvRecords = await dns.promises.resolveSrv(this.srvAddress);
srvRecords = await dns.promises.resolve(this.srvAddress, 'SRV');
} catch {
this.failure();
return;
Expand Down
79 changes: 27 additions & 52 deletions test/integration/initial-dns-seedlist-discovery/dns_seedlist.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('DNS timeout errors', () => {

beforeEach(async function () {
client = new MongoClient(CONNECTION_STRING, { serverSelectionTimeoutMS: 2000, tls: false });
stub = sinon.stub(dns.promises, 'resolve').callThrough();
});

afterEach(async function () {
Expand All @@ -31,134 +32,108 @@ describe('DNS timeout errors', () => {
await client.close();
});

const restoreDNS =
api =>
async (...args) => {
sinon.restore();
return await dns.promises[api](...args);
};
const restoreDNS = rrtype => async hostname => {
stub.restore();
return await dns.promises.resolve(hostname, rrtype);
};

describe('when SRV record look up times out', () => {
beforeEach(() => {
stub = sinon
.stub(dns.promises, 'resolveSrv')
stub
.withArgs(sinon.match.string, 'SRV')
.onFirstCall()
.rejects(new DNSTimeoutError())
.onSecondCall()
.callsFake(restoreDNS('resolveSrv'));
});

afterEach(async function () {
sinon.restore();
.callsFake(restoreDNS('SRV'));
});

it('retries timeout error', metadata, async () => {
await client.connect();
expect(stub).to.have.been.calledTwice;
expect(stub.withArgs(sinon.match.string, 'SRV')).to.have.been.calledTwice;
});
});

describe('when TXT record look up times out', () => {
beforeEach(() => {
stub = sinon
.stub(dns.promises, 'resolveTxt')
stub
.withArgs(sinon.match.string, 'TXT')
.onFirstCall()
.rejects(new DNSTimeoutError())
.onSecondCall()
.callsFake(restoreDNS('resolveTxt'));
});

afterEach(async function () {
sinon.restore();
.callsFake(restoreDNS('TXT'));
});

it('retries timeout error', metadata, async () => {
await client.connect();
expect(stub).to.have.been.calledTwice;
expect(stub.withArgs(sinon.match.string, 'TXT')).to.have.been.calledTwice;
});
});

describe('when SRV record look up times out twice', () => {
beforeEach(() => {
stub = sinon
.stub(dns.promises, 'resolveSrv')
stub
.withArgs(sinon.match.string, 'SRV')
.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;
expect(stub.withArgs(sinon.match.string, 'SRV')).to.have.been.calledTwice;
});
});

describe('when TXT record look up times out twice', () => {
beforeEach(() => {
stub = sinon
.stub(dns.promises, 'resolveTxt')
stub
.withArgs(sinon.match.string, 'TXT')
.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;
expect(stub.withArgs(sinon.match.string, 'TXT')).to.have.been.calledTwice;
});
});

describe('when SRV record look up throws a non-timeout error', () => {
beforeEach(() => {
stub = sinon
.stub(dns.promises, 'resolveSrv')
stub
.withArgs(sinon.match.string, 'SRV')
.onFirstCall()
.rejects(new DNSSomethingError())
.onSecondCall()
.callsFake(restoreDNS('resolveSrv'));
});

afterEach(async function () {
sinon.restore();
.callsFake(restoreDNS('SRV'));
});

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;
expect(stub.withArgs(sinon.match.string, 'SRV')).to.have.been.calledOnce;
});
});

describe('when TXT record look up throws a non-timeout error', () => {
beforeEach(() => {
stub = sinon
.stub(dns.promises, 'resolveTxt')
stub
.withArgs(sinon.match.string, 'TXT')
.onFirstCall()
.rejects(new DNSSomethingError())
.onSecondCall()
.callsFake(restoreDNS('resolveTxt'));
});

afterEach(async function () {
sinon.restore();
.callsFake(restoreDNS('TXT'));
});

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;
expect(stub.withArgs(sinon.match.string, 'TXT')).to.have.been.calledOnce;
});
});
});
35 changes: 22 additions & 13 deletions test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
beforeEach(async function () {
// this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation

sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
const stub = sinon.stub(dns.promises, 'resolve');
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'resolved.mongo.localhost',
Expand All @@ -30,7 +31,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
];
});

sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
throw { code: 'ENODATA' };
});

Expand Down Expand Up @@ -80,9 +81,11 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
* - the SRV mongodb+srv://blogs.mongodb.com resolving to blogs.evil.com
* Remember, the domain of an SRV with one or two . separated parts is the SRVs entire hostname.
*/
let stub;

beforeEach(async function () {
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
stub = sinon.stub(dns.promises, 'resolve');
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
throw { code: 'ENODATA' };
});
});
Expand All @@ -92,7 +95,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with one domain level causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'localhost.mongodb',
Expand All @@ -111,7 +114,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with two domain levels causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it!
Expand All @@ -130,7 +133,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with three or more domain levels causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'blogs.evil.com',
Expand Down Expand Up @@ -162,8 +165,11 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
context(
'when given a host from DNS resolution that is identical to the original SRVs hostname',
function () {
let stub;

beforeEach(async function () {
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
stub = sinon.stub(dns.promises, 'resolve');
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
throw { code: 'ENODATA' };
});
});
Expand All @@ -173,7 +179,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with one domain level causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'localhost',
Expand All @@ -194,7 +200,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with two domain levels causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'mongo.local',
Expand Down Expand Up @@ -229,8 +235,11 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
context(
'when given a returned address that does NOT share the domain name of the SRV record because its missing a `.`',
function () {
let stub;

beforeEach(async function () {
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
stub = sinon.stub(dns.promises, 'resolve');
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
throw { code: 'ENODATA' };
});
});
Expand All @@ -240,7 +249,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with one domain level causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'test_1.cluster_1localhost',
Expand All @@ -259,7 +268,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with two domain levels causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'test_1.my_hostmongo.local',
Expand All @@ -278,7 +287,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
});

it('an SRV with three domain levels causes a runtime error', async function () {
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
return [
{
name: 'cluster.testmongodb.com',
Expand Down
Loading
Loading