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

Add dns caching by default #2440

Closed
ronag opened this issue Nov 16, 2023 · 8 comments
Closed

Add dns caching by default #2440

ronag opened this issue Nov 16, 2023 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ronag
Copy link
Member

ronag commented Nov 16, 2023

Something like: https://github.com/szmarczak/cacheable-lookup#readme

Should be an option to disable.

@ronag ronag added enhancement New feature or request good first issue Good for newcomers labels Nov 16, 2023
@SimoneDutto
Copy link

Hi,
I would like to work on this.
What do you mean by adding this to undici?
Is the behavior needed or the whole library?

This would be my first time contributing to an Open Source Project, but I want to start contributing!

@MrSharpp
Copy link

can you state some instructions, where its code should be written?

@Medhansh404
Copy link

Hey!
I'm currently tackling issues #2440 and #1066. My focus is on enhancing the DNS cache lookup functionality. I plan to intercept DNS requests, check the cache before reaching out to the server, and address other caching aspects.

However, I'm facing difficulty locating the specific code related to fetch requests and DNS resolution commands within the files. Your help in navigating and identifying these components would be immensely appreciated!

@Medhansh404
Copy link

"I can come up with ways to approach this issue Planning to add DNS cache lookup. Considering two ways: 1) Middleware like HTTP dispatcher in 'lib,' or 2) Custom Resolver globally. would appreciate if I can get a head's up on the approach and which one will be better(if any) !!! @ronag

@metcoder95
Copy link
Member

The more pragmatic the approach, the better; I'd suggest you take a look at how Socket#connect handles the lookup aspect when establishing a connection; and maybe seek something down that line

antoinegomez added a commit to antoinegomez/undici that referenced this issue Dec 28, 2023
@antoinegomez
Copy link

I had a look last evening and the easiest way I think is to do like @metcoder95 suggest by using the lookup option in the socket module.

My PR is not yet ready, need to finish implementation and refactor a bit. This was mostly to play and see if it works.
I will not implement all options like in this module, otherwise why not simply copy/paste it or add it as a peer dependency and provide an option to configure.

Not sure how to make it configurable in the fetch method to keep it rfc compliant.
Could also have a global config for all requests made via undici.

hjr3 added a commit to hjr3/undici that referenced this issue Apr 19, 2024
We add an optional custom lookup function to that has the same interface
as the lookup option in TCP/UDP socket.connect. This allows users of
this library to implement their own DNS caching implementation.

This does not preclude the future possibility of including a DNS caching
implementation by default.
hjr3 added a commit to hjr3/undici that referenced this issue Apr 19, 2024
We add an optional custom lookup function to that has the same interface
as the lookup option in TCP/UDP socket.connect. This allows users of
this library to implement their own DNS caching implementation.

This does not preclude the future possibility of including a DNS caching
implementation by default.
@KhafraDev
Copy link
Member

@ronag feel free to reopen but as you mentioned it's already possible to do this

import CacheableLookup from 'cacheable-lookup';

const cacheable = new CacheableLookup(opts)

const agent = new Agent({
  connect: { lookup: cacheable.lookup }
})

@metcoder95
Copy link
Member

No strong opinion on this, but I see the potential in having it within core; maybe it will require more thought but as @KhafraDev suggests, is already doable by third-party

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

No branches or pull requests

7 participants