babashka.nrepl - Emacs Cider "Lisp Expression:" minibuffer prompt when switching projects

[kwrooijen]

Expected behavior

Connect to babashka.nrepl like any other Clojure NREPL server.

Actual behavior

When connecting to to a babashka.nrepl server everything seems to work fine until I try to switch files (mostly from another project) Emacs will keep prompting "Lisp Expression:" in the minibuffer. Sometimes it also completely locks my Emacs instance.

Steps to reproduce the problem

1. Start the repl server with lein run (code example below)
2. Using Emacs, navigate to any Clojure project
3. M-x cider-connect >> localhost >> 23456
4. Eval some stuff (this works)
5. Navigate to another clojure project and open a clj file >> Lisp expression: should pop up

Environment & Version information

Minimal Emacs config (Only install clojure + cider)

(setq gc-cons-threshold most-positive-fixnum)
(defvar bootstrap-version)

(let ((bootstrap-file
       (expand-file-name "straight/repos/straight.el/bootstrap.el" user-emacs-directory))
      (bootstrap-version 5))
  (unless (file-exists-p bootstrap-file)
    (with-current-buffer
        (url-retrieve-synchronously
         "https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el"
         'silent 'inhibit-cookies)
      (goto-char (point-max))
      (eval-print-last-sexp)))
  (load bootstrap-file nil 'nomessage))

(straight-use-package 'use-package)

(use-package clojure-mode :straight t)

(use-package cider :straight t)

Minimal babashka nrepl setup:

;; project.clj
(defproject nrepl-test "0.1.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.10.3"]
                 [babashka/babashka.nrepl "0.0.6"]]
  :main ^:skip-aot nrepl-test.core)

;; src/nrepl-test/core.clj
(ns nrepl-test.core
  (:require
   [sci.core :as sci]
   [sci.addons :as addons]
   [babashka.nrepl.server])
  (:gen-class))

(defn start-server! []
  (let [opts {:namespaces {'clojure.main {'repl-requires '[]}}}
        sci-ctx (-> (sci/init opts)
                    addons/future)]
    (babashka.nrepl.server/start-server! sci-ctx {:host "127.0.0.1" :port 23456})))

(defn -main [& args]
  (start-server!)
  @(promise))

CIDER version information

;; Connected to nREPL server - nrepl://localhost:23456
;; CIDER 1.3.0 (Ukraine)
: Nothing specified to load user 
WARNING: Can't determine Clojure version.  The refactor-nrepl middleware requires clojure 1.8.0 (or newer)
WARNING: clj-refactor and refactor-nrepl are out of sync.
Their versions are 3.5.2 and n/a, respectively.
You can mute this warning by changing cljr-suppress-middleware-warnings.

Emacs version

29.0.50

Operating system

Linux Laptop 5.16.14-1-MANJARO

Notes

I initially opened an issue at babashka.repl babashka/babashka.nrepl#50 but was redirected here.

[bbatsov]

Some stack trace would be very useful. Otherwise it's really hard to guess what might be going wrong.

[kwrooijen]

I used M-x toggle-debug-on-error and was able to find the cause of the repeating prompt.

  read-minibuffer("Lisp expression: ")
  cider-fallback-eval:classpath()
  cider-classpath-entries()
  #f(compiled-function (system session) "Check if SESSION is a friendly session." #<bytecode -0x1c94929b74a12bfd>)(CIDER ("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>))
  apply(#f(compiled-function (system session) "Check if SESSION is a friendly session." #<bytecode -0x1c94929b74a12bfd>) CIDER ("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>))
  sesman-friendly-session-p(CIDER ("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>))
  #f(compiled-function (ses) #<bytecode 0xa03aa1cc36b991>)(("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>))
  #f(compiled-function (elt) #<bytecode -0x1255077f45cad3df>)(("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>))
  mapcar(#f(compiled-function (elt) #<bytecode -0x1255077f45cad3df>) (("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>)))
  #f(compiled-function #'sequence #<bytecode 0x184349fed658a6b4>)(#f(compiled-function (elt) #<bytecode -0x1255077f45cad3df>) (("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>)))
  apply(#f(compiled-function #'sequence #<bytecode 0x184349fed658a6b4>) #f(compiled-function (elt) #<bytecode -0x1255077f45cad3df>) (("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>)) nil)
  seq-map(#f(compiled-function (elt) #<bytecode -0x1255077f45cad3df>) (("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>)))
  seq-filter(#f(compiled-function (ses) #<bytecode 0xa03aa1cc36b991>) (("Clojure/sci-test:localhost:23456" #<buffer *cider-repl Clojure/sci-test:localhost:23456(clj)*>)))
  sesman--friendly-sessions(CIDER sort)
  sesman-current-session(CIDER)
  cider-repls(clj nil)
  cider-current-repl()
  cider-connected-p()

This part of the codes gets ran, but because nil is fed to the read function it prompts the user for a Lisp Expression.

cider/cider-client.el

Lines 596 to 599 in 9130c64

	(let ((classpath (thread-first "(seq (.split (System/getProperty \"java.class.path\") \":\"))"
	(cider-sync-tooling-eval)
	(nrepl-dict-get "value")
	read))

Checking if the value is nil before calling read solves the issue for me. But I don't know if that's a proper solution?

[kwrooijen]

It looks like this was also a fix for an old bug:
4d915c3#diff-22d9c9ae87a161af132b0f6bbe63ab7e926347270900421f08df084c3e1e5801R138-R140

[kwrooijen]

After doing some more digging: The reason this breaks is because babashka.nrepl doesn't implement any functions. So by default System/getProperty doesn't exist. Implementing this function in the NREPL fixes the issue.

So then the question is if CIDER wants to implement a safety net for this case (for example, check if the value is nil before calling read). Or if we leave it as is?

[bbatsov]

I wonder if there's something else we can evaluate to get the classpath for bb, or if system properties make some sense for bb in general. @borkdude any thoughts/suggestions about this?

[borkdude]

@bbatsov I think the issue is not babashka. Babashka has System/getProperty (although bb doesn't put classpath in there, at least CIDER doesn't crash). The issue is more general: babashka.nrepl is a library which can be used from any SCI based program. So it depends on how people configure that environment whether it supports certain things.

[bbatsov]

@borkdude So in some cases these methods might be missing? Is this what's happening here? I'm just not sure what should be the right workaround for this particular issue.

[borkdude]

@bbatsov In general nREPL should only assume the nREPL protocol and not too much about the exact implementation right, whether it's a JVM, Node.js, Python, whatever runtime? Why is there a specific discussion about System/getProperty is not clear to me in the first place.

[bbatsov]

@borkdude Because of this bit

cider/cider-client.el

Lines 596 to 599 in 9130c64

	(let ((classpath (thread-first "(seq (.split (System/getProperty \"java.class.path\") \":\"))"
	(cider-sync-tooling-eval)
	(nrepl-dict-get "value")
	read))

Seems this is the root cause for the ticket, we're currently discussing.

In general nREPL should only assume the nREPL protocol and not too much about the exact implementation right, whether it's a JVM, Node.js, Python, whatever runtime?

Generally, I agree with you, but in practice it's useful to evaluate a bit of Java code here and there instead of rolling out middleware for trivial things. CIDER has been historically coupled with Clojure, that's why we might have abused this reaching out to eval things here and there.

[borkdude]

(cider-sync-tooling-eval)

Perhaps that should gracefully deal with errors then?

[bbatsov]

No, that wouldn't be right. Each such evaluation should be handled depending on its context. From what I'm hearing I guess that if this blows up it's fine to just assume the classpath is empty.

[kwrooijen]

I think this could be fixed by adding a check before calling the read function. Then the error handling stays outside of cider-sync-tooling-eval. Similar to how a earlier issue was patched: 4d915c3#diff-22d9c9ae87a161af132b0f6bbe63ab7e926347270900421f08df084c3e1e5801R138-R140

Then classpath will be nil (or '() but I think that's the same in elisp?) and the user won't get the Elisp prompt.