Skip to content

Commit 0035cd8

Browse files
puredangerstuarthalloway
puredanger
authored and
stuarthalloway
committed
CLJ-2430 Elevate error phase in throwable data
Signed-off-by: Stuart Halloway <stu@cognitect.com>
1 parent c4ebc0a commit 0035cd8

4 files changed

Lines changed: 90 additions & 60 deletions

File tree

‎src/clj/clojure/core/server.clj‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,10 @@
184184
:init repl-init
185185
:read repl-read))
186186

187+
(defn- ex->data
188+
[ex phase]
189+
(assoc (Throwable->map ex) :phase phase))
190+
187191
(defn prepl
188192
"a REPL with structured output (for programs)
189193
reads forms to eval from in-reader (a LineNumberingPushbackReader)
@@ -242,15 +246,15 @@
242246
true)))
243247
(catch Throwable ex
244248
(set! *e ex)
245-
(out-fn {:tag :ret :val (Throwable->map ex)
249+
(out-fn {:tag :ret :val (ex->data ex (or (-> ex ex-data :clojure.error/phase) :execution))
246250
:ns (str (.name *ns*)) :form s
247-
:clojure.error/phase :execution})
251+
:exception true})
248252
true)))
249253
(catch Throwable ex
250254
(set! *e ex)
251-
(out-fn {:tag :ret :val (Throwable->map ex)
255+
(out-fn {:tag :ret :val (ex->data ex :read-source)
252256
:ns (str (.name *ns*))
253-
:clojure.error/phase :read-source})
257+
:exception true})
254258
true))
255259
(recur)))
256260
(finally
@@ -284,8 +288,8 @@
284288
(try
285289
(assoc m :val (valf (:val m)))
286290
(catch Throwable ex
287-
(assoc m :val (Throwable->map ex)
288-
:clojure.error/phase :print-eval-result)))
291+
(assoc m :val (ex->data ex :print-eval-result)
292+
:exception true)))
289293
m))))))))
290294

291295
(defn remote-prepl
@@ -317,8 +321,8 @@
317321
(try
318322
(assoc m :val (valf val))
319323
(catch Throwable ex
320-
(assoc m :val (Throwable->map ex)
321-
:clojure.error/phase :read-eval-result)))
324+
(assoc m :val (ex->data ex :read-eval-result)
325+
:exception true)))
322326
m))
323327
(recur))))
324328
(finally

‎src/clj/clojure/core_print.clj‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -463,18 +463,27 @@
463463
(print-method [(symbol (.getClassName o)) (symbol (.getMethodName o)) (.getFileName o) (.getLineNumber o)] w))
464464

465465
(defn StackTraceElement->vec
466-
"Constructs a data representation for a StackTraceElement"
466+
"Constructs a data representation for a StackTraceElement: [class method file line]"
467467
{:added "1.9"}
468468
[^StackTraceElement o]
469469
[(symbol (.getClassName o)) (symbol (.getMethodName o)) (.getFileName o) (.getLineNumber o)])
470470

471471
(defn Throwable->map
472-
"Constructs a data representation for a Throwable."
472+
"Constructs a data representation for a Throwable with keys:
473+
:cause - root cause message
474+
:phase - error phase
475+
:via - cause chain, with cause keys:
476+
:type - exception class symbol
477+
:message - exception message
478+
:data - ex-data
479+
:at - top stack element
480+
:trace - root cause stack elements"
473481
{:added "1.7"}
474482
[^Throwable o]
475483
(let [base (fn [^Throwable t]
476-
(merge {:type (symbol (.getName (class t)))
477-
:message (.getLocalizedMessage t)}
484+
(merge {:type (symbol (.getName (class t)))}
485+
(when-let [msg (.getLocalizedMessage t)]
486+
{:message msg})
478487
(when-let [ed (ex-data t)]
479488
{:data ed})
480489
(let [st (.getStackTrace t)]
@@ -484,15 +493,16 @@
484493
(if t
485494
(recur (conj via t) (.getCause t))
486495
via))
487-
^Throwable root (peek via)
488-
m {:cause (.getLocalizedMessage root)
489-
:via (vec (map base via))
490-
:trace (vec (map StackTraceElement->vec
491-
(.getStackTrace ^Throwable (or root o))))}
492-
data (ex-data root)]
493-
(if data
494-
(assoc m :data data)
495-
m)))
496+
^Throwable root (peek via)]
497+
(merge {:via (vec (map base via))
498+
:trace (vec (map StackTraceElement->vec
499+
(.getStackTrace ^Throwable (or root o))))}
500+
(when-let [root-msg (.getLocalizedMessage root)]
501+
{:cause root-msg})
502+
(when-let [data (ex-data root)]
503+
{:data data})
504+
(when-let [phase (-> o ex-data :clojure.error/phase)]
505+
{:phase phase}))))
496506

