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

[feat] Add a cacheable-lookup example #1

Closed
Kikobeats opened this issue Feb 25, 2023 · 12 comments
Closed

[feat] Add a cacheable-lookup example #1

Kikobeats opened this issue Feb 25, 2023 · 12 comments
Labels
enhancement New feature or request

Comments

@Kikobeats
Copy link

Kikobeats commented Feb 25, 2023

The library is mentioned to use it with cacheable-lookup has been taking into account.

Since cacheable-lookup already accepts a cache, what should be a good example, using it with tangerine?

Maybe in this way?

const Keyv = require('@keyvhq/core')

const cache = new Keyv()

const cacheableLookup = new CacheableLookup({
  cache,
  resolver: new Tangerine({ 
    // explicitly disabled since `cacheable-lookup` already takes case
    cache: false 
  }) 
})
@titanism
Copy link
Contributor

v1.1.0 released with support for other libraries!

https://github.com/forwardemail/tangerine/releases/tag/v1.1.0

see updated README options at https://github.com/forwardemail/tangerine#options and example with got at https://github.com/forwardemail/tangerine/blob/1de724a704bd669e5b2e92d1d5dd5d5f4bc65204/test/test.js#L607-L614.

@titanism
Copy link
Contributor

v1.2.0 released which converts the package to use undici as an optional peer dependency (so there's one less dependency to install when you npm install tangerine, which is useful if you're bringing your own HTTP library such as got).

In v1.2.0 you need to pass your custom request library as new Tangerine(options, request), e.g. new Tangerine(options, got). See updated README docs for usage.

I would have done a major semver bump to v2.0.0 but since this project just came out yesterday I'm doing a minor.

@titanism
Copy link
Contributor

@Kikobeats to answer your question in this issue, yes, your example from above #1 (comment) is accurate! 🎉

The reason being is that tangerine.lookup uses internal calls to tangerine.resolve4 and tangerine.resolve6 using Promise.any (to match dns module spec). Further, if you inspect the internals of cacheable-lookup, you will see it checks for instanceof here https://github.com/szmarczak/cacheable-lookup/blob/9e60c9f6e74a003692aec68f3ddad93afe613b8f/source/index.mjs#L97-L104 - which will return true since Tangerine extends dns.promises.Resolver. Therefore the resolver will be bound properly.

@titanism
Copy link
Contributor

@Kikobeats we recommend you upgrade to v1.2.0 of tangerine if you already started to integrate it - as this is now a clean polished version with support for other HTTP libraries.

@titanism
Copy link
Contributor

@Kikobeats see PR szmarczak/cacheable-lookup@3e7889b at szmarczak/cacheable-lookup#77 for docs update there too

@Kikobeats
Copy link
Author

Kikobeats commented Feb 25, 2023

@titanism I think you can drop requestOptions, http libraries already provides a way to bind options, e.g.:

const got = require('got').extend({
  responseType: 'buffer', 
  decompress: false
})

const tangerine = new Tangerine({ 
  cache: false 
}, got)

BTW, why you explicitly disabled decompress there?

@titanism
Copy link
Contributor

I disabled it since I didn't see any popular DoH resolver (e.g. Cloudflare nor Google) using encoded responses.

@titanism
Copy link
Contributor

Also, I think that we should keep requestOptions because they're required defaults (e.g. the Accept/Content-Type headers) for talking to DoH servers from Google/Cloudflare - and the average user won't know to set those headers.

@titanism
Copy link
Contributor

@Kikobeats please upgrade to v1.2.2 - this should be a final stable tested released now!!! 🎉

https://github.com/forwardemail/tangerine/releases/tag/v1.2.2

@Kikobeats
Copy link
Author

In this example:

const tangerine = new Tangerine(
    {
      requestOptions: {
        responseType: 'buffer',
        decompress: false,
        retry: {
          limit: 0
        }
      }
    },
    got
  );

got also has a default timeout. what it should be there to play nice with tangerine?
why not pass { tries: 0, timeout: false } at tangerine and leverage that logic into the HTTP library?

@titanism
Copy link
Contributor

Got should have retries and timeout turned off to work with tangerine.

@titanism
Copy link
Contributor

According to Got docs, it says there's no default timeout at https://github.com/sindresorhus/got/blob/main/documentation/6-timeout.md.

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

2 participants