vvvvalvalval / scope-capture-nrepl

nREPL middleware for scope-capture
MIT License
30 stars 2 forks source link

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

Open RickMoynihan opened 5 years ago

RickMoynihan commented 5 years ago

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

vvvvalvalval commented 5 years ago

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,

RickMoynihan commented 5 years ago

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

vvvvalvalval commented 5 years ago

@RickMoynihan Maybe by doing something like

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

?

RickMoynihan commented 5 years ago

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

mjmeintjes commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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))
vvvvalvalval commented 2 years ago

@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.
mjmeintjes commented 2 years ago

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

RickMoynihan commented 1 year ago

Did you ever push that fork @mjmeintjes?

RickMoynihan commented 1 year ago

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.