scope-capture-nrepl icon indicating copy to clipboard operation
scope-capture-nrepl copied to clipboard

scope capture causes load-file operation to choke on aliased keywords

Open RickMoynihan opened this issue 5 years ago • 11 comments

I'm not sure why this is occuring but it seems scope-capture-nrepl somehow prevents cider/nrepl "load-file" operation from working; if the namespace being loaded includes an aliased keyword.

I'm not sure if this is peculiar to my setup but if I disable scope capture the operation appears to work, where as it would fail with scope capture.

A failing case would be something like this:

(ns my.ns
   (:require [clojure.set :as set]] ;; create an ns alias
))

::set/foo ;; => RuntimeException: InvalidToken: ::set/foo
java.lang.RuntimeException: Invalid token: ::api/foo
	at clojure.lang.Util.runtimeException(Util.java:221)
	at clojure.lang.LispReader.interpretToken(LispReader.java:390)
	at clojure.lang.LispReader.read(LispReader.java:283)
	at clojure.lang.LispReader.read(LispReader.java:196)
	at clojure.lang.LispReader.read(LispReader.java:190)
	at clojure.core$read.invokeStatic(core.clj:3664)
	at clojure.core$read.invokeStatic(core.clj:3639)
	at clojure.core$read.invoke(core.clj:3639)
	at sc.nrepl.impl$read_tl_forms.invokeStatic(impl.clj:32)
	at sc.nrepl.impl$read_tl_forms.invoke(impl.clj:12)
	at sc.nrepl.impl$add_letsc.invokeStatic(impl.clj:41)
	at sc.nrepl.impl$add_letsc.invoke(impl.clj:38)
	at sc.nrepl.impl$rewrite_args$fn__1917$fn__1918.invoke(impl.clj:59)
	at clojure.core$update.invokeStatic(core.clj:5960)
	at clojure.core$update.invoke(core.clj:5952)
	at sc.nrepl.impl$rewrite_args$fn__1917.invoke(impl.clj:59)
	at clojure.core$update.invokeStatic(core.clj:5960)
	at clojure.core$update.invoke(core.clj:5952)
	at sc.nrepl.impl$rewrite_args.invokeStatic(impl.clj:53)
	at sc.nrepl.impl$rewrite_args.invoke(impl.clj:48)
	at sc.nrepl.impl$wrap_handle.invokeStatic(impl.clj:73)
	at sc.nrepl.impl$wrap_handle.invoke(impl.clj:69)
	at sc.nrepl.middleware$wrap_letsc$fn__1945.doInvoke(middleware.clj:13)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at nrepl.middleware$wrap_conj_descriptor$fn__459.invoke(middleware.clj:16)
	at nrepl.server$handle_STAR_.invokeStatic(server.clj:18)
	at nrepl.server$handle_STAR_.invoke(server.clj:15)
	at nrepl.server$handle$fn__933.invoke(server.clj:27)
	at clojure.core$binding_conveyor_fn$fn__4676.invoke(core.clj:1938)
	at clojure.lang.AFn.call(AFn.java:18)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:835)

Additionally I've tried recreating things minimally by modifying the rewrite-args forms here to include the above ns in the string, but I can't seem to manufacture a failure that way; though I don't really understand the details here.

https://github.com/vvvvalvalval/scope-capture-nrepl/blob/a94c0fed6d14cc0876dd59f182ec49535bdc21a6/src/sc/nrepl/impl.clj#L78-L79

RickMoynihan avatar May 15 '19 09:05 RickMoynihan

Thanks for reporting this @RickMoynihan !

What would help me investigate is if we could have a look at the args the middleware is receiving.

Could you run this in your REPL:

