fix(dns): handle a null pick, and stop failing over to an expired record

[datrixlab]

This relates to...

Nothing filed — found by reading lib/interceptor/dns.js and reproduced through the public API.

Rationale

Two places where the DNS interceptor mishandles a record it should not use. Each has a sibling in the same file that gets it right, which is what makes them look like oversights rather than choices.

1. runLookup's fresh-lookup branch dereferences pick() without checking it.

const ip = this.pick(origin, records, newOpts.affinity)

let port
if (typeof ip.port === 'number') {

The cached branch twenty lines below does check, and recovers. pick() returns null whenever nothing it holds matches the request — no custom pick needed: with dualStack: false and an affinity set, #defaultPick takes records[affinity] with no fallback. So a host with no AAAA record, requested over IPv6 only, gives:

TypeError: Cannot read properties of null (reading 'port')

thrown inside the DNS callback, where the dispatch error path cannot catch it, instead of the InformationalError('No DNS entries found') raised two lines above for an empty result.

Reproduced on main through the public API:

const client = new Agent().compose(dns({
  dualStack: false,
  affinity: 6,
  lookup (origin, opts, cb) { cb(null, [{ address: '127.0.0.1', family: 4, ttl: 100000 }]) }
}))
await client.request({ method: 'GET', path: '/', origin })
// caught : TypeError |  | Cannot read properties of null (reading 'port')

The fix reports the same error as the empty-result case. It deliberately does not re-run the lookup the way the cached branch does: the records were just fetched, so a second lookup would return the same ones.

2. pickFamily evicts a TTL-expired record and returns it anyway.

if (Date.now() - ip.timestamp > ip.ttl) { // record TTL is already in ms
  // We delete expired records
  // It is possible that they have different TTL, so we manage them individually
  family.ips.splice(position, 1)
}

return ip

#defaultPick carries the identical comment and, after the same splice, re-picks. pickFamily backs the dual-stack fail-over in DNSDispatchHandler.onResponseError, so an ETIMEDOUT/ECONNREFUSED gets retried against an address the interceptor has just dropped as stale.

Worth noting because it changes how reachable this is: the default DNSStorage.set(hostname, records) takes two parameters and drops the { ttl } it is given, and get() never checks expiry. These per-record checks are the only expiry there is, so this is reachable with the default storage — it just needs two records of the same host with different TTLs, which is the normal case for a dual-stack host.

Reproduced on main, warming the cache while the server is up, then letting the IPv6 record go stale and taking the server away:

attempts: ["127.0.0.1","[::1]"]     <- the second is the record just evicted as expired

With the fix, ["127.0.0.1"], and the original ECONNREFUSED surfaces.

Changes

Features

N/A

Bug Fixes

• runLookup reports No DNS entries found when a fresh lookup yields nothing the request can use, instead of throwing a TypeError on null.
• pickFamily re-picks after evicting an expired record, so the dual-stack fail-over cannot retry at a stale address.

Breaking Changes and Deprecations

None. Both paths currently end in a crash or a doomed retry.

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• [S] Documented
• Review ready
• In review
• Merge ready

Two tests in test/interceptors/dns.js, both through the public API. Reverting only lib/interceptor/dns.js and keeping them fails both, exactly where it should:

✖ Should not fail over to a TTL-expired address (dual stack)
    actual: 2, expected: 1
✖ Should report an unusable lookup result instead of crashing (dual stack disabled)
    actual:   TypeError: Cannot read properties of null (reading 'port')
    expected: { code: 'UND_ERR_INFO', message: 'No DNS entries found' }

node --test test/interceptors/dns.js: 31 passed, 1 failed. That one failure, Should respect DNS origin hostname for SNI on TLS, is pre-existing on this machine — it fails identically with the patch reverted (ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE, a local OpenSSL/cert issue on Windows). eslint is clean on both changed files.

Nothing user-facing changed, so I marked Documented as skipped — happy to add something to docs/docs/api/Interceptors.md if you would rather the null contract for a custom pick were written down there.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (b73952a) to head (0ca89a2).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5791      +/-   ##
==========================================
+ Coverage   93.51%   93.54%   +0.02%     
==========================================
  Files         110      110              
  Lines       39359    39370      +11     
==========================================
+ Hits        36808    36829      +21     
+ Misses       2551     2541      -10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.