Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: allow a custom dns lookup function to be supplied #421

Closed
JakeChampion opened this issue Sep 17, 2020 · 9 comments
Closed
Labels
enhancement New feature or request

Comments

@JakeChampion
Copy link

The http built-in module can have a custom lookup function defined, which is great as it enables applications to decide if they want to implement a dns-caching system or something else.

Do we think it is a good idea to also implement such a feature for undici?

@mcollina
Copy link
Member

Sure, would you like to send a PR?

@JakeChampion
Copy link
Author

JakeChampion commented Sep 17, 2020

Would this be the correct change?

Make these net.connect calls take a custom lookup function and default to dns.lookup otherwise?

? net.connect(client[kSocketPath])

Are these other places that would need to be updated also?

@ronag
Copy link
Member

ronag commented Jun 29, 2021

I think this could be resolved through #852

@GeoffreyBooth
Copy link
Member

I’m running into this issue as well. My use case is an app that needs to connect to a server that can be accessed over the public internet but not within the virtual network where my app is running. In an older app with the same need, I wrote this code to patch DNS resolution for requests to use a public DNS server as a fallback when the internal lookup failed:

import { lookup } from 'node:dns'
import http from 'node:http'
import https from 'node:https'

export async function patchDnsResolution() {
	http.globalAgent = new http.Agent({
		keepAlive: true,
		lookup: customLookup,
	})
	https.globalAgent = new https.Agent({
		keepAlive: true,
		lookup: customLookup,
	})
}

function customLookup(hostname, _family, callback) {
	lookup(hostname, 4, (lookupError, address, family) => {
		if (hostname === 'localhost') {
			return callback(undefined, '127.0.0.1', 4)
		}

		if (lookupError) {
			console.log(`Using public DNS resolver to look up ${hostname}`)
			// https://developers.cloudflare.com/1.1.1.1/encrypted-dns/dns-over-https/make-api-requests
			fetchWithTimeout(`https://1.1.1.1/dns-query?name=${encodeURIComponent(hostname)}`, {
				headers: {
					accept: 'application/dns-json',
				},
			})
				.then(response => response.json())
				.then(response => {
					if (response?.Answer) {
						// https://developers.cloudflare.com/1.1.1.1/encrypted-dns/dns-over-https/make-api-requests/dns-json
						// Find the first IPv4 or IPv6 address; https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4
						const { data: address, type } = response.Answer.find(({ type }) => type === 1 || type === 28)
						family = type === 1 ? 4 : 6 // IPv4 type is 1, IPv6 type is 28
						callback(undefined, address, family)
					} else {
						callback(new Error(`Public DNS resolver unrecognized response: ${response}`))
					}
				}).catch(error => {
					console.error(`Public DNS resolver error: ${error}`)
				})
		} else {
			callback(undefined, address, family)
		}
	})
}

async function fetchWithTimeout(resource, options = {timeout: 2000}) {
	const { timeout } = options
	const controller = new AbortController()
	const id = setTimeout(() => controller.abort(), timeout)
	const response = await fetch(resource, {
		...options,
		signal: controller.signal
	})
	clearTimeout(id)
	return response
}

In my new app, one of its dependencies uses Node’s global fetch to make a call to this unreachable server. The above code doesn’t patch the DNS resolution of global fetch, but there doesn’t appear to be a way to do an equivalent for Undici or global fetch.

@mcollina mcollina added the enhancement New feature or request label Mar 15, 2023
@mcollina
Copy link
Member

@ronag @GeoffreyBooth shouldn't this be solved by using the connect() option?

@ronag
Copy link
Member

ronag commented Mar 15, 2023

The connect option solves this.

@GeoffreyBooth
Copy link
Member

The connect option solves this.

Can you provide a working example?

@michael-braun
Copy link

I think I've got it working by using an Agent-instance with the connect-option. I don't know whether this is the preferred option, but it seems to work:

import { Agent } from 'undici';

await fetch('https://example.com', {
  dispatcher: new Agent({
    connect: {
      lookup: (hostname, options, callback) => {
        return dns.lookup(hostname, options, callback);
      },
    },
  }),
});

Alternatively, the Agent can be registered globally using the setGlobalDispatcher function from the undici package.

@Chadzzz1
Copy link

Chadzzz1 commented May 5, 2023

****

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants