Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
RickMoynihan opened this issue May 15, 2019 · 11 comments

Comments

@RickMoynihan
Copy link
Contributor

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.

(rewrite-args @current-ep-id
[{:file "(in-ns 'bf.nrepl)\n\n\n\n\n\n\n\n\n(+ 1 2)", :file-name "nrepl.cljc", :file-path "/Users/val/projects/breakform/src/bf/nrepl.cljc", :id "65172bfb-6638-4b40-a8b7-85b9932d7c09", :op "load-file", :session "144d968e-5ccb-4f41-9f98-69c76ac73e7b", :transport nil}])

@vvvvalvalval
Copy link
Owner

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
Copy link
Contributor Author

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

@vvvvalvalval
Copy link
Owner

@RickMoynihan Maybe by doing something like

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

?

@RickMoynihan
Copy link
Contributor Author

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

@mjmeintjes
Copy link

mjmeintjes commented Aug 3, 2022

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
Copy link

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
Copy link

mjmeintjes commented Aug 4, 2022

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
Copy link
Owner

vvvvalvalval commented Aug 6, 2022

@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
Copy link

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

@RickMoynihan
Copy link
Contributor Author

Did you ever push that fork @mjmeintjes?

RickMoynihan added a commit to RickMoynihan/scope-capture-nrepl that referenced this issue Dec 19, 2022
@RickMoynihan
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants