Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Default to IPv4 localhost for Node 17 compatibility #214

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

pimterry
Copy link

Fix #209. Without this adbkit is not usable with Node 17 (or any other future Node versions, presumably).

When testing locally, on my machine with Node 17.0.1 the tests today fail with:

  1) Sync
       "before all" hook in "Sync":
     Error: connect ECONNREFUSED ::1:5037
      at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1161:16)

This happens because Node 17 defaults to IPv6 and ADB's local server never listens for IPv6 connections, as far as I can tell.

This does change the default, which could affect anybody where localhost doesn't resolve to 127.0.0.1, in theory, but in practice I don't think this will cause issues - anybody who resolves localhost to a different IP address has a very unusual configuration and likely has other networking issues already.

Interestingly, I had to add the host option from scratch for this PR, even though it's already documented as existing. I guess it got lost in the TypeScript rewrite? Pretty clear from the source that it didn't work until now.

Signed-off-by: Tim Perry <pimterry@gmail.com>
@koral--
Copy link
Member

koral-- commented Dec 13, 2021

LGTM

I guess it got lost in the TypeScript rewrite?

I'm not a TS/coffee expert but it seems that it wasn't present as well at the time of transition to TS: 3bd94ba#diff-066f213e1d28e2d82eb24abc58dcd804134270d3dabfe5096cd1953cee17903bL59

@koral-- koral-- merged commit 0c1c070 into DeviceFarmer:master Dec 13, 2021
@pimterry pimterry deleted the ipv4-default branch December 14, 2021 12:27
drauggres pushed a commit to drauggres/adbkit that referenced this pull request Mar 25, 2022
Default to IPv4 localhost for Node 17 compatibility

refs DeviceFarmer#214

Signed-off-by: Tim Perry <pimterry@gmail.com>
(cherry picked from commit 0c1c070)
Signed-off-by: Sergey Volkov <s.volkov@netris.ru>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

connection issue with nodejs 17 on system where localhost will resolve by default to an ipv6 address
2 participants