nrepl/piggieback

out and err have the wrong message ID, vanish entirely on reconnection

Olical opened this issue · 0 comments

I've been doing a little digging into a simple piggieback nREPL server. I've found that messages to out and err have the message ID of the call that started piggieback in the first place, not the message that triggered the output. For example.

  • I eval to start piggieback in my nREPL session (id: foo)
  • I get a response saying I can use :cljs/quit to exit (id: foo)
  • I eval (println "hey") (id: bar)
  • I see the output of "hey", (id: foo)
  • Then I see the return value of the eval (id: bar, as expected)

I had a dig into the source trying to fix it myself and realised output-bindings doesn't seem to do anything.

(defn- output-bindings [{:keys [session] :as msg}]
(when-let [replying-PrintWriter (resolve 'nrepl.middleware.print/replying-PrintWriter)]
{#'*out* (replying-PrintWriter :out msg {})
#'*err* (replying-PrintWriter :err msg {})}))
(defn do-eval [{:keys [session transport ^String code ns] :as msg}]
(with-bindings (merge {#'ana/*cljs-warnings* ana/*cljs-warnings*
#'ana/*cljs-warning-handlers* ana/*cljs-warning-handlers*
#'ana/*unchecked-if* ana/*unchecked-if*
#'env/*compiler* (get @session #'*cljs-compiler-env*)}
;; *repl-env* was added in CLJS 1.10.126
(when-let [v (find-var 'cljs.repl/*repl-env*)]
{v (get @session #'*cljs-repl-env*)})
@session
(when ns
{#'ana/*cljs-ns* (symbol ns)})
(output-bindings msg))
(let [repl-env *cljs-repl-env*
repl-options *cljs-repl-options*
init-ns ana/*cljs-ns*
special-fns (merge cljs.repl/default-special-fns (:special-fns repl-options))
is-special-fn? (set (keys special-fns))]

You can take it out completely and get the exact same result, so I think these bindings aren't being used and they're falling through to the outer environment which still has the original message ID from when piggieback started.

I also discovered that if I reconnect to the nREPL server and assume the same session I won't see any out or err whatsoever. I think this is to do with the same issue, I'm pretty sure they'd both be fixed by the same change.

My theory is the bindings just need to go elsewhere, where CLJS can hook into the pre-defined out and err handlers, I just can't work out where. If anyone knows where those bindings should be (or what their vars should be called?) I'll happily make the change.