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

Update to new cares_wrap.getaddrinfo() interface. #200

Merged
merged 1 commit into from
Nov 13, 2017
Merged

Update to new cares_wrap.getaddrinfo() interface. #200

merged 1 commit into from
Nov 13, 2017

Conversation

bnoordhuis
Copy link
Contributor

As of node.js v8.6.0, the method takes a fourth argument that is a
boolean. false is as of this time the default value.

This is a quick fix. node_mdns really should not be depending on
node.js internals like this because it inevitably breaks.

Fixes: #199
Refs: nodejs/node#14731

I don't know what node.js versions node_mdns ostensibly supports (I saw a node-waf script in there!) but I'd rip out this code altogether and use the public API.

Copy link

@superhawk610 superhawk610 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you check commit nodejs/node@e007f66, you will see that this function now actually requires 5 arguments, so the correct syntax would be cares.getaddrinfo(req, host, family, 0, false).

As of node.js v8.6.0, the method takes a fifth argument that is a
boolean.  `false` is as of this time the default value.

This is a quick fix.  node_mdns really should not be depending on
node.js internals like this because it inevitably breaks.

Fixes: #199
Refs: nodejs/node#14731
@Nastras
Copy link

Nastras commented Oct 13, 2017

Hello, I have made an update to node on 8.7.0 I get the following error message. The same message I have already received with 8.6.0 and 8.5.0.

The last working version is 8.4.0 without this error.

I have had hope that it was fixed in the 8.7.0?

Okt 13 20:21:42 Server homebridge[27375]: /usr/local/bin/node[27375]: ../src/cares_wrap.cc:1944:void node::cares_wrap::{anonymous}::GetAddrInfo(const v8::FunctionCallbackInfov8::Value&): Assertion `args[4]->IsBoolean()' failed.
Okt 13 20:21:42 Server systemd[1]: homebridge.service: Main process exited, code=killed, status=6/ABRT
Okt 13 20:21:42 Server systemd[1]: homebridge.service: Unit entered failed state.
Okt 13 20:21:42 Server systemd[1]: homebridge.service: Failed with result 'signal'.
Okt 13 20:21:52 Server systemd[1]: homebridge.service: Service hold-off time over, scheduling restart.
Okt 13 20:21:52 Server systemd[1]: Stopped Node.js HomeKit Server.
Okt 13 20:21:52 Server systemd[1]: Started Node.js HomeKit Server.
Okt 13 20:21

@superhawk610
Copy link

superhawk610 commented Oct 13, 2017

@Nastra888 Node 8.5+ just changed how this call functions. Node was never broken, node_mdns is. Either use the change from this PR or downgrade to Node 8.4 or lower until this PR is committed.

@Nastras
Copy link

Nastras commented Oct 13, 2017

I do not quite understand what you mean with PR?

The error is not with node, if I understand you correctly with node mdns?

@superhawk610
Copy link

superhawk610 commented Oct 13, 2017

@Nastra888 in your project, open node_modules/mdns/lib/resolver_sequence_tasks.js and change line 117 to

, err = cares.getaddrinfo(req, host, family, 0, false)

@Nastras
Copy link

Nastras commented Oct 14, 2017

Thank You for your quick answer.

I have change line 117 but I get the same error message. Do you have a other idea ?

Okt 14 06:51:37 Server homebridge[1178]: /usr/local/bin/node[1178]: ../src/cares_wrap.cc:1944:void node::cares_wrap::{anonymous}::GetAddrInfo(const v8::FunctionCallbackInfov8::Value&): Assertion `args[4]->IsBoolean()' failed.

@agnat
Copy link
Owner

agnat commented Nov 13, 2017

Thanks for the patch, @bnoordhuis. I just published it as v2.3.4.

[...] but I'd rip out this code altogether and use the public API.

Absolutely. The linux getaddr stuff is such a sore point. Constantly (semi-) broken and just a mess.

Apologies to everyone. This totally slipped my mind.

@Nastras
Copy link

Nastras commented Nov 15, 2017

@agnat
Hello, I made the update to node 8.9.1 and your update from mdns to version 2.3.4.

I still get this error displayed, do you have an idea why?

img_0151

@agnat
Copy link
Owner

agnat commented Nov 15, 2017

Nope, no clue. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node 8.5 on raspberry breaks the current GetAddrInfo call
4 participants