CLJS-1057: Nashorn REPL should not use EDN rep for errors

Clean up error handling in general. Giving functions top level names is very noisy and
introduces incredible complexity with respect to function name printing, every engine
is different.

Author: dnolen

src/clj/cljs/repl.clj

@@ -109,9 +109,9 @@
   IReplEnvOptions
   (-repl-options [_] nil))
 
-(defprotocol IParseErrorMessage
-  (-parse-error-message [repl-env message error build-options]
-    "Given the original JavaScript error message return the string to actually
+(defprotocol IParseError
+  (-parse-error [repl-env error build-options]
+    "Given the original JavaScript error return the error to actually
      use."))
 
 (defprotocol IGetError
@@ -271,8 +271,7 @@
                 [line' column'] (if ns-info
                                   (mapped-line-and-column sm line column)
                                   [line column])
-                name' (if (and ns-info function)
-                        (symbol (name ns) (cljrepl/demunge function))
+                name' (when (and ns-info function)
                         function)
                 file' (if no-source-file?
                         file
@@ -994,20 +993,16 @@ str-or-pattern."
   ([e]
    (let [{:keys [repl-env] :as env} &env]
      (when (and e repl-env)
-       (let [ret (if (satisfies? IGetError repl-env)
+       (when-let [ret (if (satisfies? IGetError repl-env)
                    (-get-error repl-env e env *repl-opts*)
                    (edn/read-string
                      (evaluate-form repl-env env "<cljs repl>"
                        `(when ~e
                           (pr-str
                             {:value (.-message ~e)
                              :stacktrace (.-stack ~e)})))))]
-         (when ret
-           (let [ret (update-in ret [:value]
-                       (fn [msg]
-                         ;; give REPL environments a chance to fix or
-                         ;; or elide redundant information
-                         (if (satisfies? IParseErrorMessage repl-env)
-                           (-parse-error-message repl-env msg ret *repl-opts*)
-                           msg)))]
-             (display-error repl-env ret nil *repl-opts*))))))))
+         (display-error repl-env
+           (if (satisfies? IParseError repl-env)
+             (-parse-error repl-env ret *repl-opts*)
+             ret)
+           nil *repl-opts*))))))

src/clj/cljs/repl/nashorn.clj

@@ -15,7 +15,8 @@
             [cljs.repl :as repl]
             [cljs.compiler :as comp]
             [cljs.closure :as closure])
-  (:import [javax.script ScriptEngine ScriptEngineManager ScriptException ScriptEngineFactory]
+  (:import [java.io File]
+           [javax.script ScriptEngine ScriptEngineManager ScriptException ScriptEngineFactory]
            [jdk.nashorn.api.scripting NashornException]))
 
 ;; Nashorn Clojurescript repl binding.
@@ -141,12 +142,6 @@
       (string/replace-first file-name with-slash "")
       file-name)))
 
-(defn- convert-stacktrace-element [^StackTraceElement el]
-  {:function (.getMethodName el)
-   :file (.getFileName el)
-   :line (.getLineNumber el)
-   :column 0})
-
 (def repl-filename "<cljs repl>")
 
 (defrecord NashornEnv [engine debug]
@@ -183,26 +178,43 @@
     (try
       {:status :success
        :value (if-let [r (eval-str engine js)] (.toString r) "")}
-
-      ;; Stringify the stacktrace to edn for easy parsing in -parse-stacktrace
       (catch ScriptException e
         (let [^Throwable root-cause (clojure.stacktrace/root-cause e)]
           {:status :exception
            :value (.getMessage root-cause)
-           :stacktrace (pr-str (map convert-stacktrace-element
-                                    (NashornException/getScriptFrames root-cause)))}))
+           :stacktrace (NashornException/getScriptStackString root-cause)}))
       (catch Throwable e
         (let [^Throwable root-cause (clojure.stacktrace/root-cause e)]
           {:status :exception
            :value (.getMessage root-cause)
-           :stacktrace (pr-str (map convert-stacktrace-element (.getStackTrace root-cause)))}))))
+           :stacktrace
+           (apply str
+             (interpose "\n"
+               (map str
+                 (.getStackTrace root-cause))))}))))
   (-load [{engine :engine :as this} ns url]
     (load-ns engine ns))
   (-tear-down [this])
   repl/IParseStacktrace
   (-parse-stacktrace [this frames-str ret {output-dir :output-dir}]
-    (when-let [frames (read-string frames-str)]
-      (vec (map #(update-in %1 [:file] (fn [s] (strip-file-name s output-dir))) frames)))))
+    (vec
+      (map
+        (fn [frame-str]
+          (let [frame-str (string/replace frame-str #"\s+at\s+" "")
+                [function file-and-line] (string/split frame-str #"\s+")
+                [file-part line-part] (string/split file-and-line #":")]
+            {:file (string/replace (.substring file-part 1)
+                     (str output-dir File/separator) "")
+             :function function
+             :line (Integer/parseInt
+                     (.substring line-part 0 (dec (.length line-part))))
+             :column 0}))
+        (string/split frames-str #"\n"))))
+  repl/IParseError
+  (-parse-error [_ err _]
+    (update-in err [:stacktrace]
+      (fn [st]
+        (string/join "\n" (drop 1 (string/split st #"\n")))))))
 
 (defn repl-env* [{:keys [debug] :as opts}]
   (let [engine (create-engine opts)]

src/clj/cljs/repl/node.clj

@@ -173,8 +173,9 @@
   (-repl-options [this]
     {:output-dir ".cljs_node_repl"
      :target :nodejs})
-  repl/IParseErrorMessage
-  (-parse-error-message [_ _ _ _])
+  repl/IParseError
+  (-parse-error [_ err _]
+    (assoc err :value nil))
   repl/IJavaScriptEnv
   (-setup [this opts]
     (setup this opts))