Copilot commented on code in PR #479:
URL:
https://github.com/apache/cassandra-nodejs-driver/pull/479#discussion_r4221331484
##########
lib/control-connection.js:
##########
@@ -895,40 +895,21 @@ class ControlConnection extends events.EventEmitter {
}
/**
- * Uses the DNS protocol to resolve a IPv4 and IPv6 addresses (A and AAAA
records) for the hostname.
+ * Uses the operating system resolver to resolve all IPv4 and IPv6 addresses
for the hostname.
* It returns an Array of addresses that can be empty and logs the error.
* @private
* @param name
*/
async _resolveAll(name) {
- const addresses = [];
- const resolve4 = util.promisify(dns.resolve4);
- const resolve6 = util.promisify(dns.resolve6);
const lookup = util.promisify(dns.lookup);
- // Ignore errors for resolve calls
- const ipv4Promise = resolve4(name).catch(() => {}).then(r => r ||
utils.emptyArray);
- const ipv6Promise = resolve6(name).catch(() => {}).then(r => r ||
utils.emptyArray);
-
- let arr;
- arr = await ipv4Promise;
- arr.forEach(address => addresses.push({address, isIPv6: false}));
-
- arr = await ipv6Promise;
- arr.forEach(address => addresses.push({address, isIPv6: true}));
-
- if (addresses.length === 0) {
- // In case dns.resolve*() methods don't yield a valid address for the
host name
- // Use system call getaddrinfo() that might resolve according to host
system definitions
- try {
- arr = await lookup(name, { all: true });
- arr.forEach(({address, family}) => addresses.push({address, isIPv6:
family === 6}));
- } catch (err) {
- this.log('error', `Host with name ${name} could not be resolved`, err);
- }
+ try {
+ const addresses = await lookup(name, { all: true });
Review Comment:
`dns.lookup()` result order can be affected by Node's default result order
(e.g., via `dns.setDefaultResultOrder('ipv4first')`), which may undermine the
PR goal of following OS/system resolver policy for address preference. Consider
passing `verbatim: true` (i.e., `{ all: true, verbatim: true }`) so the driver
always uses the OS-provided ordering regardless of process-wide Node settings.
Tests that assert the options object would need to be updated accordingly.
##########
lib/control-connection.js:
##########
@@ -895,40 +895,21 @@ class ControlConnection extends events.EventEmitter {
}
/**
- * Uses the DNS protocol to resolve a IPv4 and IPv6 addresses (A and AAAA
records) for the hostname.
+ * Uses the operating system resolver to resolve all IPv4 and IPv6 addresses
for the hostname.
* It returns an Array of addresses that can be empty and logs the error.
* @private
* @param name
*/
async _resolveAll(name) {
- const addresses = [];
- const resolve4 = util.promisify(dns.resolve4);
- const resolve6 = util.promisify(dns.resolve6);
const lookup = util.promisify(dns.lookup);
- // Ignore errors for resolve calls
- const ipv4Promise = resolve4(name).catch(() => {}).then(r => r ||
utils.emptyArray);
- const ipv6Promise = resolve6(name).catch(() => {}).then(r => r ||
utils.emptyArray);
-
- let arr;
- arr = await ipv4Promise;
- arr.forEach(address => addresses.push({address, isIPv6: false}));
-
- arr = await ipv6Promise;
- arr.forEach(address => addresses.push({address, isIPv6: true}));
-
- if (addresses.length === 0) {
- // In case dns.resolve*() methods don't yield a valid address for the
host name
- // Use system call getaddrinfo() that might resolve according to host
system definitions
- try {
- arr = await lookup(name, { all: true });
- arr.forEach(({address, family}) => addresses.push({address, isIPv6:
family === 6}));
- } catch (err) {
- this.log('error', `Host with name ${name} could not be resolved`, err);
- }
+ try {
+ const addresses = await lookup(name, { all: true });
Review Comment:
When the system resolver returns an empty array, `addresses.map(...)`
allocates a new empty array rather than reusing `utils.emptyArray` (which is
returned on error). Consider returning `utils.emptyArray` when
`addresses.length === 0` to keep return values consistent and avoid an
unnecessary allocation in this hot path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
