Hello, I'm a bit stuck trying to improve some stacktrace information when calling Hiccup from Sci, specifically with lazy sequences ๐งต
I've created an issue https://github.com/babashka/sci/issues/942 Hopefully it is clear enough
thanks!
Works well! Even some quick tries with transducers. I could find one more case specific to map i think:
["(str (map (fn [a] a) 1))" [1 6]] ;=> [1 1]I don't think that case is solvable due to laziness, clojure itself also doesn't know the location when you execute this:
test.clj
(str (
map
(fn [a] a)
1))
user=> (load-file "/tmp/test.clj")
Execution error (IllegalArgumentException) at java.lang.Object/toString (Object.java:270).
Don't know how to create ISeq from: java.lang.Longif I would radically re-engineer SCI in how the stack trace is produced, I could maybe make it work...
Yeah can imagine this one is tricky. But to be fair I think Sci's error reporting is close to be better than Clojure's error reporting ๐
just out of curiosity, what are you building with SCI on the JVM?
Something like pcp , like a web framework, but it is very early days
But I try to use the error information to display inline errors. A bit like babashka
I use it with hiccup, so if the error information is not precise the displayed error is not very helpful
In general with the column information you normally get very nice errors with Sci
I'm using the stacktrace information to highlight the exact location in the code
The problem is that lazy sequences are only triggered during the html building, so the Sci stacktraces don't point to the line of a for call
To illustrate this problem:
;; line 1, column 1
(try (sci/eval-string "(str
(for [[a] 1] a))")
(catch Exception e
(select-keys (ex-data e) [:line :column])))
;; vs doall, line 2, column 26
(try (sci/eval-string "(str
(doall (for [[a] 1] a)))")
(catch Exception e
(select-keys (ex-data e) [:line :column])))
A quick hack is to change the 'for' binding to a for wrapped with doall
But I would prefer to do this only in the context of hiccup and not for the whole Sci context. Not sure if this can be done from a macro?
The other thing I was thinking about is to port the Hiccup interpreter to Sci and do something with the stacktraces, but I'm not sure if I can manipulate the stacktrace in a sensible manner this way
It seems to be only an issue when something goes wrong in the destructuring or in the arguments of for or map et al. If there is an issue in the body of the macro or function the stacktrace is correct
e.g.
(try (sci/eval-string "(str
(for [a 1] (/ a 0) a))")
(catch Exception e
(select-keys (ex-data e) [:line :column])))
=> {:line 2, :column 26}vs
(try (sci/eval-string "(str
(map (fn [a] (/ a 0)) 1))")
(catch Exception e
(select-keys (ex-data e) [:line :column])))
=> {:line 1, :column 1}hey @jeroenvandijk let's see...
yeah I think this is just how laziness works unfortunately, not much SCI can do here I'm afraid
you're using SCI directly for hiccup, so this isn't related to bb or scittle or so right?
Yeah Sci directly for hiccup
This is another hack I found
(sci/eval-string* (sci/init {:deny '[for]
:bindings {'for2 (macrofy (fn [&form & args]
(list (with-meta 'doall (meta &form)) (apply for-macro/expand-for &form args))))
'html (macrofy
(fn [_ _ body]
(clojure.walk/postwalk (fn [x]
(if (= x 'for)
(with-meta 'for2 (meta x))
x))
body)))}
})
"(html (for [[i] (range 100)] (/ i 0)))")
So the hiccup macro would change the bindings, although it can mess thing up with other things named foryou could just do this:
{:namespaces {'clojure.core {'for ...}}}
to replace foryeah true, but then i would do it everywhere, even if I am not rendering html. I was hoping to isolate more to hiccup
I was also wondering if there is a way to access the ctx from a macro, but i think this is not possible, right?
you could use with-redefs in your macro, but also this would not play well with laziness
the way to access the ctx from a macro would be something like this:
(require '[sci.ctx-store :as store])
(def ctx (sci/init ..))
(store/reset-ctx! ctx)
And in the macro call (store/get-ctx)hmmm i will give that I try. I think in sci you don't have to worry about thread safety for with-redefs
> I think in sci you don't have to worry about thread safety for with-redefs hmm, it depends if you're running SCI in multiple threads
I do, but maybe I'm still safe because of sci/fork. Although not sure
the built-in vars are all shared
ah i see
so maybe it would be the safest to make a combination of what we had. copy the SCI for macro into a new var, add this to clojure.core and then use with-redefs
or what you could also do
make the for macro in SCI dynamic and then use binding
then it would be thread-safe
ah cool, yeah that's smart
I will give it a try, thanks!
I can't seem to bind the macro to the dynamic var. Here is some of my experiment. I'm probably missing something
(require '[sci.impl.for-macro :as for-macro])
(def for-macro (sci/new-var 'for nil #_for-macro/expand-for {;:sci/macro true
:dynamic true}))
(sci/eval-string* (sci/init {:namespaces {'hiccup {'eager-for (macrofy (fn [&form & args]
'(throw (ex-info "for eager" {}))
#_(list (with-meta 'doall (meta &form))
(apply for-macro/expand-for &form args))))
'html (macrofy (fn [_ _ & body]
(concat (list 'binding '[clojure.core/for hiccup/eager-for])
body)))}
'clojure.core {'for for-macro
'*for* for-macro}}})
"(binding [*for* hiccup/eager-for] (*for* 2))"
#_ "(str
(binding [*for* hiccup/eager-for]
(clojure.core/*for* 2 ;#_[[i] (range 100)] (/ i 0)
)))"
#_ "(str
(hiccup/html
(for [[i] (range 100)] (/ i 0))))")I think you meant to make for a dynamic var, right? In clojure this would not be possible AFAIK
There's probably something hard-coded in SCI concerning for
But something like this isn't go work anyway:
(sci/eval-string* ctx "(defmacro my-for [& args] 42) (binding [for @#'my-for] (for [i [1 2 3]] i))")
since the binding doesn't take effect during macro-expansion, the macro is expanded at that timeso never mind what I said, too optimistic when not trying it for real in a REPL ;)
ah too bad, was worth a try though
so yeah I think your postwalk thing might be ok, but at the risk that you would also replace for local variables and function arguments etc ;)
Thanks for your help. I'll play with it and see how it goes
One more question, since it is possible to point to the exact location in case of an error in the body of a lazy sequence e.g. in the case of (for [i (range 10)] (/ i 0)) or (map (fn [i] (/ i 0)) (range 10)), sci can point out the exact location of /. Shouldn't this be possible somehow for the destructuring bit for instance? I tried to look at the code, but I don't see directly where all this is happening
hmm, you may be right, can you maybe post a SCI issue about a better error location reporting for your case? perhaps it's just a matter of fixing some metadata somewhere
Fixed:
$ clj -M:babashka/dev -e '(str (let [[a] 1] a))'
----- Error --------------------------------------------------------------------
Type: java.lang.UnsupportedOperationException
Message: nth not supported on this type: Long
Location: <expr>:1:6
----- Context ------------------------------------------------------------------
1: (str (let [[a] 1] a))
^--- nth not supported on this type: LongHm, for should also still be improved
for is a lot harder due to laziness probably
Can imagine. I will have a look at your fix. I didn't know where to look actually to fix it
o I wait, I do see something improving here:
1: (str (for [[a] [0]] (/ 1 a)))
^--- nth not supported on this type: LongThanks!
anyway, I'll also dig a little further
> o I wait, I do see something improving here: I think that's a major improvement
Looks small here, but in a bigger context it is significant
are you using this from JVM or CLJS?
From the JVM
pushed another improvement for for. feel free to post more cases
Cool! I found one more. Do you want me to post it here?
Failing:
["(str (map (fn [[a]] a) [0]))" [1 6]] ;=> [1 1]
["(str (filter (fn [[a]] a) [0]))" [1 6]] ;=> [1 1]
I'm guessing this is a different kind of caseFailing:
["(str (if-let [[a] 0] a))" [1 6]] ;=> [1 1]
["(str (when-let [[a] 0] a))" [1 6]] ;=> [1 1]
["(str (if-some [[a] 1] a))" [1 6]] ;=> [1 1]
["(str (when-some [[a] 1] a))" [1 6]] ;=> [1 1]
I'm guessing this is more of what you did beforeOk:
["(str (loop [[a] 0] a))" [1 6]]
["(str (letfn [(f [[a]] a)] (f 1)))" [1 27]]thanks!
np!
all of the above work now
also handled most doseq cases
Wow, that's amazing. Thank you!