;;;; instrumenting rewrite-args to see what it receives
(in-ns 'sc.nrepl.impl)

(def seen-args (atom [])

(defn rewrite-args
  [ep-id args]
  (swap! seen-args conj args)
  (if
    (nil? ep-id)
    args
    (update args 0
      (fn [arg]
        (cond
          (and
            (-> arg :op (= "load-file"))
            (string? (:file arg)))
          (update arg :file #(add-letsc ep-id %))

          (and
            (-> arg :op (= "eval"))
            (string? (:code arg)))
          (update arg :code #(add-letsc ep-id %))

          :else
          arg)))))

... then reproduce the error, and post me the result of @sc.nrepl.impl/seen-args ?

Cheers,

vvvvalvalval avatar May 15 '19 09:05 vvvvalvalval

Hmm... I can't easily print that form as it contains circular references.

RickMoynihan avatar May 15 '19 10:05 RickMoynihan

@RickMoynihan Maybe by doing something like

(binding [*print-level* 5]
   (prn @sc.nrepl.impl/seen-args))

?

vvvvalvalval avatar May 15 '19 11:05 vvvvalvalval

Hmmm... whenever I try and reproduce this in a minimal context it appears to work.

RickMoynihan avatar May 15 '19 23:05 RickMoynihan

I have the same issue - while in the context of an Execution Point, I cannot evaluate any code that contains an aliased namespaced keyword. If I replace the alias with the full namespace it works again.

The following throws an error: ::testns/abc The following works: :test.ns/abc

I get the same stacktrace as above.

mjmeintjes avatar Aug 03 '22 22:08 mjmeintjes

Here's some more info:

I wrapped rewrite-args update function in an exception handler, and the arg value that throws the exception is:

Contents: 
  :transport = nrepl.middleware.print$printing_transport$reify__3222@3f504e56
  :ns = "mattsum.task-feed.scratch"
  :nrepl.middleware.print/print-fn = nrepl.middleware.print$wrap_print$fn__3233$print__3235@d18305d
  :file = "/home/matthys/work/projects/clj-tools/src/mattsum/task_feed/scratch.clj"
  :nrepl.middleware.print/print = "cider.nrepl.pprint/pr"
  :op = "eval"
  :column = 3
  :line = 384
  :id = "4801"
  :code = "::ent/test"
  :nrepl.middleware.print/stream? = []
  :nrepl.middleware.print/options = { :length 100, :level 5, :print-level 5 }
  :session = "8dece820-41f0-4d63-b678-0c18647d1639"

What's strange is that when I manually run it I can't replicate the error, only when it is run as part of the nrepl middleware.

I can reliably replicate this, so please let me know if I can send more information.

mjmeintjes avatar Aug 04 '22 01:08 mjmeintjes

It seems to be a problem with the clojure reader when reading aliased keywords (see here as well)

I've changed the read-tl-forms fn to use rewrite-clj. It seems to work based on my limited testing, but requires a dependency on rewrite-clj:

(:require [sc.api]
          [clojure.string :as str]
          [rewrite-clj.node :as rwn]
          [rewrite-clj.parser :as rwp])
(defn read-tl-forms
  [^String s]
  (let [res (->> (rwp/parse-string-all s)
                 :children
                 (map (fn [n]
                        (rwn/string n)))
                 (remove #(-> % str/trim empty?))
                 vec)]
    res))

mjmeintjes avatar Aug 04 '22 08:08 mjmeintjes

@mjmeintjes thanks for investigating. Unfortunately I currently don't have much time for reviewing this; in the meantime:

  1. please do feel free to push a fork that can be consumed via Clojure Deps' :git/url
  2. aside from that, I can only recommend to make do without scope-capture-nrepl :/ I personally don't use it much, relying only on the basic scope-capture ops defsc and letsc.

vvvvalvalval avatar Aug 06 '22 10:08 vvvvalvalval

Thanks, I'll test it a bit more and then push a fork.

mjmeintjes avatar Aug 08 '22 03:08 mjmeintjes

Did you ever push that fork @mjmeintjes?

RickMoynihan avatar Dec 19 '22 11:12 RickMoynihan

Ok, I can confirm the suggested fix works, though obviously it involves picking up a dependency.

Regardless this is useful for me, and may be handy for others so I pushed a fork and coined a clojars release with @mjmeintjes patch.

If there's any interest in merging a PR with the fix here I can issue one.

RickMoynihan avatar Dec 19 '22 12:12 RickMoynihan