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

DNS cache is not compatible with DNS suffixes #1175

Closed
2 tasks done
SkeLLLa opened this issue Apr 22, 2020 · 13 comments
Closed
2 tasks done

DNS cache is not compatible with DNS suffixes #1175

SkeLLLa opened this issue Apr 22, 2020 · 13 comments
Labels
enhancement This change will extend Got features

Comments

@SkeLLLa
Copy link

SkeLLLa commented Apr 22, 2020

Describe the bug

  • Node.js version: 12, 14
  • OS & version: linux glibc, alpine
  • got v11 (v10 works fine)

Actual behavior

Got doesn't resolve short dns names and fails with error.
...

Expected behavior

Got resolve short dns names.
...

Code to reproduce

await got(`http://short-name-host`)

With v11 it produce

(node:53) UnhandledPromiseRejectionWarning: RequestError: ENOTFOUND short-name-host
    at ClientRequest.<anonymous> (/opt/service/node_modules/got/dist/source/core/index.js:759:25)
    at Object.onceWrapper (events.js:422:26)
    at ClientRequest.emit (events.js:327:22)
    at ClientRequest.origin.emit (/opt/service/node_modules/@szmarczak/http-timer/dist/source/index.js:39:20)
    at Socket.socketErrorListener (_http_client.js:461:9)
    at Socket.emit (events.js:315:20)
    at emitErrorNT (internal/streams/destroy.js:96:8)
    at emitErrorCloseNT (internal/streams/destroy.js:68:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)
    at CacheableLookup.lookupAsync (/opt/service/node_modules/cacheable-lookup/source/index.js:167:19)

It's easily reproduced also with some kubernetes dns names as well. E.g. http://short-name-host.default and so on.
And it starts working only if you provide full name like short-name-host.default.svc.cluster.local.

In other words v11 doesn't work well with anything that's written in search section of resolv.conf
Might be an issue with https://github.com/szmarczak/cacheable-lookup#readme, so @szmarczak maybe you can figure out.

Checklist

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@szmarczak
Copy link
Collaborator

I don't have kubernetes so I cannot reproduce...

@szmarczak
Copy link
Collaborator

But I think I can reproduce in another way, will check tomorrow.

@szmarczak szmarczak added the unconfirmed bug The issue is possibly a bug - needs more info label Apr 22, 2020
@adityapatadia
Copy link

I can confirm that this issue on Kubernetes.

@szmarczak
Copy link
Collaborator

Short host name resolving works as expected. It's just that it doesn't communicate with resolv.conf.

@szmarczak szmarczak changed the title Short host DNS resolution broken in 11 DNS cache is not compatible with resolv.conf Apr 23, 2020
@adityapatadia
Copy link

adityapatadia commented Apr 23, 2020

Okay, will it be fixed? Right now I have worked around by disabling DNS cache.

@adityapatadia
Copy link

Actually on Kubernetes, the CoreDNS service has its own caching mechanism so we are not seeing much difference between cache enable and disable.

@szmarczak
Copy link
Collaborator

I can reproduce on Windows

@szmarczak szmarczak changed the title DNS cache is not compatible with resolv.conf DNS cache is not compatible with DNS suffixes Apr 23, 2020
@szmarczak szmarczak added enhancement This change will extend Got features and removed unconfirmed bug The issue is possibly a bug - needs more info labels Apr 23, 2020
@szmarczak
Copy link
Collaborator

The only possible way is to fallback to dns.lookup(...), which for sure will have a negative impact on performance.

@SkeLLLa
Copy link
Author

SkeLLLa commented Apr 23, 2020

@szmarczak I think some flag or option that will switch "preformance mode" and regular dns lookup should be good enough. Or try to fallback to dns.lookup in case of ENOTFOUND error.

@szmarczak
Copy link
Collaborator

The code is pretty much finished, I just need to write the tests.
szmarczak/cacheable-lookup#15

@szmarczak
Copy link
Collaborator

The only possible way is to fallback to dns.lookup(...), which for sure will have a negative impact on performance.

Well, by default it remembers the errors for 150ms and the OS results for 1s, so there should be no impact at all.

@szmarczak
Copy link
Collaborator

Fixed in szmarczak/cacheable-lookup@dfefae2 but haven't made a release yet. I'll try to fix #1173 first.

@szmarczak
Copy link
Collaborator

Fixed in cacheable-lookup@4.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This change will extend Got features
Projects
None yet
Development

No branches or pull requests

3 participants