Skip to content

Commit

Permalink
Fix compatibility with Node.js 17 in test/test.js
Browse files Browse the repository at this point in the history
Node.js 17, which as of writing is the most recent version, contains a
breaking change in its DNS resolver, causing Firefox not to start
anymore in our test framework. The inline comment together with the
following resources provide more background:

- nodejs/node#40702
- nodejs/node#39987
- cyrus-and/chrome-remote-interface#467
- https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V17.md#other-notable-changes
- DeviceFarmer/adbkit#209
- https://nodejs.org/api/dns.html#dnssetdefaultresultorderorder

This commit ensures that versions both older and newer than Node.js 17
work as expected. This is mainly necessary since the bots as of writing
run Node.js 14.17.0 which is from before this API got introduced and for
example Node.js 12 LTS is only end-of-life in April 2022, so we have to
keep support for those older versions unfortunately.
  • Loading branch information
timvandermeij committed Nov 28, 2021
1 parent 5309133 commit 60ed3cd
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,24 @@ var os = require("os");
var puppeteer = require("puppeteer");
var url = require("url");
var testUtils = require("./testutils.js");
const dns = require("dns");
const yargs = require("yargs");

// Chrome uses host `127.0.0.1` in the browser's websocket endpoint URL while
// Firefox uses `localhost`, which before Node.js 17 also resolved to the IPv4
// address `127.0.0.1` by Node.js' DNS resolver. However, this behavior changed
// in Node.js 17 where the default is to prefer an IPv6 address if one is
// offered (which varies based on the OS and/or how the `localhost` hostname
// resolution is configured), so it can now also resolve to `::1`. This causes
// Firefox to not start anymore since it doesn't bind on the `::1` interface.
// To avoid this, we switch Node.js' DNS resolver back to preferring IPv4
// since we connect to a local browser anyway. Only do this for Node.js versions
// that actually have this API since it got introduced in Node.js 14.18.0 and
// it's not relevant for older versions anyway.
if (dns.setDefaultResultOrder !== undefined) {
dns.setDefaultResultOrder("ipv4first");
}

function parseOptions() {
yargs
.usage("Usage: $0")
Expand Down

0 comments on commit 60ed3cd

Please sign in to comment.