cider-jack-in fails when cider-enlighten-mode is enabled

[plexus]

Expected behavior

cider-jack-in results in a buffer with a REPL prompt

Actual behavior

The minibuffer pops up with Lisp expression: . Entering anything there results in

Debugger entered--Lisp error: (error "Not an nREPL dict object: (+ 1 1)")  
signal(error ("Not an nREPL dict object: (+ 1 1)")) 
error("Not an nREPL dict object: %s" (+ 1 1))  
nrepl-dict-get((+ 1 1) "interns")  
cider-resolve-ns-symbols((+ 1 1))  
cider-refresh-dynamic-font-lock((+ 1 1))  
cider-set-buffer-ns((+ 1 1)) 
 cider-repl-set-initial-ns(#<buffer *cider-repl App*>)  
cider-repl-init(#<buffer *cider-repl App*>)  
cider--connected-handler()  run-hooks(nrepl-connected-hook)  
nrepl-start-client-process(nil 42389 #<process nrepl-server>)  
nrepl-server-filter(#<process nrepl-server> "nREPL server started on port 42389 on host 127.0.0.1 - nrepl://127.0.0.1:42389\n")

This shows up in nrepl-messages:

(--> 
  op  "eval"
  code  "(str *ns*)"
  enlighten  "true"
  session  "b72d41fa-6418-441f-bc52-cc8e7138c986"
  id  "4"
)
(<-- 
  ex  "class java.lang.Exception"
  id  "4"
  root-ex  "class java.lang.Exception"
  session  "b72d41fa-6418-441f-bc52-cc8e7138c986"
  status  ("eval-error")
)
(<-- 
  err  "Exception Debugger not initialized  user/eval27659/fn--27660 (form-init2222324724219143458.clj:1)
"
  id  "4"
  session  "b72d41fa-6418-441f-bc52-cc8e7138c986"
)

The user gets an empty repl buffer, but pressing enter does bring up a prompt. Except that it shows nil as the namespace.

Steps to reproduce the problem

Open a Clojure file, M-x cider-enlighten-mode, M-x cider-jack-in.

CIDER version information

;; CIDER 0.15.0snapshot (package: 20170129.1941), nREPL 0.2.12
;; Clojure 1.9.0-alpha14, Java 1.8.0_91

Emacs version

GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.18.9) of 2016-11-04

Operating system

Ubuntu 16.04.2 LTS

[dpsutton]

You can observe this in cider-repl-set-initial-ns.

    (defun cider-repl-set-initial-ns (buffer)
      "Set the REPL BUFFER's initial namespace (by altering `cider-buffer-ns').
    This is \"user\" by default but can be overridden in apps like lein (:init-ns)."
      ;; we don't want to get a timeout during init
      (let ((nrepl-sync-request-timeout nil))
        (with-current-buffer buffer
          (let ((initial-ns (or (read
                                 (nrepl-dict-get
                                  (cider-nrepl-sync-request:eval "(str *ns*)")
                                  "value"))
                                "user")))
            (cider-set-buffer-ns initial-ns)))))

You get the response from the nrepl eval:

Result: (dict "status" ("eval-error" "done") "ex" "class java.lang.Exception" "id" "4" "root-ex" "class java.lang.Exception" "session" "8c93729e-0cfd-42a7-b7e6-42474d680cfd" "err" "Exception Debugger not initialized  user/eval20213/fn--20214 (form-init3297300632922439840.clj:1)\n")

So it is a bug in our invocation of read. Something is throwing an error in start up. I don't know if its fatal or not. But then we are calling (read nil) since our dictionary lookup fails which is the signature that you call if you want emacs to prompt the user.

So a short term fix is to prevent reading on nil and a long term fix is to figure out why we are in a bad state here.

[dpsutton]

And the problem is that enlighten mode evals things with the debugger. The debugger is initialized on item 6 of CIDER startup. Evaling the current ns for the repl is item 4. So we try to eval with the debugger before the debugger has been initialized and there's where we get our error.

[bbatsov]

We should probably make enligthen-mode do nothing in the absence of a connection.

[dpsutton]

Its not that there's not a connection, it's that enlighten hijacks eval but we eval things before the hijack has been fully set up.

There's a function called nrepl--eval-request that takes care of ensuring the op list of what to do is well formatted. And this checks to see if enlighten mode is on, and if so, adds it to the op code.

(defun nrepl--eval-request (input &optional ns line column)
  "Prepare :eval request message for INPUT.
NS provides context for the request.
If LINE and COLUMN are non-nil and current buffer is a file buffer, \"line\",
\"column\" and \"file\" are added to the message."
  (append (and ns (list "ns" ns))
          (list "op" "eval"
                "code" input)
          (when cider-enlighten-mode
            (list "enlighten" "true"))
          (let ((file (or (buffer-file-name) (buffer-name))))
            (when (and line column file)
              (list "file" file
                    "line" line
                    "column" column)))))

So we could do one of a few things:

1. Have the enlighten inhibited if we are still initializing the repls. This seems like the right thing to do but comes with the downside of how we pass that information here. There are already several optional arguments so passing it in directly is kinda gross. Perhaps we can switch over to the cl-defun so that we can pass in parameters with more fine grained precision that just position 7 of a bunch of nils.

2. We can reorder the sequence of events. Right now, there is a dance of steps that happen on jack in: clone the session for eval-ing, clone the session for tooling, get the startup ns, eval some code that we require for the repl, intialize the debugger, subscribe to the out printstream, etc.
   We could simply reorder these to initialize the debugger before eval-ing the ns and thereby resolve our problem. This has an added benefit of making us clean up where these different start up tasks are called from, as they are spread out quite far in the code. There's a mess of callbacks and such that I'm not sure are still required. It would be nice if we could make a single function called cider-repl-startup-tasks that called the 7 or 8 startup tasks required. Then re-sequencing would be trivial.

[bbatsov]

I think I like option 2 more as a potential solution.

[xiongtx]

@dpsutton Try using backquote and , instead of list everywhere. Also, since the lists are not stored in vars, we can use nconc.

(nconc (and ns `("ns" ,ns))
       `("op" "eval"
         "code" ,input)
       (when cider-enlighten-mode
         '("enlighten" "true"))
       (let ((file (or (buffer-file-name) (buffer-name))))
         (when (and line column file)              
           `("file" ,file
             "line" ,line
             "column" ,column))))

[dpsutton]

Sounds good. I was just quoting preexisting code in this case though.