Skip to content

Comments

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

Open
MitchLewis930 wants to merge 1 commit intopr_026_beforefrom
pr_026_after
Open

feat(dns): added support for a custom lookup function; (#5339)#6
MitchLewis930 wants to merge 1 commit intopr_026_beforefrom
pr_026_after

Conversation

@MitchLewis930
Copy link

PR_026

@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

This PR adds support for custom DNS lookup functions in axios HTTP adapter, allowing users to override DNS resolution behavior for Node.js environments.

Key Changes:

  • Added family and lookup options to AxiosRequestConfig interface
  • Created callbackify helper to convert async lookup functions to callback-based format
  • Added utility functions (isAsyncFn, isGeneratorFn, etc.) to detect function types
  • Implemented automatic IPv4/IPv6 family inference when lookup returns only an IP string
  • Comprehensive test coverage for callback-based, async, and string-return lookup functions

Critical Issue:

  • The IPv4/IPv6 family detection logic on line 160 of lib/adapters/http.js is inverted, which will cause incorrect address family assignment

Testing:

  • Unit tests verify callback, async, and simplified string-return patterns
  • TypeScript type definitions include both function signatures

Confidence Score: 2/5

  • This PR contains a critical logic error in IPv4/IPv6 detection that will cause incorrect behavior
  • The inverted family detection logic will assign family 6 to IPv4 addresses and family 4 to IPv6 addresses, causing connection failures or unexpected behavior. The feature is well-tested and the architecture is sound, but this bug must be fixed before merging.
  • lib/adapters/http.js requires immediate attention to fix the IPv4/IPv6 detection logic on line 160

Important Files Changed

Filename Overview
lib/adapters/http.js Added custom DNS lookup support with async function conversion, but IPv4/IPv6 family detection logic is inverted
lib/helpers/callbackify.js New helper to convert async functions to callback-based functions with optional value transformation
lib/utils.js Added utility functions to detect async/generator functions and thenable objects

Sequence Diagram

sequenceDiagram
    participant User
    participant Axios
    participant httpAdapter
    participant callbackify
    participant lookup as Custom Lookup
    participant NodeHTTP as Node.js HTTP/HTTPS

    User->>Axios: axios.get(url, {lookup, family})
    Axios->>httpAdapter: httpAdapter(config)
    
    alt lookup is async function
        httpAdapter->>callbackify: callbackify(lookup, reducer)
        Note over callbackify: Converts async to callback
        callbackify-->>httpAdapter: callback-based lookup
    end
    
    httpAdapter->>httpAdapter: Build request options
    Note over httpAdapter: Add family & lookup to options
    
    httpAdapter->>NodeHTTP: http.request(options)
    
    alt DNS resolution needed
        NodeHTTP->>lookup: lookup(hostname, options, callback)
        
        alt Original async lookup
            lookup->>lookup: Resolve DNS async
            lookup-->>callbackify: Return [address, family] or string
            callbackify->>callbackify: Apply reducer transform
            callbackify-->>NodeHTTP: callback(null, address, family)
        else Original callback lookup
            lookup->>lookup: Resolve DNS
            lookup-->>NodeHTTP: callback(null, address, family)
        end
    end
    
    NodeHTTP->>NodeHTTP: Connect to resolved address
    NodeHTTP-->>httpAdapter: Response
    httpAdapter-->>Axios: Formatted response
    Axios-->>User: Response data
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

if (lookup && utils.isAsyncFn(lookup)) {
lookup = callbackify(lookup, (entry) => {
if(utils.isString(entry)) {
entry = [entry, entry.indexOf('.') < 0 ? 6 : 4]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPv4/IPv6 detection logic is inverted. IPv4 addresses contain dots (like 127.0.0.1), while IPv6 addresses use colons (like ::1). This will assign family 6 to IPv4 addresses and family 4 to IPv6 addresses.

Suggested change
entry = [entry, entry.indexOf('.') < 0 ? 6 : 4]
entry = [entry, entry.indexOf('.') < 0 ? 4 : 6]
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/adapters/http.js
Line: 160:160

Comment:
IPv4/IPv6 detection logic is inverted. IPv4 addresses contain dots (like `127.0.0.1`), while IPv6 addresses use colons (like `::1`). This will assign family 6 to IPv4 addresses and family 4 to IPv6 addresses.

```suggestion
          entry = [entry, entry.indexOf('.') < 0 ? 4 : 6]
```

How can I resolve this? If you propose a fix, please make it concise.


if (lookup && utils.isAsyncFn(lookup)) {
lookup = callbackify(lookup, (entry) => {
if(utils.isString(entry)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after if keyword for consistency with the codebase style

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/adapters/http.js
Line: 159:159

Comment:
Missing space after `if` keyword for consistency with the codebase style

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants