BetterThanTomorrow / calva

Clojure & ClojureScript Interactive Programming for VS Code
https://marketplace.visualstudio.com/items?itemName=betterthantomorrow.calva
Other
1.68k stars 217 forks source link

calva/vscode wrongly assumes nrepl started at ::1 (ipv6) #2310

Open birdspider opened 1 year ago

birdspider commented 1 year ago

not sure if its my system/system-update but:

description

calva jack-in fails to connect (ECONNREFUSED).

I think because vscode chooses/returns localhost as ipv6 ::1 (maybe in addition to 127.0.0.1).

However, nrepl.cmdline seems not to be started with ipv6 bindings (see below)

calva repl log:

; Starting Jack-in Terminal: pushd [snip]; clojure -Sdeps '{:deps {nrepl/nrepl {:mvn/version,"1.0.0"},cider/cider-nrepl {:mvn/version,"0.37.1"}}}' -M -m nrepl.cmdline --middleware "[cider.nrepl/cider-middleware]" ; popd
; Using host:port localhost:42907 ...
; Hooking up nREPL sessions ...
; nREPL connection failed: Error: connect ECONNREFUSED ::1:42907
; Failed connecting.

I do not know what changed - that makes vscode/calva use ::1 - it's not required for me/my case.

manual setup works

see param -b, with 0.0.0.0 it should bind to any local address, hence connecting when starting like this, works:

clojure -Sdeps '{:deps {nrepl/nrepl {:mvn/version,"1.0.0"},cider/cider-nrepl {:mvn/version,"0.37.1"}}}' -M -m nrepl.cmdline -b "0.0.0.0" --middleware "[cider.nrepl/cider-middleware]" 
birdspider commented 1 year ago

workaround (force ipv4)

diff --git a/src/nrepl/index.ts b/src/nrepl/index.ts
index 8d685fea..86c253d3 100644
--- a/src/nrepl/index.ts
+++ b/src/nrepl/index.ts
@@ -130,7 +130,7 @@ export class NReplClient {
    */
   static create(opts: { host: string; port: number; onError: (e) => void }) {
     return new Promise<NReplClient>((resolve, reject) => {
-      const socket = net.createConnection(opts, () => {
+      const socket = net.createConnection({ ...opts, family: 4 }, () => {
         const nsId = client.nextId;
         const cloneId = client.nextId;
         const describeId = client.nextId;
PEZ commented 1 year ago

Maybe what changed is some default socket creation options? But how would those change... Very strange.

birdspider commented 1 year ago

Maybe what changed is some default socket creation options? But how would those change... Very strange.

since localhost means both ::1 and 127.0.0.1 and nrepl start-server doc states

:bind — bind address, by default \"127.0.0.1\"

(with the explicit default set here)

I think this is simply under-specified. If calva wants to use nrepl defaults - 127.0.0.1 (ip4) it is, otherwise a specific bind address is needed -b. But thats for the calva devs to decide, or upstream/nrepl to change.

PEZ commented 1 year ago

Do you mean that if we connect to the nrepl server on port 127.0.0.1, we would avoid the ambiguity?

A problem is that the nrepl server reports that it starts the nrepl on localhost, and this is what Calva uses to figure out what address to connect to. The nrepl code for this is here, and I can't quite follow: https://github.com/nrepl/nrepl/blob/5839006c5f522fd5e6cf2adbcc5b59e4bd0677dd/src/clojure/nrepl/cmdline.clj#L446

Maybe you should report this on nrepl, @birdspider? I can also do it, but you seem to understand the issue better than I do. Meanwhile I can make a translation localhost -> 127.0.0.1 in Calva when acting on the server started message. You think that makes sense?

PEZ commented 1 year ago

Or, maybe that would still be a problem for when we are connecting to some other hostname? Is your workaround, forcing ipv4 better? I'm leaning towards that here.

Please advice. 😄

birdspider commented 1 year ago

Hi,

@PEZ , sorry I didn't catch your replies, also: I'm not in a hurry with this issue.

the way I see it, the problem is that localhost normally means 1 and/or 2 things, that is 127.0.0.1 or ::1 or both.

Meanwhile I can make a translation localhost -> 127.0.0.1 in Calva when acting on the server started message. You think that makes sense?

I think this would make sense currently, because nrepl (as started by calva, with default args) only ever binds to 127.0.0.1.

I also think that the server-started message is part of the problem (see below).

problem is that the nrepl server reports

well, it might report anything, but with no extra args it hard-binds to 127.0.0.1 (not ::1)


failure scenario:

1) calva starts a nrepl (with no/default args) 2) an nrepl is started on "localhost:42907" (actually bound to 127.0.0.1:42907) 3) then calva reads (server-started-message), presumably parses "localhost". (*) 4) calva asks vscode/node/OS: please resolve localhost for me: (getent hosts localhost) --> ::1. (I seems this is per design/default behaviour, ip6 is preferred for localhost when exists) 5) now calva/vscode tries to connect to ::1:42907 - but nothing was started on ::1

(*) I didn't check the msg


I only see 3 solutions:

since in this case, calva starts the nrepl - I think the 2nd solution is the appropriate one


PS: another workaround:

  "customJackInCommandLine": "clojure -Sdeps '{:deps {nrepl/nrepl {:mvn/version,\"1.0.0\"},cider/cider-nrepl {:mvn/version,\"0.28.5\"}}}' -M -m nrepl.cmdline -b ::1 --middleware \"[cider.nrepl/cider-middleware]\"",
PEZ commented 1 year ago

Thanks!

With these commits, Calva would consistently use 127.0.0.1 instead of localhost, even while the nrepl server reports localhost: https://github.com/BetterThanTomorrow/calva/compare/73f172f2093501e326179504a896f1d14175a4d5...2310-ipv6-ambiguity

However, if the nrepl server is started on some host, the connect string could be my-app-being-developed:12345 and currently Calva doesn't do any DNS resolve or anything like that. So I'm thinking that might fail in the same way as localhost does? Or does the link you now provided tell me that this is specific to localhost?

Your first workaround above https://github.com/BetterThanTomorrow/calva/issues/2310#issuecomment-1714085569 seems to dodge all this and just force ipv4, which looks somewhat like what we want?

birdspider commented 1 year ago

With these commits ...

I think this is appropriate default behavior since it guarantees compatibility with the auto-started nrepl. A custom connection is custom anyway - with user provided host.

However, if the nrepl server is started on some host, the connect string could be my-app-being-developed:12345 and currently Calva doesn't do any DNS resolve or anything like that. So I'm thinking that might fail in the same way as localhost does?

true, but to my knowledge, remote apps, that support (as in resolved DNS) both ipv4 and ipv6 are usually listening on both (see i.e. apache config)

in such a case

a) it's either remote, probably resolved by dns (by the OS) (which practice could also return N addrs, some ip4, some ip6) b) it's local, so someone (the developer) put it in his /etc/hosts hopefully only 1 time (or bound the nrepl to both addresses - if thats something nrepl can do)

Or does the link you now provided tell me that this is specific to localhost?

it (getent, dns resolve) seems to always prefer ipv6 - if enabled/configured

getent hosts google.com
2a00:1450:4001:829::200e google.com

Your first workaround above #2310 (comment) seems to dodge all this and just force ipv4, which looks somewhat like what we want?

I think this would break all (incl. remote) ipv6-only host connections


Hm, thinking about the problem a bit more, maybe nrepl should resolve "localhost" and use that address for binding.

birdspider commented 1 year ago

maybe nrepl should resolve "localhost" and use that address for binding.

maybe they do that, but "their" jvm returns ipv4:

it can depend on how java is started (-Djava.net.preferIPv6Addresses=true)


~ $ clojure -J-Djava.net.preferIPv6Addresses=true -A:rebel
nREPL server started on port 36905 on host localhost - nrepl://localhost:36905
user=> (java.net.InetSocketAddress. "" 3333)
#object[java.net.InetSocketAddress 0x13390a96 "localhost/[0:0:0:0:0:0:0:1]:3333"]
user=> (java.net.InetSocketAddress. "localhost" 3333)
#object[java.net.InetSocketAddress 0x5ee581db "localhost/[0:0:0:0:0:0:0:1]:3333"]
birdspider commented 1 year ago

if I do what calva does, in pure node-js (with an open nrepl)

const net = await import("net");
const b = net.createConnection({host:'localhost',port:36227});

there is no error, even though calva can't connect to the repl.

if I use a nonexisting repl port, for illustration, node/net even errors on both IPs:

[errors]: [
    Error: connect ECONNREFUSED ::1:36220
        at createConnectionError (node:net:1634:14)
        at afterConnectMultiple (node:net:1664:40)
        at TCPConnectWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
      errno: -111,
      code: 'ECONNREFUSED',
      syscall: 'connect',
      address: '::1',
      port: 36220
    },
    Error: connect ECONNREFUSED 127.0.0.1:36220
        at createConnectionError (node:net:1634:14)
        at afterConnectMultiple (node:net:1664:40)
        at TCPConnectWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
      errno: -111,
      code: 'ECONNREFUSED',
      syscall: 'connect',
      address: '127.0.0.1',
      port: 36220
    }
  ]

EDIT: native nodejs cmd nodejs=20.6.1, vscode nodejs=18.15.0

birdspider commented 1 year ago

I think I found the "reason" this happend at all:

as per vscode-versions there was a switch inside vscode bringing nodejs from 16.17.1 to 18.15.0 in Aug 2023.

Further, as per https://github.com/nodejs/node/pull/39987 (and as explained in https://github.com/nodejs/node/issues/40537#issuecomment-948047193) node 17 resolves/returns addresses as delivered by resolver/OS - whereas previously (node<17) it ordered them ip4 first.

birdspider commented 1 year ago

possible fix in nrepl, returning the actual address

diff --git a/src/clojure/nrepl/socket.clj b/src/clojure/nrepl/socket.clj
--- src/clojure/nrepl/socket.clj
+++ src/clojure/nrepl/socket.clj
@@ -207,9 +207,9 @@
           (URI. (str transport-scheme
                      (when (instance? SSLServerSocket sock)
                        "s"))
                 nil ;; userInfo
-                (-> sock .getInetAddress .getHostName)
+                (-> sock .getInetAddress .getHostAddress)
                 (.getLocalPort sock)
                 nil       ;; path
                 nil       ;; query
                 nil)))))) ;; fragment

maybe @bbatsov has some ideas if this is to handle in nrepl or calva

PEZ commented 1 year ago

Thanks for doing all this research! Please consider reporting it on the nrepl repository.