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

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_026


Note

Medium Risk
Touches the core Node HTTP adapter request option wiring (DNS resolution path), which could affect connectivity/redirect behavior, but is scoped and covered by new unit/typing tests.

Overview
Adds Node-only request config support for DNS control by passing family and a custom lookup function through to the underlying Node http(s).request options.

Async lookup functions are now accepted by wrapping them into callback style via a new callbackify helper, and typings + tests were updated to cover both callback and async lookup variants (including async returning just an IP string).

Written by Cursor Bugbot for commit 2701911. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

lookup: (hostname: string, opt: object) => {
return ['127.0.0.1', 4];
}
});
Copy link

Choose a reason for hiding this comment

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

TypeScript async lookup test missing async keyword

Medium Severity

The "lookup async" TypeScript test is missing the async keyword. The function returns a plain array ['127.0.0.1', 4] instead of a Promise as required by the type definition Promise<[address: string, family: number] | string>. This causes a type mismatch and doesn't correctly validate the async lookup type signature. The runtime test at test/unit/adapters/http.js:2151 correctly uses async.

Fix in Cursor Fix in Web

}
return entry;
})
}
Copy link

Choose a reason for hiding this comment

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

Promise-returning lookup functions not detected unless using async

Low Severity

The type definition allows any function returning Promise<[address, family] | string>, but the implementation only detects functions declared with the async keyword via utils.isAsyncFn(). A regular function that returns a Promise (e.g., function lookup(h, o) { return Promise.resolve(['127.0.0.1', 4]); }) won't be wrapped by callbackify, causing the HTTP request to hang since the callback is never invoked.

Additional Locations (1)

Fix in Cursor Fix in Web

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