szmarczak / cacheable-lookup Goto Github PK

View Code? Open in Web Editor NEW

180.0 180.0 28.0 125 KB

A cacheable dns.lookup(…) that respects TTL :tada:

License: MIT License

JavaScript 95.48% TypeScript 4.52%

cache dns lookup query resolve

cacheable-lookup's Introduction

`HTTP/1.1 418 I'm a teapot`

cacheable-lookup's People

Contributors

Stargazers

Watchers

cacheable-lookup's Issues

Not using local DNS in /etc/resolver/* without internet

When there is no internet connectivity (either interface down, or when there are problems at uplink), cacheable-lookup stops resolving xxx.test records from /etc/resolver/test.

With internet:

> require('got').get('http://django.test:8000/admin/').then(res => console.log(res.body))
Promise { <pending> }
> <!DOCTYPE html>

without internet:

> require('got').get('http://django.test:8000/admin/').then(res => console.log(res.body))
Promise { <pending> }
>
> (node:13938) UnhandledPromiseRejectionWarning: RequestError: ENOTFOUND django.test
...
    at CacheableLookup.lookupAsync (/Users/semenov/test/node_modules/cacheable-lookup/source/index.js:157:18)

for comparison, modules that don't use cacheable-lookup work just fine without internet:

> require('axios').get('http://django.test:8000/admin/').then(res => console.log(res.data))
Promise { <pending> }
> <!DOCTYPE html>

or:

> require('got').get('http://django.test:8000/admin/', { dnsCache: false }).then(res => console.log(res.body))
Promise { <pending> }
> <!DOCTYPE html>

my DNS setup is:

❯ cat /etc/resolver/test
nameserver 127.0.0.1

❯ cat /usr/local/etc/dnsmasq.d/development.conf
address=/.test/127.0.0.1

This is Mac OS 10.15.4.

cacheable-lookup does not appear to refresh cached host-file resolutions when the hosts file is altered

Tested on windows 10, node 12.16.2, via got

clone the repo at https://github.com/fluffynuts/cacheable-lookup-issue-demo
add this line to your hosts file: 127.0.0.1 server.dev.local
npm ci
npm start
in another term, run test.bat

If you get ENOTFOUND for the second test and you're on windows, it may be because your hosts file has CRLF EOL. Change with dos2unix (get from scoop) and try again, until all calls pass

Now, with the api still running, update the hosts file -- delete the line from [2] or alter it, eg 196.25.1.1 server.dev.local.

Expected behavior: if deleting the line, I should get ENOENT, if altering to 196.25.1.1, then I expect an ECONNREFUSED (196.25.1.1 is a dns server for South Africa, iirc)

Actual behavior: even if I wait and retry in 30 seconds, the calls never fail until I restart the api. Then they consistently fail, even if I undo the changes to the system hosts file and wait.

Can you obtain the local hosts first and then use resolve4 or resolve6

Hello, I have a requirement that the local hosts should be used first and then the IP resolved by DNS service should be used. I see in the code you wrote, queryAndCache should use _resolve4 or _resolve6 first and then Lookup. Controls which parsing mode is preferred

Querying for family=6 when family=4 is explicitly requested?

Hi,

I'm trying to understand the design of the module and how it compares to the Node.js core DNS resolution. The Node.js docs state that family can be specified on a request:

family IP address family to use when resolving host or hostname. Valid values are 4 or 6. When unspecified, both IP v4 and v6 will be used.

If I dig into Node.js source code for HTTP requests it looks like this is indeed the behaviour (setting family on the request results in family being passed down into dns.lookup). If we are requesting private DNS records and know only v4 is available it seems to make sense to pass 4 here to avoid the ip v6 lookup being made.

When we use cacheable-lookup we see it requesting both ipv4 and ipv6 regardless of the requested family. Both results are cached and then the cached results are filtered to only return the requested family. This seems different to how a similar module does it (https://github.com/LCMApps/dns-lookup-cache/).

I'm by no means an expert on DNS resolution and best practise, but is this intentional? Is it a bad idea to specify the family on a request?

I can appreciate that this is likely a micro-optimisation and has a maintenance cost if the DNS server is later changed to update the family specified in code from 4 to 6. Saying that, the default configuration of the module is to use a pretty short cache TTL for ENODATA errors so it looks like we do get quite a lot of ipv6 lookups despite them being unsupported, and I'm unsure if increasing the error ttl is a good idea.

Test Windows OS

Let's use GitHub Actions for that and ditch Travis

Add stats to see how many calls were cached

Cacheable lookup responds with ESERVFAIL while native dns.resolve works

Hello,
I have come across a issue where cacheable.lookup or cacheable.lookupAsync was giving ESERVFAIL while dns.resolve or dns.lookup worked perfectly fine.
When i went through the code, i found out that we are trying to resolve for both IPv4 and IPv6 addresses for the domain. If the server sends failure (ESERVFAIL) for the dns lookup since the hostname doesn't exists in the respective IP family, even if it can resolve for the other IP family the library throws the error.
Reference:
[https://github.com/szmarczak/cacheable-lookup/blob/master/source/index.js#L244]
Any reasons where we should lookup for both IPv4 and IPv6 addresses?
const [A, AAAA] = await Promise.all([ ignoreNoResultErrors(this._resolve4(hostname, ttl)), ignoreNoResultErrors(this._resolve6(hostname, ttl)) ]);
For a domain i found that resolve4 was working fine but resolve6 was failing with ESERVFAIL error

jest reports that cacheable-lookup is leaking

This is not my best issue report, since ATM I don't have time to put up the proper minimal repro.

But, when running jest --detectLeaks --detectOpenHandles (I guess only --detectOpenHandles is relevant here) it often prints this:

Test Suites: 1 failed, 11 passed, 12 total
Tests:       1 todo, 17 passed, 18 total
Snapshots:   0 total
Time:        10.615s

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  DNSCHANNEL

    > 1 | import got from 'got'
        | ^
      2 | 
      3 | const detectLeaks = process.argv.some(a => a.includes('detectLeaks'))
      4 | 

      at new CacheableLookup (node_modules/cacheable-lookup/source/index.js:62:14)
      at Object.<anonymous> (node_modules/got/dist/source/index.js:60:19)
      at Object.<anonymous> (src/jestOffline.util.test.ts:1:1)

I'll just leave this issue there until I have more information, or someone else comes with a better reproduction..

got@11 is used here

Module '"dns"' has no exported member 'promises'

I've added got to my project which has cacheable-lookup as a dependency. I can't compile the TypeScript project as I get the following error.

node_modules/cacheable-lookup/index.d.ts(1,34): error TS2305: Module '"dns"' has no exported member 'promises'.

If I remove promises as dnsPromises from the first line of index.d.ts then I can compile the project.
import {Resolver, LookupAddress, promises as dnsPromises} from 'dns';

thanks!

TypeScript: 3.8.3
Node: 14.0.0
cacheable-lookup: 4.1.2

Errors during fallback are no longer handled

In #43, it looks like we slightly changed the error handling behaviour. Repro:

const CacheableLookup = require('cacheable-lookup');

const cl = new CacheableLookup({
    lookup: (hostname, options, callback) => {
        setTimeout(() => {
            callback(Object.assign(new Error('Fake DNS error'), {
                code: 'EAI_AGAIN'
            }));
        }, 10);
    }
});

cl.lookup('example.test', (result) => {
    console.log('Got result', result);
});

In 6.0.1, this returns Error: cacheableLookup ENOTFOUND example.test
In 6.0.2, this returns Error: Fake DNS error

This affects any cases where normal resolution fails with ENOTFOUND or ENODATA, and then fallback fails with some other error. Previously we treated that as an ENOTFOUND result, now we use the fallback's error instead.

I think this comes from c5d06c5 (#43), which removed the try/catch inside _lookup.

I don't think this is necessarily wrong, but it's a change in behaviour that I don't think was intentional. What should happen in this case?

Failure with Node v16.5.0

ModuleNotFoundError: Module not found: Error: Can't resolve 'dns' in '/Users/myuser/code/myproject/node_modules/cacheable-lookup/source'

Trying to install dns, etc manually results in

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '[email protected]',
npm WARN EBADENGINE required: { node: '>= 0.10.0 < 0.11.0' },
npm WARN EBADENGINE current: { node: 'v16.5.0', npm: '7.19.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '[email protected]',
npm WARN EBADENGINE required: { node: '0.8.x' },
npm WARN EBADENGINE current: { node: 'v16.5.0', npm: '7.19.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '[email protected]',
npm WARN EBADENGINE required: { node: '0.8.x' },
npm WARN EBADENGINE current: { node: 'v16.5.0', npm: '7.19.1' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '[email protected]',
npm WARN EBADENGINE required: { node: '>= 0.8.0 < 0.11.0' },
npm WARN EBADENGINE current: { node: 'v16.5.0', npm: '7.19.1' }
npm WARN EBADENGINE }

Does this module work with my version of node?

Memory leak each time cachableLookup is imported and constructed.

Hi!

I think I've stumbled into a memory leak in cacheable lookup. This leak is present in got, as well as other libraries that are using got, such as webdriver.io.

const CL = require('cacheable-lookup');
new CL();

I'm working on a standalone example to help reproduce the problem.

Respect IPv6 in `hosts` file

DNS over HTTPS support

Does not resolve `localhost` on OS X

Steps for reproduction:
package.json:

{
  "name": "cacheable-lookup-localhost",
  "version": "1.0.0",
  "private": true,
  "dependencies": {
    "cacheable-lookup": "^2.0.1"
  }
}

index.js:

const CacheableLookup = require('cacheable-lookup');
const cacheable = new CacheableLookup();

(async () => {
  try {
    console.log(await cacheable.lookupAsync("localhost"));
  } catch (e) {
    console.log(e);
  }
})();

The output under OS X Mojave:

$  node .
Error: queryA ENOTFOUND localhost
    at QueryReqWrap.onresolve [as oncomplete] (dns.js:207:19) {
  errno: undefined,
  code: 'ENOTFOUND',
  syscall: 'queryA',
  hostname: 'localhost'
}

This problem in consequence makes got to fail with dnsCache enabled on OS X when requests to http://localhost are issued.

P.S. Please note that issue reproduces only on OS X, it does not reproduce neither on Windows nor on Linux

[Question] How to install globally properly

Hello,

I was wondering if global installation needs to wrap just HTTP agent or also HTTPS as well?

const http = require('http');
const CacheableLookup = require('cacheable-lookup');
const cacheable = new CacheableLookup({
    // Set any custom options here
});
cacheable.servers = [
    // Put your DNS servers of choice here
];

// Configure this to be used by a single request:
http.get("http://example.com", {
    lookup: cacheable.lookup
});

Essentially, I should do this:

cacheable.install(http.globalAgent);

or also this?

cacheable.install(http.globalAgent);
cacheable.install(https.globalAgent);

Fallback fails when one family returns ENOENT

Another fallback issue (sorry!). In this case:

I'm not quite sure of the cause, but I'm testing on a Windows machine on a different network.
On this machine, dns.lookup('google.com', { family: 6 }, console.log) prints a ENOENT error (while family: 4 or family: 0 works just fine).
Given that, cacheable-lookup's fallback fails, because that's not an ignored 'no result' error (here). Even though we get a usable IPV4 result, the IPv6 error takes precedence, and fallback fails.

I think the network used here doesn't support IPv6, and so this is how Windows handles that. It would be good if we could still resolve IPv4 in that case though.

I can see a few options:

Add ENOENT as an ignored error (I'm not sure when ENOENT fires for dns.lookup, so I have no idea what the implications of this are!)
Change the _lookup logic so that if one family works but the other throws an error, we use just the working result (and ignore the error, unless both error).

What do you think?

No way to dispose and timeout problems

I've been trying to figure out problems with tests not disposing very well and I appear to have narrowed it down to CacheableLookup, and specifically the timeout within _tick, there's no way to dispose it externally.

Forgive the long stack.

    console.warn
      Uncleared call to setTimeout at "node_modules/timeout-monitor/src/timeout-monitor.js", line 91, column 24
      
      Error: 
          at findCallerLocation (/Users/paul/Documents/GitHub/brand-engine/node_modules/timeout-monitor/src/find-caller-location.js:30:16)
          at monitor._global.setTimeout (/Users/paul/Documents/GitHub/brand-engine/node_modules/timeout-monitor/src/timeout-monitor.js:91:24)
          at CacheableLookup._tick (/Users/paul/Documents/GitHub/brand-engine/node_modules/cacheable-lookup/source/index.js:352:27)
          at CacheableLookup._set (/Users/paul/Documents/GitHub/brand-engine/node_modules/cacheable-lookup/source/index.js:317:10)
          at CacheableLookup.queryAndCache (/Users/paul/Documents/GitHub/brand-engine/node_modules/cacheable-lookup/source/index.js:339:3)
          at CacheableLookup.query (/Users/paul/Documents/GitHub/brand-engine/node_modules/cacheable-lookup/source/index.js:210:15)
          at CacheableLookup.lookupAsync (/Users/paul/Documents/GitHub/brand-engine/node_modules/cacheable-lookup/source/index.js:159:16)

Unless otherwise specified the cache duration is 3600 seconds, or 1 hour. As a result the next tick where this._removalTimeout can be cleared is an hour away which holds the tests open. There needs to be a check after clear() to see if there are any entries in the cache, if not clear the timeout. Failing that there needs to be a dispose function` implemented to clear cache and kill timeouts.

I would appreciate if this fix could be back-ported to the non-ESM release.

Memory leak

Install dnsmasq
Add address=/.test/127.0.0.1 to /etc/dnsmasq.conf
sudo /usr/bin/dnsmasq (start the server)
Run ([email protected]):

const CacheableLookup = require('cacheable-lookup');
const cacheable = new CacheableLookup();

cacheable.servers = ['127.0.0.1'];

setInterval(() => {
	cacheable.lookup(`${Math.random().toString().substr(2)}.test`, () => {});
}, 1);

The memory usage slowly increases every minute. It takes about 10-15 minutes to crash. To cause an instant crash, replace setInterval with while (true).

return IPv6 by default

got v11 introduced DNS cache as a default behavior by using cacheable-lookup.

Unexpectedly, it breaks http request in some IPv6-only environment where the DNS server support
both IPv4 and IPv6.

By default, cacheable-lookup return IPv4 addresses which are unreachable from the server.

With the commonly used library glibc (http://man7.org/linux/man-pages/man3/getaddrinfo.3.html#NOTES), I expect IPv6 addresses without extra configurations in the same situation.

In other words, the following test is expected to be passed.

	{
		const CacheableLookup = mockedInterfaces({has4: false, has6: true});
		const cacheable = new CacheableLookup({resolver, customHostsPath: false});

		verify(t, await cacheable.lookupAsync('localhost'), {
			address: '::ffff:127.0.0.2',
			family: 6
		});
	}

set dns.lookup family by this._iface(has4 and has6)

cacheable-lookup/source/index.js

Line 285 in 5960e1d

all: true

could we set family by this._iface(has4 and has6), not set 0 always.

Respect the`hosts` file

So it doesn't break on internal routes, e.g.

192.168.10.138 database

// Throws because the DNS server has no `database` entry,
// but it's defined in our `hosts` file.
https.request('https://database/get?key=foo', {lookup: cacheable.lookup});

Cannot run on Node 10

const {
V4MAPPED,
ADDRCONFIG,
promises: {
Resolver: AsyncResolver
},
lookup
} = require('dns');

TypeError: Cannot destructure property Resolver of 'undefined' or 'null'.
at Object. (d:\parapara_oppo\server\LoginServer\node_modules\cacheable-lookup\source\index.js:9:5)
at Module._compile (internal/modules/cjs/loader.js:678:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:689:10)
at Module.load (internal/modules/cjs/loader.js:589:32)
at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
at Function.Module._load (internal/modules/cjs/loader.js:520:3)
at Module.require (internal/modules/cjs/loader.js:626:17)
at require (internal/modules/cjs/helpers.js:20:18)

What are the breaking changes for version 6.0.0

TS incompatibility between CacheableLookup.lookup and @types/node LookupFunction

Hi, I am getting the following type incompatibility:

With the code:

http.request({ lookup: (new CacheableLookup()).lookup });

I get the error:

Type '{ (hostname: string, family: IPFamily, callback: (error: ErrnoException | null, address: string, family: IPFamily) => void): void; (hostname: string, callback: (error: ErrnoException | null, address: string, family: IPFamily) => void): void; (hostname: string, options: LookupOptions & { ...; }, callback: (error: Err...' is not assignable to type 'LookupFunction'.
  Types of parameters 'family' and 'options' are incompatible.
    Type 'LookupOneOptions' is not assignable to type 'IPFamily'.
      Type 'LookupOneOptions' is not assignable to type '6'.ts(2322)

I have:

@types/[email protected]
[email protected] 
[email protected]

Source isn't returned in Entry object

I'm on the latest version 7.0.0 and the lookup code is this but on the master branch the code has support for source as well.

Maybe you should publish another version

[propose] how about to add cache from dns.lookup

cacheable-lookup/source/index.js

Line 330 in 5960e1d

query = await this._lookup(hostname);

this propose comes from my own experience when i'm using got client with cacheable-lookup cache in my node.js application:

I found dns.resolve* always failed if i set the request host to k8s-service name (eg: my-service), i know the reason is calling c-ares in nodejs on the network and it seems about kube-dns not support this type of host, it will always return "NXDomain".

afterwards, cacheable-lookup use dns.lookup by getaddrinfo(3) in system (could search suffix from /etc/resolv.conf) and record it to _hostnamesToFallback(has 1h timeout), then my request will always use dns.lookup each time. so the dns-cache feature is practically deprecated for my application unless the host is the whole k8s-service-name (like: my-service.svc.cluster.local).

Back to the propose, could we cache those dns from dns.lookup and add timeout to options? i think it is useful for node.js application in k8s-cluster and avoid some dns issues (like: lookup timeout) in high concurrency scenarios.

round-robin dns support

It seems like cacheable-lookup doesn't support round-robin DNS. Is there any options to support it?

http2 & cacheable-lookup

It seems that cacheable.install() should probably work with the http2 agent from https://github.com/szmarczak/http2-wrapper.

Currently it emits the TS error.

Type 'Agent' is missing the following properties from type 'Agent': maxFreeSockets, maxSockets, maxTotalSockets, freeSockets, and 2 more.

I think it should work if the typings to install() were a bit more relaxed as to what they consider to be an agent and leave verifyAgent to decide if install is possible.

Very slow when TTL caching is disabled

It's only about 25-250 op/s (compared to millions of op/s when using TTL or to 20-30k when using dns.lookup(...)).

I think the reason is that the DNS server does throttling on purpose.

Add `cacheableLookup.tick()` to remove outdated entries

Also this should be applied to agent.createConnection without any big performance impacts.

dnsCache.lookup types might be wrong?

I'm saying 'might' because I'm a typescript n00b.

But from what I understand, the callback of the native dns.lookup, which is supposed to be what this library's types seem to be based on, always has a callback signature of (err, address, family).

Therefore, when you util.promisify it, the result will always be the address (the string).

However, that is not the case for this library's lookup (https://github.com/szmarczak/cacheable-lookup/blob/master/index.d.ts#L109), causing inconsistencies in callback handling.

Furthermore, because of the inconsistencies in the callback signatures in this library, when I util.promisify the dnsCache.lookup, it thinks the "promisified lookup async" can return a void.

Now, you may think "well, why not just use lookupAsync directly anyway?", and that would be an excellent choice if I were directly consuming CacheableLookup; however, as a got plugin author, I have to deal with the fact that got's dnsLookup option is typed as CacheableLookup['lookup'], which, as we've learned, may be incorrect.

So the hope is that the wrong types are fixed at the root of the problem (and hopefully it'll "bubble up" to got).

Thank you.

Consider bumping `keyv`.

To keyv@4 so got can dedupe the version it uses.

TS error when using QuickLRU from readme example

export const cacheable = new CacheableLookup({
  cache: new QuickLRU({ maxSize: 1000 }),
});

Type 'QuickLRU<string, EntryObject[] | Promise<EntryObject[] | undefined>>' is not assignable to type 'CacheInstance'.
  Types of property 'set' are incompatible.
    Type '(key: string, value: EntryObject[] | Promise<EntryObject[] | undefined>, options?: { maxAge?: number | undefined; } | undefined) => QuickLRU<string, EntryObject[] | Promise<...>>' is not assignable to type '(hostname: string, entries: EntryObject[], ttl: number) => TPromise<boolean | void | CacheInstance>'.
      Types of parameters 'options' and 'ttl' are incompatible.
        Type 'number' is not assignable to type '{ maxAge?: number | undefined; } | undefined'.ts(2322)

Question: How is DNS resolved?

Sorry if this seems like a basic question, but how does cacheable-lookup resolve DNS? Looking at the code it seems to be using dns.resolve with dns.lookup as a fallback. Am I correct in this assumption?

Override DNS entries

Ability to set minTtl?

As best as I can tell, the app code uses the minimum of the TTL value on the DNS record or the maxTtl to decide how long records should be cached.

Is there any way to set a minimum TTL value?

One of the hosts that we look up has a TTL set to only 10 seconds, but we want to ensure that values are cached for longer than that.

Throws `setTimeout(...).unref is not a function` in Electron

Run the following code in an Electron renderer process (use https://www.electronjs.org/fiddle for faster repro):

const http = require('http');
const CacheableLookup = require('cacheable-lookup');
const cacheable = new CacheableLookup();
http.get('http://example.com', {lookup: cacheable.lookup});

It'll throw:

Uncaught TypeError: setTimeout(...).unref is not a function
    at CacheableLookup.tick (/private/var/folders/qp/tmp/T/tmp-/node_modules/cacheable-lookup/source/index.js:290)
    at CacheableLookup.query (/private/var/folders/qp/tmp/T/tmp-/node_modules/cacheable-lookup/source/index.js:176)
    at CacheableLookup.lookupAsync (/private/var/folders/qp/tmp/T/tmp-/node_modules/cacheable-lookup/source/index.js:137)
    at CacheableLookup.lookup (/private/var/folders/qp/tmp/T/tmp-/node_modules/cacheable-lookup/source/index.js:121)
    at net.js:1030
    at defaultTriggerAsyncIdScope (internal/async_hooks.js:301)
    at lookupAndConnect (net.js:1029)
    at Socket.connect (net.js:963)
    at Agent.connect [as createConnection] (net.js:172)
    at Agent.createSocket (_http_agent.js:234)

Tried Electron 8.2.5 and 9.0.0-beta.22

Cross posted: sindresorhus/got#1219

Addressed in electron/electron#21162

Possible fix would be to conditionally call unref() if it exists. Or use const { setTimeout } = require('timers')

Fallback can incorrectly fail to resolve with some /etc/hosts configurations

Repro:

Make sure you have localhost defined as 127.0.0.1 in your /etc/hosts, but no IPv6 ::1 definition.

Run:

const dns = require('dns')
const CacheableLookup = require("cacheable-lookup");

dns.lookup('localhost', { family: 6 }, (err, result) => console.log('dns', err, result));

const cacheable = new CacheableLookup();
cacheable.servers = ['1.1.1.1']; // Or any other remote DNS server
cacheable.lookup('localhost', { family: 6 }, (err, result) => console.log('cacheableLookup', err, result));

dns.lookup successfully resolves, cacheable lookup returns ENOTFOUND

Having only IPv4 localhost defined but no IPv6 might seem contrived, but it's actually the default in many places, including all Ubuntu releases (see https://gist.github.com/ghoneycutt/e531984406b4b86ace687ea8958a6dc3). localhost can still be resolved to ::1 for IPv6 on these machines, it's just that it's handled elsewhere (no idea why).

This means that on most Ubuntu machines (and probably others) http.get('localhost', { family: 6, lookup: cacheableLookup.lookup }) with a DNS server configured will always incorrectly fail to resolve.

This happens because:

The family is set to 6
There's a server defined, so cacheable lookup sends a request there first
That returns ENOTFOUND, so cacheable lookup tries to use dns.lookup and cache the result
Cacheable-lookup doesn't pass the family to that call and instead always passes { all: true }, presumably to maximize the fallback caching.
dns.lookup({ all: true }) actually returns only the IPv4 address from /etc/hosts, with no IPv6 value, even though dns.lookup({ family: 6 }) can successfully get the IPv6 result. Presumably this is because it stops once it finds an answer in /etc/hosts?

That last step might seem like bad Node.js behaviour, but it's also true in Python apparently so I expect it's something that comes from Linux's DNS resolution itself.

I think this will affect any lookup where only an IPv4 or IPv6 address is available in the hosts file - requests for the other family will simply fail unexpectedly. Haven't tested other OS, no idea if this can happen elsewhere too.

fallbackDuration behaviour doesn't match docs

fallbackDuration is documented as:

When the DNS server responds with ENOTFOUND or ENODATA and the OS reports that the entry is available, it will use dns.lookup(...) directly for the requested hostnames for the specified amount of time (in seconds).

In reality, it's not a cache time, it's a periodic reset frequency:

cacheable-lookup/source/index.js

Lines 97 to 106 in da10b58

 this._hostnamesToFallback = new Set(); 

 if (fallbackDuration < 1) { 

 this._fallback = false; 

 } else { 

 this._fallback = true; 

 const interval = setInterval(() => { 

 this._hostnamesToFallback.clear(); 

 }, fallbackDuration * 1000);

For example, if it's set to 1 hour, and then 55 minutes after creating the CacheableLookup instance a hostname lookup fails, the fallback usage will only be cached for 5 minutes (the time until the next reset).

This isn't a big problem for me particularly right now, just something I noticed.

dns lookup is not cached the way I understand

Hello there,

I could be missing something here.

I am trying to reduce the calls that go to dns.lookup in order to reduce the C++ libuv threads allocation so I am using this library.

How I am testing that, by running this code:

const nodeDnsLookup = dns.lookup;
let dnsLookupCounter = 0;
dns.lookup = (...args) => {
  console.log(`I am calling the native node dns lookup ${JSON.stringify(args)}, number ${dnsLookupCounter}`);
  dnsLookupCounter++;
  return nodeDnsLookup(...args);
};

so, without installing this library I can see a lot of dns lookups.

Now after I installed this library with its global http(s) agent as follows:

import CacheableLookup from 'cacheable-lookup';
const cacheable = new CacheableLookup();

cacheable.install(http.globalAgent);
cacheable.install(https.globalAgent);

I was expecting the number of dns lookups would go down and cache hits to happen, but it did not!
I see from the logs that the same hostname with the same family are still calling the native dns.lookup and not served from a cache.

Prefetch before expiration

_tick seems to do the cache clean up based on the ttl on first set.

After deletion, the dns query doesn't get refreshed until the next query invocation. This means that http clients will still occasionally pay the cost of a dns lookup on their hot path. For comparison, coredns does this.

Would it make sense for this library to support a refresh mechanism prior to query expiration?

DNS lookup are not cached

@szmarczak , we are using package - https://www.npmjs.com/package/cacheable-lookup and https://www.npmjs.com/package/@szmarczak/http-timer packages.

Now, when we are hitting domain for the first time , lookup event is getting triggered and lookupListener function is getting called.
when we are hitting same domain the next time, caching should take place and 'dns' should get reduced acording to calculation , i.e, -> dns - timings.lookup - timings.socket , but our dns timing is not getting reduced, also, this time our 'lookup' event is not getting fired and lookupListener function is not getting called.

So according to the source code -

timings.lookup = timings.connect;
timings.phases.dns = timings.lookup - timings.socket!;

so we are getting high dns time which shouldn't be the case.

Please suggest how we can use both the packages such that we can observe lesser dns time when entry is picking from cache.

Awaiting for a positive response.

Allow fallbackDuration = 0 without disabling fallback

I want to allow fallback, but never cache any addresses as always using the fallback address.

In my case, that would be useful because I'm using a custom local DNS server that's doing some funky logic (resolving against the currently running set of docker containers) and the addresses that this server can resolve can change very frequently. I want to ensure every single lookup checks my custom DNS server doing running a normal resolution.

Is there a specific reason that this is currently not allowed? For now I'm using 1, which is obviously pretty close and mostly works for my case, but it would be nice if I could disable that caching entirely whilst still using the fallback functionality.

Jest reporting CachableLookup is preventing shutdown

Seems like #19 might still be occurring:

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  DNSCHANNEL

      at new CacheableLookup (../node_modules/cacheable-lookup/source/index.js:91:14)
      at new Request (../node_modules/amply.js/lib/request.js:64:21)
      at new Client (../node_modules/amply.js/lib/client.js:9:20)
      at Object.<anonymous> (../node_modules/amply.js/index.js:4:18)

I'm not sure if the bug is on the ampliy.js side though.

Enable tests again

I couldn't figure out why the tests were just timing out without any error message.

cacheable-lookup/package.json

Lines 18 to 19 in 649ebeb

 "//test": "xo && nyc --reporter=lcovonly --reporter=text ava && tsd", 

 "test": "tsd"

Error while working with got + electron.

**
If I remove this module from got. it shows no error. but taking too long to make requests.
**

What is the root cause?

I am using node LTS 14 LATEST & TRIED 16.10. SAME ERROR.

Cannot test the typings

See tsdjs/tsd#57

This can be solved via converting this project to TypeScript. Feel free to pick this up.

The process never exits

Just create a file with new (require('cacheable-lookup')) and run it.

Make `.globalAgent` use `cacheable-lookup`

Hello,

I love this library and I'm waiting for the next got version for starting using it, thanks for creating it 🙂

However, although most of my code is using got, still I feel I'm losing the opportunity for caching some requests that are using other libraries.

so I thought, how risky could be monkey patch http/https to be sure a cacheable lookup is being used?

const http = require('http');
const shimmer = require('shimmer');
const CacheableLookup = require('cacheable-lookup');
const cacheable = new CacheableLookup();

shimmer.wrap(http, 'get', function (originalGet) {

  // a good opportunity to setup good DNS!
  dns.setServers([
    '1.1.1.1',
    '1.0.0.1',
    '2606:4700:4700::1111',
    '2606:4700:4700::1001'
  ]);

  // override `http.get` for always use `cacheable.lookup` by default
  return function (url, opts, cb) {
	return originalGel(url, {lookup: cacheable.lookup ...opts}, cb)
  };
});

How viable do you think could be this? 👨‍🔬

	this._hostnamesToFallback = new Set();

	if (fallbackDuration < 1) {
	this._fallback = false;
	} else {
	this._fallback = true;

	const interval = setInterval(() => {
	this._hostnamesToFallback.clear();
	}, fallbackDuration * 1000);

	"//test": "xo && nyc --reporter=lcovonly --reporter=text ava && tsd",
	"test": "tsd"

szmarczak / cacheable-lookup Goto Github PK

cacheable-lookup's Introduction

cacheable-lookup's People

Contributors

Stargazers

Watchers

Forkers

cacheable-lookup's Issues

Recommend Projects

Recommend Topics

Recommend Org