babashka / scittle

Execute Clojure(Script) directly from browser script tags via SCI
https://babashka.org/scittle
Eclipse Public License 1.0
326 stars 30 forks source link

Invocations of `load-string` and/or `js/scittle.core.eval_string` from within a Scittle script lose error context (line and column) #33

Closed jurjanpaul closed 2 years ago

jurjanpaul commented 2 years ago

Errors in Scittle scripts are logged with full context (position, code snippet), which is great! (Even so, I found an exception to this rule, w.r.t. unresolvable symbols in the analysis phase, but that's for another issue.)

It would be great if the same quality of error logging, particularly including position info, would be available when invoking load-string or js/scittle.core.eval_string from within a Scittle script (to facilitate an in-browser Clojure(Script) playground), but at the moment that is not the case for both these functions.

So, the following logs an error with informative context:

<html>
  <head>
    <script src="https://cdn.jsdelivr.net/npm/scittle@0.2.8/dist/scittle.js" type="application/javascript"></script>
    <script type="application/x-scittle">
(defn f []
  (subs nil 42))

(defn g []
  (f))

(g)
    </script>
  </head>
  <body>
  </body>
</html>

but the next example does not:

<html>
  <head>
    <script src="https://cdn.jsdelivr.net/npm/scittle@0.2.8/dist/scittle.js" type="application/javascript"></script>
    <script type="application/x-scittle">
(defn try-load-string [s]
  (binding [*file* "foo"] ; included per suggestion in Slack, but doesn't yet help getting to the line and column
    (try
      ;;(load-string s)
      (js/scittle.core.eval_string s)
      (catch :default e
        (println "Caught error:" e)
        (prn e)
        (.log js/console e)))))

(try-load-string "
(defn f []
  (subs nil 42))

(defn g []
  (f))

(g)
")
    </script>
  </head>
  <body>
  </body>
</html>

The aim here is of course to get to the error position within the provided string, not in the context of the Scittle script itself (which is what gets logged when the (try ... (catch ...)) is omitted.

jurjanpaul commented 2 years ago

The actual context might make this wish clearer, so here’s my Scittle based Away from Preferred Editor Clojure(Script) Playground: https://jurjanpaul.github.io/ape-cljs-playground/

borkdude commented 2 years ago

@jurjanpaul That's really cool! This problem is on my radar, currently on a vacation and working through some other stuff first.

jurjanpaul commented 2 years ago

Thank you @borkdude! No hurries whatsoever! (I still have no idea how hard this would be to solve.) Enjoy the time off and the other stuff. 🙂

borkdude commented 2 years ago

Try this in the next (forthcoming) release:

      (require '[sci.core :as sci])
      (defn try-load-string [s]
       (try
         (js/scittle.core.eval_string s)
        (catch ^:sci/error js/Error e
         (run! println (-> (sci/stacktrace e) (sci/format-stacktrace))))))

(try-load-string "
(defn f []
  (subs nil 42))

(defn g []
  (f))

(g)
")
jurjanpaul commented 2 years ago

Thank you @borkdude ! That indeed fixes things. Really nice.

I only wish I could understand your code (changes) a bit better. :-)

I don't even quite understand the ^:sci/error part of the catch clause you suggest. (Is it a metadata flag that gets set on the caught value of e, presumably because sci/stacktrace needs it? I haven't yet found any reference for this syntax.)

I ended up using load-string again instead of js/scittle.core.eval_string (mainly because it would not - prematurely - log any exception to console) and to my surprise that also works just as well with sci/stacktrace!

Thanks again!

borkdude commented 2 years ago

This is a special SCI feature which communicates to SCI that it should return extra location information as part of the exception, that you can then feed into sci/stacktrace.

jurjanpaul commented 2 years ago

Aha! Thanks for explaining that particular magic! :-)