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

[durran]

Description

Refactors all dns resolution to only go through lookup and resolve.

Summary of Changes

• Refactors dns resolution to only use lookup and resolves
• Fixes all tests

What is the motivation for this change?

NODE-7313

Release Highlight

Release notes highlight

Double check the following

• Lint is passing (npm run check:lint)
• Self-review completed using the steps outlined here
• PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Example: feat(NODE-1234)!: rewriting everything in coffeescript
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[baileympearson]

Can we choose an approach that doesn't necessitate casting? One approach would be something like:

Suggested change

	return async function dnsReqRetryTimeout(lookupAddress: string) {
	try {
	return await dns.promises[api](lookupAddress);
	return (await dns.promises.resolve(lookupAddress, rrtype)) as dns.SrvRecord[] | string[][];
	} catch (firstDNSError) {
	if (firstDNSError.code === dns.TIMEOUT) {
	return await dns.promises[api](lookupAddress);
	return (await dns.promises.resolve(lookupAddress, rrtype)) as dns.SrvRecord[] | string[][];
	} else {
	throw firstDNSError;
	}
	}
	};
	}
	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 resolve(lookupAddress);
	} catch (firstDNSError) {
	if (firstDNSError.code === dns.TIMEOUT) {
	return await resolve(lookupAddress);
	} else {
	throw firstDNSError;
	}
	}
	};

Another would be modifying the function to instead accept a lookup function, which it then calls.

[tadjik1]

@PavelSafronov I have updated this block using generics (basically map rrtype to response). please let me know if this works

[baileympearson]

There's still a cast, it's just been moved to a different place

[tadjik1]

In my view it's essentially the same as you suggested, but with the different mechanism: generics vs. runtime if-else:

const resolve =
    rrtype === 'SRV'
      ? (address: string) => dns.promises.resolve(address, 'SRV')
      : (address: string) => dns.promises.resolve(address, 'TXT');

// is effectively the same as (from the Typescript point of view)

interface DNSLookupMap {
  SRV: dns.SrvRecord[];
  TXT: string[][];
}

const resolve = () => dns.promises.resolve(lookupAddress, rrtype) as Promise<DNSLookupMap[T]>;

In other words, I don't want to add runtime check for the compilation step (it just feels wrong, however I'm not strongly against it).

Would you agree? Or is it casting in general that you would like to avoid?

[baileympearson]

Casting in general. I don't feel terribly strongly, but if we have the capability to write this in way that doesn't require casting, I guess I don't see why we wouldn't?

Runtime checks to have a perf cost (although in general I think an if-statement is effectively negligable). But regardless, this is a factory, so the perf cost is once per module instantiation per function.

[tadjik1]

agree! switched implementation back (with the resolve being defined with ternary condition)