Skip to content

DNS lookup failure dns.js:82 #4545

Description

@allthetime
TypeError: Cannot read property 'indexOf' of undefined    
 at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:82:51)

here's the relevant line in node.js lib (https://github.com/nodejs/node/blob/master/lib/dns.js#L82)

I have a bot running that polls an sftp server for the existence of a file that is uploaded daily. It connects to socketio so that it can make posts to our main app.

It has suddenly not been working properly for the last 3 days. We have made no changes to the relevant code for a couple of weeks and the stack trace is incredibly unhelpful as it doesn't show what initiated it.

This seems to be happening before the request to the sftp server is made because I would be getting more feedback/logs. The bot successfully connects to our socket server as well, so I'm at a loss for where DNS is even being hit.

connect to socket io --> poll sftp server --> grab file when ready

is all that is happening, so it seems something is happening in the nodejs blackbox between step one and two...

any ideas?

Activity

added
dnsIssues and PRs related to the dns subsystem.
on Jan 6, 2016

saghul commented on Jan 6, 2016

@saghul
Member

Looks like getaddrinfo returned an empty list of results somehow? I thought that would fail with EAI_NODATA. Do you have a small reproducible test case?

silverwind commented on Jan 12, 2016

@silverwind
Contributor

@allthetime can you post the hostname of that sftp server?

evanlucas commented on Jan 14, 2016

@evanlucas
Contributor

@saghul it seems like https://github.com/nodejs/node/blob/master/src/cares_wrap.cc#L947 might be the cause of this. uv_inet_ntop could be returning UV_ENOSPC. So in that case, res exists, but the array is an empty array when we switch back to js. So perhaps we should add a check for an empty array and if it exists, callback with an EAI_NONAME err code?

saghul commented on Jan 15, 2016

@saghul
Member

@evanlucas it could be, yes. I'd say there is nothing to fix here other than documentation. It's possible the list comes back empty, so one should look out for that.

evanlucas commented on Jan 15, 2016

@evanlucas
Contributor

Well we are assuming that the array has at least one element

saghul commented on Jan 15, 2016

@saghul
Member

Bad assumption, I guess 😄

a1arias commented on Jan 16, 2016

@a1arias

This seems to be the same as nodejs/node-v0.x-archive#8698

I too see this when hitting EMFILE errors, same on (all?) versions of node (testing on 4.2.3 and 5.4.0). Interestingly, when running with a version of node compiled with --debug it doesn't happen. Why is that?

saghul commented on Jan 16, 2016

@saghul
Member

If you are seeing EMFILE errors you should raise the file descriptor limits
with uname -n.
On Jan 16, 2016 03:29, "Adrian Arias" notifications@github.com wrote:

This seems to be the same as nodejs/node-v0.x-archive#8698
nodejs/node-v0.x-archive#8698

I too see this when hitting EMFILE errors, same on (all?) versions of node
(testing on 4.2.3 and 5.4.0). Interestingly, when running with a version of
node compiled with --debug it doesn't happen. Why is that?

—
Reply to this email directly or view it on GitHub
#4545 (comment).

a1arias commented on Jan 19, 2016

@a1arias

@saghul that is a good for dev only work-around, not a solution, and will only differ the problem. What do you do when the security limits have already been increased to the max the machine can physically handle?

In my case, a more robust solution would be to refactor the app so that a) we use connection pooling and b) we don't keep the connections open while we process the request. However, that is a architecture flaw and besides the point: this is a confirmed bug and EMFILE helps reproduce it reliably.

A node process should not crash, as described in this issue, when EMFILE gets hit.

To easily reproduce this error, these conditions need to exists.

  1. An external domain name must be used. That is, a domain that is not resolvable locally and for which the resolver must send an actual DNS query. This is so the flow control routes to the correct code path in the core dns.js module where the issue exists.

  2. EMFILE error must be hit. To simulate an EMFILE error, you can do the following:

    • set the max open file limit really low
    ulimit -n 1024
    
    • setup a test http server route which keeps the connections open by not sending a reply.

With these conditions met, run the app and wait for the error.

Strangely, this does not happen when running with the debug version of node, node_g, compiled with the --debug and --gdb option. Maybe this is because the debug version introduces some delay that mitigates an underlying race condition.

saghul commented on Jan 20, 2016

@saghul
Member

We try hard to make EMFILE errors propagate as far up as possible (IIRC), so Node doesn't crash and your application handles those.

AFAIS the patch @cjihrig made does not alter that behavior, because it only applies if status == 0, that is, if getaddrinfo succeeded.

set the max open file limit really low

ulimit -n 1024

It defaults to 256 on OSX, BTW.

cjihrig commented on Jan 20, 2016

@cjihrig
Contributor

AFAIS the patch @cjihrig made does not alter that behavior, because it only applies if status == 0, that is, if getaddrinfo succeeded.

Yes, the crash in JavaScript was the result of getaddrinfo() succeeding, but passing back an empty array. In JS, we were assuming that the array had at least one element. My patch only ensured that that situation didn't happen.

stanislav-kozyrev commented on Nov 4, 2016

@stanislav-kozyrev

Hi.

TL;DR Any chance the fix gets back-ported to 4.x?

I am on Ubuntu 16.04.1 LTS which is stuck with 4.2.6 at the moment. npm install worked like a charm until recently. I read that NodeJS is using libuv for DNS resolution so it probably has something to do with my network configuration (which is strange since I haven't changed/rebooted anything in months). Either way, I tried the fix on Ubuntu's version of NodeJS and npm is working again. Should I talk to Ubuntu/Debian developers about that? Thank you in advance.

bnoordhuis commented on Nov 4, 2016

@bnoordhuis
Member

@stakoz It was released in v4.4.1, commit 7c3b844. Getting in touch with the maintainer of the distro package is your best course of action, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dnsIssues and PRs related to the dns subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions