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

Firefox support broken (Node 17?) #467

Closed
Niek opened this issue Nov 18, 2021 · 6 comments
Closed

Firefox support broken (Node 17?) #467

Niek opened this issue Nov 18, 2021 · 6 comments
Labels

Comments

@Niek
Copy link

Niek commented Nov 18, 2021

Component Version
Operating system macOS
Node.js Node 17
Chrome/Chromium/... Firefox 95.0b8
chrome-remote-interface 0.31.0

Is Chrome running in a container? No

Since recently, CRI can no longer connect to Firefox. After starting firefox with firefox --remote-debugging-port 41000, i can run a list and version command, but inspect fails:

$ npx chrome-remote-interface -t 127.0.0.1 -p 41000 list
[
    {
        "description": "",
        "devtoolsFrontendUrl": null,
        "faviconUrl": "",
        "id": "9448b9b3-f3fd-4664-bff3-5af49a54d539",
        "type": "page",
        "url": "about:blank",
        "browsingContextId": 25,
        "webSocketDebuggerUrl": "ws://localhost:41000/devtools/page/9448b9b3-f3fd-4664-bff3-5af49a54d539"
    },
    {
        "description": "Main process target",
        "devtoolsFrontendUrl": "",
        "faviconUrl": "",
        "id": "259f3d39-36d4-42bc-99bd-1f6c19b6b1ee",
        "title": "Main process target",
        "type": "browser",
        "url": "",
        "webSocketDebuggerUrl": "ws://localhost:41000/devtools/browser/259f3d39-36d4-42bc-99bd-1f6c19b6b1ee"
    }
]
$ npx chrome-remote-interface -t 127.0.0.1 -p 41000 version
{
    "Browser": "Firefox/95.0",
    "Protocol-Version": "1.0",
    "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:94.0.1) Gecko/20100101 Firefox/94.0.1",
    "V8-Version": "1.0",
    "WebKit-Version": "1.0",
    "webSocketDebuggerUrl": "ws://localhost:41000/devtools/browser/259f3d39-36d4-42bc-99bd-1f6c19b6b1ee"
}
$ npx chrome-remote-interface -t 127.0.0.1 -p 41000 inspect
Cannot connect to remote endpoint: Error: connect ECONNREFUSED ::1:41000

I think the issue is that -t 127.0.0.1 is not working properly with the inspect command, it seems to try to connect to the IPv6 interface as ::1 which Firefox doesn't listen on.

@cyrus-and
Copy link
Owner

Alright so, this is not strictly related to Firefox, it's a consequence of a new change introduced in Node.js 17 (nodejs/node#39987). And can be reproduced as easily as:

const http = require('http');

const server = http.createServer((req, res) => {
    res.end('OK\n');
});

server.listen(4444, '127.0.0.1', () => {
    let url;
    url = 'http://127.0.0.1:4444'; // works
    url = 'http://localhost:4444'; // doesn't; it just tries ::1 (IPv6)
    http.get(url, (res) => {
        res.pipe(process.stdout);
        server.close();
    });
});

This behaviour kind of surprises me, not the fact that IPv6 is preferred over IPv4, which is commonplace nowadays, rather the fact that Node.js stops after the first (IPv6) attempt. AFAIK it's unique in this, for example in Python 3.9.8 the following code tries both IPv6 and IPv4 addresses (in that order):

import http.client
http.client.HTTPConnection('localhost:4444').request('GET', '/')

Having said that, the --host/-t switch in chrome-remote-interface only controls the HTTP connection to the target, whereas the WebSocket URLs are used as they are (you can always pass them manually though). And here's the difference between Firefox and Chrome: the former uses ws://localhost whereas the latter uses ws://127.0.0.1, when returning the target information.

Now for the solution, since the implementations never (?) listen on IPv6, I think we can safely instruct Node.js to prefer IPv4 with dns.setDefaultResultOrder('ipv4first'). Thoughts about this?

@cyrus-and cyrus-and added the bug label Nov 19, 2021
@Niek
Copy link
Author

Niek commented Nov 19, 2021

Preferring IPv4 sounds like a very reasonable default, I'm sure than 95%+ of the time users connect to a local browser, and as you said both Chrome and FF don't even bind on the ::1 interface.

An alternative approach could be to rewrite localhost in the websocket host to 127.0.0.1 - might be a bit more hacky but possibly safer.

@cyrus-and
Copy link
Owner

Former it is. It really escapes me why Node.js doesn't go on and try IPv4 after an ECONNREFUSED on IPv6. Do you have any idea? Has it always been this way (but the other way around)?

@Niek
Copy link
Author

Niek commented Nov 23, 2021

Thanks for the fix!

There's some background and more info here: nodejs/node#40702 (comment)

I still fail to see why the Node team thought it was a good idea to change the default, I'm sure this will break quite a few other packages.

timvandermeij added a commit to timvandermeij/pdf.js that referenced this issue Nov 28, 2021
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.
@treysis
Copy link

treysis commented Jan 6, 2022

An alternative approach could be to rewrite localhost in the websocket host to 127.0.0.1 - might be a bit more hacky but possibly safer.

The preferred solution would indeed be to rewrite it to 127.0.0.1 and not use localhost anymore due to its ambiguity. This will keep compatibility in those cases where people try to connect from outside, instead of changing the sorting order of the DNS result.

Former it is. It really escapes me why Node.js doesn't go on and try IPv4 after an ECONNREFUSED on IPv6. Do you have any idea? Has it always been this way (but the other way around)?

The solution you're referring to is called "Happy Eyeballs". This is also good solution when multi-homing. However, NodeJS never implemented happy eyeballs. Thus, yes, it has always been this way but the other way round. It's also the reason why resorting by default was abandoned.

I still fail to see why the Node team thought it was a good idea to change the default, I'm sure this will break quite a few other packages.

Because it was causing more and more problems with people on IPv6-enabled connections in other settings. So there was good reason to go back to respecting the DNS sorting order of the OS. v17 is also only an intermediate release. v14 and v16 (and soon v18) are the LTS releases. It's expected to encounter problems when upgrading to a new NodeJS version.

Kafva pushed a commit to Kafva/pdf.js that referenced this issue Jan 20, 2022
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.
bh213 pushed a commit to bh213/pdf.js that referenced this issue Jun 3, 2022
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.
@whimboo
Copy link

whimboo commented Jul 13, 2022

Having said that, the --host/-t switch in chrome-remote-interface only controls the HTTP connection to the target, whereas the WebSocket URLs are used as they are (you can always pass them manually though). And here's the difference between Firefox and Chrome: the former uses ws://localhost whereas the latter uses ws://127.0.0.1, when returning the target information.

Thanks for this hint! This is indeed problematic and we are going to fix this problem in Firefox via https://bugzilla.mozilla.org/show_bug.cgi?id=1769994. Reporting the actual resolved local IP via stderr or the DevToolsActivePort seems to be the right way for various affected clients.

rousek pushed a commit to signosoft/pdf.js that referenced this issue Aug 10, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants