feat(dns): added support for a custom lookup function; (#5339)

[MitchLewis930]

PR_026

Summary by CodeRabbit

• New Features

  • Added family configuration option (IPv4/IPv6) to customize address family for requests.
  • Added lookup configuration option supporting both callback and promise-based custom DNS resolution.

• Tests

  • Added tests for custom DNS lookup functionality with synchronous and asynchronous resolvers.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds DNS lookup customization and address family configuration capabilities to axios. It introduces type definitions, a new callbackify utility for wrapping async functions, helper predicates for function type detection, HTTP adapter implementation for handling these options, and comprehensive tests with examples.

Changes

Cohort / File(s)	Summary
Type Definitions index.d.cts, index.d.ts	Added family (4 | 6) and lookup (callback or promise-based DNS resolver) as optional properties to AxiosRequestConfig interface.
Utility Helpers lib/helpers/callbackify.js, lib/utils.js	New callbackify module wraps async functions into callback-style wrappers with optional reducer. Added function type detectors: isAsyncFn, isGeneratorFn, isAsyncGeneratorFn, isPlainFunction, and isThenable predicate.
HTTP Adapter Implementation lib/adapters/http.js	Integrated lookup and family config options; async lookup functions are wrapped via callbackify with normalization logic to convert promise results to [ip, family] tuples; both options propagated to underlying HTTP/HTTPS request.
Tests & Documentation CHANGELOG.md, test/module/typings/esm/index.ts, test/unit/adapters/http.js	Added contributor entry, TypeScript examples demonstrating synchronous and asynchronous lookup usage, and new DNS test suite validating custom lookup callback behavior with stream payloads.

Sequence Diagram

sequenceDiagram
    participant Client as User Code
    participant Config as Axios Config
    participant Adapter as HTTP Adapter
    participant Callbackify as Callbackify Helper
    participant LookupFn as Lookup Function
    participant Transport as HTTP/HTTPS Transport

    Client->>Config: Pass config with async lookup function
    Config->>Adapter: Initialize HTTP adapter with config
    Adapter->>Adapter: Destructure lookup and family from config
    alt lookup is async function
        Adapter->>Callbackify: Wrap async lookup with callbackify
        Callbackify->>Callbackify: Check if function is async
        Note over Callbackify: Create callback-style wrapper
    else lookup is sync callback
        Adapter->>Adapter: Use lookup as-is
    end
    Adapter->>LookupFn: Call lookup(hostname, options, callback)
    LookupFn->>LookupFn: Resolve address and family
    LookupFn->>Adapter: Return result via callback
    Adapter->>Adapter: Normalize result to [ip, family] pair
    Adapter->>Transport: Pass lookup and family in request options
    Transport->>Transport: Use custom DNS resolver

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A whisker-twitching contribution so fine,
DNS lookups now dance in a queued design,
Async becomes sync through callbackify's spell,
Address families aligned, all working well!
From helpers to adapters, the network takes flight,
Hopping faster with family fourscore and sight! 🌐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding support for custom DNS lookup functions in axios.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

lib/utils.js (1)

665-669: Consider renaming the destructured constructor parameter to avoid shadowing the global.

The static analysis tool flags this as shadowing the global constructor property. While the code is functionally correct, renaming improves clarity.

♻️ Suggested refactor

-const [isPlainFunction, isAsyncFn, isGeneratorFn, isAsyncGeneratorFn] = (
-  (...fns) => fns.map(
-    ({constructor})=> (thing) => thing && typeof thing === 'function' && thing.constructor === constructor
-  )
-)(()=> {}, async()=>{}, function*(){}, async function*(){});
+const [isPlainFunction, isAsyncFn, isGeneratorFn, isAsyncGeneratorFn] = (
+  (...fns) => fns.map(
+    ({constructor: ctor}) => (thing) => thing && typeof thing === 'function' && thing.constructor === ctor
+  )
+)(()=> {}, async()=>{}, function*(){}, async function*(){});

lib/helpers/callbackify.js (1)

3-14: Consider adding validation for the callback argument.

If the wrapped function is called without a callback as the last argument, cb will be undefined and the code will throw when attempting to invoke it. This could happen if the function is misused.

🛡️ Defensive validation suggestion

 const callbackify = (fn, reducer) => {
   return utils.isAsyncFn(fn) ? function (...args) {
     const cb = args.pop();
+    if (typeof cb !== 'function') {
+      throw new TypeError('The last argument must be a callback function');
+    }
     fn.apply(this, args).then((value) => {
       try {
         reducer ? cb(null, ...reducer(value)) : cb(null, value);
       } catch (err) {
         cb(err);
       }
     }, cb);
   } : fn;
 }

test/module/typings/esm/index.ts (1)

638-643: The "async" lookup example is not actually asynchronous.

The comment indicates this is an async lookup example, but the function body returns a synchronous array ['127.0.0.1', 4] without async keyword or returning a Promise. To properly demonstrate the async lookup pattern, consider:

✨ Proposed fix to make the example actually async

 // lookup async
 axios.get('/user', {
-  lookup: (hostname: string, opt: object) => {
-    return ['127.0.0.1', 4];
+  lookup: async (hostname: string, opt: object) => {
+    return ['127.0.0.1', 4] as [string, number];
   }
 });

test/unit/adapters/http.js (1)

2131-2136: Consider verifying the hostname parameter passed to the lookup function.

The test verifies that the lookup function is called but doesn't assert that the correct hostname (fake-name.axios) was passed. This could help catch regressions where the hostname isn't properly forwarded.

✨ Suggested enhancement

       let isCalled = false;
+      let receivedHostname = null;

       const {data} = await axios.post(`http://fake-name.axios:4444`, payload,{
         lookup: (hostname, opt, cb) =>  {
           isCalled = true;
+          receivedHostname = hostname;
           cb(null, '127.0.0.1', 4);
         }
       });

       assert.ok(isCalled);
+      assert.strictEqual(receivedHostname, 'fake-name.axios');

       assert.strictEqual(data, payload);

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6f7053 and 2701911.

📒 Files selected for processing (8)

• CHANGELOG.md
• index.d.cts
• index.d.ts
• lib/adapters/http.js
• lib/helpers/callbackify.js
• lib/utils.js
• test/module/typings/esm/index.ts
• test/unit/adapters/http.js

🧰 Additional context used

🧬 Code graph analysis (3)

index.d.ts (1)

lib/adapters/http.js (1)

• options (386-397)

lib/adapters/http.js (1)

lib/helpers/callbackify.js (1)

• callbackify (3-14)

index.d.cts (2)

lib/adapters/http.js (1)

• options (386-397)

lib/helpers/callbackify.js (1)

• cb (5-5)

🪛 Biome (2.3.13)

lib/utils.js

[error] 667-667: Do not shadow the global "constructor" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

🔇 Additional comments (11)

CHANGELOG.md (1)

517-517: LGTM!

The contributor entry is appropriately added.

index.d.ts (1)

347-349: LGTM!

The new family and lookup properties align with the HTTP adapter implementation in lib/adapters/http.js and follow Node.js dns.lookup conventions. The type union for lookup correctly supports both callback-based (synchronous) and promise-based (async) forms.

lib/utils.js (2)

671-672: LGTM!

The isThenable implementation correctly detects promise-like objects by checking for both then and catch methods, which is the standard pattern for thenable detection.

---

723-728: LGTM!

The new utility functions are correctly exported alongside existing utilities.

lib/adapters/http.js (3)

26-26: LGTM!

The import of callbackify helper is correctly placed with other helper imports.

---

157-166: LGTM!

The async lookup handling is well-implemented:

• Correctly detects async functions using utils.isAsyncFn
• Normalizes string results to [ip, family] tuples with sensible IPv4/IPv6 detection based on dot presence
• Provides a clear error message for invalid return shapes
• Uses callbackify to convert the promise-based function to Node's expected callback signature

---

393-394: LGTM!

The family and lookup options are correctly propagated to the transport request options, enabling custom DNS resolution strategies.

index.d.cts (1)

406-408: LGTM!

The type definitions are consistent with index.d.ts, ensuring parity between ESM and CommonJS type declarations for the new family and lookup configuration options.

test/module/typings/esm/index.ts (1)

630-636: LGTM!

The callback-based lookup example correctly demonstrates the synchronous callback pattern matching Node.js dns.lookup signature.

test/unit/adapters/http.js (2)

59-59: LGTM!

Clean and reusable stream echo handler for tests.

---

2123-2180: Good test coverage for the new DNS lookup feature.

The tests cover the three main lookup patterns:

1. Callback-based synchronous lookup
2. Async lookup returning [address, family] tuple
3. Async lookup returning only the IP address

Consider adding a test for error handling when the lookup function fails (e.g., calling cb(new Error('DNS resolution failed')) or rejecting the promise in the async version). This would ensure proper error propagation to the axios consumer.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}