Fix stacktrace extraction for clojurescript repls

[dhleong]

Originally written to go along with #222, these changes also prevent errors when using the existing Piggieback, as well.

Specifically, this solves the error that would get thrown when whatever you executed in a clojurescript quasi-repl raised an Exception.

[dhleong]

@Deraen Woops, you're right. Removed them

[tpope]

Overlooked this PR but I think it was fixed elsewhere. Please do follow up if there are still issues.

[dhleong]

I just checked with the latest master and it looks like it's still a problem. My repro is:

1. Open a clojurescript file
2. Connect to a clojurescript repl and use piggieback as: Piggieback (figwheel-sidecar.repl-api/repl-env)
3. Use the pseudo repl via the cqc mapping and execute something that'll throw an exception, eg: (clojure.string/split nil).

On master, this just shows an error about how (cljs.core._STAR_e.getStackTrace is not a function). There is no location list at all, either, so there's no way to know what happened.

On this branch, it shows WARNING: Wrong number of args (1) passed to clojure.string/split at line 1 <cljs repl> etc, and a short stack pointing to the clojure/string.cljs file. There is a location list here, but it points to js files instead of cljs files so isn't super useful. I didn't do anything specific to inflate a clojurescript location list here, though, so that's not overly surprising.

[tpope]

So I am wary of adding is_piggieback state, and especially averse to coupling the adapter to the current filename with expand('%:e'). Is there a way we can tell it's not a plain Clojure stacktrace just by looking at the response?

[dhleong]

Fair enough. It's been a while since I looked at this, but after playing around a bit this morning, here's what I've got:

1. When running code in a Rhino repl (which I achieved by starting a regular lein repl on a clojurescript project and not doing any piggieback stuff), the value on *e is an EcmaError which has a .getStackTrace() method but which, for some reason, throws an exception on (map str *e). To get around this, I used amap in this branch as-is, and that seems to work.
2. When running in a Piggieback repl, there is no .getStackTrace(), but .-stack returns a string that's already nicely \n-delimited. The Rhino repl returns nil for this field, while Clojure understandably throws a "no such field" exception.
3. Since Clojure's exceptions are Java's exceptions, we know that .getStackTrace() will always return an array of java.lang.StackTraceElement. This is not true on Rhino, however, so we can check if we're in Clojure using (= "class [Ljava.lang.StackTraceElement;" (str (type stack-trace))).
4. Clojure try-catch is not compatible with Clojurescript try-catch. Clojure requires you use java.lang.Throwable to catch any errors, while Clojurescript requires :default, and neither will accept the other's. So, we can't use that to distinguish environments
5. We can check if we're running in a Clojurescript environment like this: (= "#'cljs.core/str" (str #'str)). This seems pretty safe, and works as expected across the above three cases (clojure, rhino, piggieback).

Putting these together, here's what I came up with:

(let [st (or (when (= "#'cljs.core/str" (str #'str))
               (.-stack *e))
             (.getStackTrace *e)) ]
  (symbol
    (str "\n\b"
         (if (string? st)
           st
           (let [parts (if (= "class [Ljava.lang.StackTraceElement;" (str (type st)))
                         (map str st)
                         (amap st idx ret (str (aget st idx))))]
             (apply str (interleave (repeat "\n") parts))))
         "\n\b\n")))

The new fix, then, is to simply replace the let format_st line with:

  let format_st = '(let [st (or (when (= "#' . "'" . 'cljs.core/str" (str #' . "'" . 'str)) (.-stack *e)) (.getStackTrace *e))] (symbol (str "\n\b" (if (string? st) st (let [parts (if (= "class [Ljava.lang.StackTraceElement;" (str (type st))) (map str st) (amap st idx ret (str (aget st idx))))] (apply str (interleave (repeat "\n") parts)))) "\n\b\n")))'

If this works for you I can open a new PR or munge this one, whichever you prefer. Let me know what you think!

[tpope]

Looks great. Updating this PR will be fine.

• Try to make it a bit more readable than a one liner.
• You can use two single quotes ('') inside a single quoted string to include one single quote.

[dhleong]

Updated the PR.

While testing this I discovered another problem, but I think it's unrelated to this issue. When executing something like (deref 1), the exception will be caught and printed as expected. With something like (map deref 1), however, the repl doesn't seem to "catch" the error, and the throw string(response) line is triggered, with the error message in the err field of that map.

This happened even when I made the format_st = '(symbol "Woops")'—and the exception is properly "caught" from a regular clojure repl—so I think the source of that problem must be elsewhere.