497507
(defn- print-throwable [^Throwable o ^Writer w]
498508
(.write w "#error {\n :cause ")

‎src/clj/clojure/main.clj‎

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"clojure.string" "clojure.template" "clojure.uuid" "clojure.walk" "clojure.xml" "clojure.zip"})
5353

5454
(defn- core-class?
55-
[class-name]
55+
[^String class-name]
5656
(and (not (nil? class-name))
5757
(or (.startsWith class-name "clojure.lang.")
5858
(contains? core-namespaces (second (re-find #"^([^$]+)\$" class-name))))))
@@ -184,7 +184,9 @@
184184
"Returns an analysis of the phase, error, cause, and location of an error that occurred
185185
based on Throwable data, as returned by Throwable->map. All attributes other than phase
186186
are optional:
187-
:clojure.error/phase - keyword phase indicator
187+
:clojure.error/phase - keyword phase indicator, one of:
188+
:read-source :compile-syntax-check :compilation :macro-syntax-check :macroexpansion
189+
:execution :read-eval-result :print-eval-result
188190
:clojure.error/source - file name (no path)
189191
:clojure.error/line - integer line number
190192
:clojure.error/column - integer column number
@@ -194,39 +196,47 @@
194196
:clojure.error/spec - explain-data for spec error"
195197
{:added "1.10"}
196198
[datafied-throwable]
197-
(let [{:keys [via trace]} datafied-throwable
199+
(let [{:keys [via trace phase] :or {phase :execution}} datafied-throwable
198200
{:keys [type message data]} (last via)
199201
{:clojure.spec.alpha/keys [problems fn], :clojure.spec.test.alpha/keys [caller]} data
200202
{:clojure.error/keys [source] :as top-data} (:data (first via))]
201-
(case (:clojure.error/phase top-data)
202-
(:read-source :compile-syntax-check :compilation :macro-syntax-check :macroexpansion)
203-
(cond-> top-data
204-
source (assoc :clojure.error/source (file-name source))
205-
(#{"NO_SOURCE_FILE" "NO_SOURCE_PATH"} source) (dissoc :clojure.error/source)
206-
type (assoc :clojure.error/class type)
207-
message (assoc :clojure.error/cause message)
208-
problems (assoc :clojure.error/spec data))
209-
210-
:print-eval-result
211-
(let [[source method file line] (-> trace first)]
203+
(assoc
204+
(case phase
205+
:read-source
206+
(let [{:clojure.error/keys [line column]} data]
207+
(cond-> (merge (-> via second :data) top-data)
208+
source (assoc :clojure.error/source (file-name source))
209+
(#{"NO_SOURCE_FILE" "NO_SOURCE_PATH"} source) (dissoc :clojure.error/source)
210+
message (assoc :clojure.error/cause message)))
211+
212+
(:compile-syntax-check :compilation :macro-syntax-check :macroexpansion)
212213
(cond-> top-data
213-
line (assoc :clojure.error/line line)
214-
file (assoc :clojure.error/source file)
215-
(and source method) (assoc :clojure.error/symbol (symbol (-> source name) (-> method name demunge)))
214+
source (assoc :clojure.error/source (file-name source))
215+
(#{"NO_SOURCE_FILE" "NO_SOURCE_PATH"} source) (dissoc :clojure.error/source)
216216
type (assoc :clojure.error/class type)
217-
message (assoc :clojure.error/cause message)))
218-
219-
;; execution
220-
(let [[source method file line] (->> trace (drop-while #(core-class? (name (first %)))) first)
221-
file (first (remove #(or (nil? %) (#{"NO_SOURCE_FILE" "NO_SOURCE_PATH"} %)) [(:file caller) file]))
222-
err-line (or (:line caller) line)]
223-
(cond-> {:clojure.error/phase :execution
224-
:clojure.error/class type}
225-
err-line (assoc :clojure.error/line err-line)
226217
message (assoc :clojure.error/cause message)
227-
(or fn (and source method)) (assoc :clojure.error/symbol (or fn (symbol (-> source name) (-> method name demunge))))
228-
file (assoc :clojure.error/source file)
229-
problems (assoc :clojure.error/spec data))))))
218+
problems (assoc :clojure.error/spec data))
219+
220+
(:read-eval-result :print-eval-result)
221+
(let [[source method file line] (-> trace first)]
222+
(cond-> top-data
223+
line (assoc :clojure.error/line line)
224+
file (assoc :clojure.error/source file)
225+
(and source method) (assoc :clojure.error/symbol (symbol (-> source name) (-> method name demunge)))
226+
type (assoc :clojure.error/class type)
227+
message (assoc :clojure.error/cause message)))
228+
229+
:execution
230+
(let [[source method file line] (->> trace (drop-while #(core-class? (name (first %)))) first)
231+
file (first (remove #(or (nil? %) (#{"NO_SOURCE_FILE" "NO_SOURCE_PATH"} %)) [(:file caller) file]))
232+
err-line (or (:line caller) line)]
233+
(cond-> {:clojure.error/class type}
234+
err-line (assoc :clojure.error/line err-line)
235+
message (assoc :clojure.error/cause message)
236+
(or fn (and source method)) (assoc :clojure.error/symbol (or fn (symbol (-> source name) (-> method name demunge))))
237+
file (assoc :clojure.error/source file)
238+
problems (assoc :clojure.error/spec data))))
239+
:clojure.error/phase phase)))
230240

231241
(defn ex-str
232242
"Returns a string from exception data, as produced by ex-triage.
@@ -273,6 +283,9 @@
273283
loc
274284
cause)
275285

286+
:read-eval-result
287+
(format "Error reading eval result (%s) at %s (%s).%n%s%n" simple-class symbol loc cause)
288+
276289
:print-eval-result
277290
(format "Error printing return value (%s) at %s (%s).%n%s%n" simple-class symbol loc cause)
278291

@@ -380,12 +393,7 @@ by default when a new command-line REPL is started."} repl-requires
380393
input (try
381394
(with-read-known (read request-prompt request-exit))
382395
(catch LispReader$ReaderException e
383-
(throw (ex-info
384-
(str "Syntax error reading source at (" (.-line e) ":" (.-column e) ")")
385-
{:clojure.error/phase :read-source
386-
:clojure.error/line (.-line e)
387-
:clojure.error/column (.-column e)}
388-
e))))]
396+
(throw (ex-info nil {:clojure.error/phase :read-source} e))))]
389397
(or (#{request-prompt request-exit} input)
390398
(let [value (binding [*read-eval* read-eval] (eval input))]
391399
(set! *3 *2)
@@ -394,9 +402,7 @@ by default when a new command-line REPL is started."} repl-requires
394402
(try
395403
(print value)
396404
(catch Throwable e
397-
(throw (ex-info (format "Error printing return value. Cause: " (.getMessage e))
398-
{:clojure.error/phase :print-eval-result}
399-
e)))))))
405+
(throw (ex-info nil {:clojure.error/phase :print-eval-result} e)))))))
400406
(catch Throwable e
401407
(caught e)
402408
(set! *e e))))]

‎src/jvm/clojure/lang/LispReader.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,24 @@ static void unread(PushbackReader r, int ch) {
143143
}
144144
}
145145

146-
public static class ReaderException extends RuntimeException{
146+
public static class ReaderException extends RuntimeException implements IExceptionInfo{
147147
public final int line;
148148
public final int column;
149+
public final Object data;
150+
151+
final static public String ERR_NS = "clojure.error";
152+
final static public Keyword ERR_LINE = Keyword.intern(ERR_NS, "line");
153+
final static public Keyword ERR_COLUMN = Keyword.intern(ERR_NS, "column");
149154

150155
public ReaderException(int line, int column, Throwable cause){
151156
super(cause);
152157
this.line = line;
153158
this.column = column;
159+
this.data = RT.map(ERR_LINE, line, ERR_COLUMN, column);
160+
}
161+
162+
public IPersistentMap getData(){
163+
return (IPersistentMap)data;
154164
}
155165
}
156166

0 commit comments

Comments
 (0)