Skip to content

Commit cd81dc5

Browse files
ikappakiikappaki
authored
Improve on nrepl exception formating and logging interface (#973)
Hi, could you please consider patch to improve on the nREPL server exception messages, inline with what has been done for the REPL recently. It addresses #968. Tests updated to reflect the new exc messages. I've also in-lined the logger functions to be closer to the actual logging interface, and added a new target in the markefile for easier debugging. Thanks --------- Co-authored-by: ikappaki <ikappaki@users.noreply.github.com>
1 parent c6ce8a1 commit cd81dc5

File tree

4 files changed

+73
-29
lines changed

4 files changed

+73
-29
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

77
## [Unreleased]
8+
### Changed
9+
* Improved on the nREPL server exception messages by matching that of the REPL user friendly format (#968)
10+
811
### Other
912
* Run PyPy CI checks on Github Actions rather than CircleCI (#971)
1013

‎Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ repl:
3232
@BASILISP_USE_DEV_LOGGER=true poetry run basilisp repl
3333

3434

35+
LOGLEVEL ?= INFO
36+
.PHONY: nrepl-server
37+
nrepl-server:
38+
@BASILISP_USE_DEV_LOGGER=true BASILISP_LOGGING_LEVEL=$(LOGLEVEL) poetry run basilisp nrepl-server
39+
3540
.PHONY: test
3641
test:
3742
@rm -f .coverage*

‎src/basilisp/contrib/nrepl_server.lpy

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"A port of `nbb <https://github.com/babashka/nbb>`_ 's nREPL server implementation to Basilisp."
55
(:require [basilisp.contrib.bencode :as bc]
66
[basilisp.string :as str])
7-
(:import logging
7+
(:import basilisp.logconfig
8+
logging
89
socketserver
910
sys
1011
traceback
@@ -14,18 +15,21 @@
1415
"The logger for this namespace."
1516
(logging/getLogger (namespace ::)))
1617

18+
(defmacro ^:private trace [& values]
19+
`(when (.isEnabledFor logger basilisp.logconfig/TRACE)
20+
(.log logger basilisp.logconfig/TRACE (str/join " " [~@values]))))
1721
(defmacro ^:private debug [& values]
18-
(when (.isEnabledFor logger logging/DEBUG)
19-
`(.debug logger (str/join " " [~@values]))))
22+
`(when (.isEnabledFor logger logging/DEBUG)
23+
(.log logger logging/DEBUG (str/join " " [~@values]))))
2024
(defmacro ^:private info [& values]
21-
(when (.isEnabledFor logger logging/INFO)
22-
`(.info logger (str/join " " [~@values]))))
25+
`(when (.isEnabledFor logger logging/INFO)
26+
(.log logger logging/INFO (str/join " " [~@values]))))
2327
(defmacro ^:private warn [& values]
24-
(when (.isEnabledFor logger logging/WARNING))
25-
`(.warning logger (str/join " " [~@values])))
28+
`(when (.isEnabledFor logger logging/WARNING)
29+
(.log logger logging/WARNING (str/join " " [~@values]))))
2630
(defmacro ^:private error [& values]
27-
;; we want error to always output to `sys/stderr` unfiltered.
28-
`(.write sys/stderr (str ~(keyword (str *ns*) "error") " " (str/join " " [~@values]) \newline)))
31+
`(when (.isEnabledFor logger logging/ERROR)
32+
(.log logger logging/ERROR (str/join " " [~@values]))))
2933

3034
(definterface ^:private IStdOut
3135
;; Pythonic interface for creating `sys/stdout` like File objects.
@@ -55,12 +59,12 @@
5559

5660
(defn- log-request-mw [handler]
5761
(fn [request send-fn]
58-
(info :request (dissoc request :client*))
62+
(debug :request (dissoc request :client*))
5963
(handler request send-fn)))
6064

6165
(defn- log-response-mw [handler]
6266
(fn [request response]
63-
(info :response response)
67+
(debug :response response)
6468
(handler request response)))
6569

6670
(declare ops)
@@ -139,8 +143,10 @@
139143
result (last results)]
140144
(send-value request send-fn [result {:ns (str *ns*)}]))
141145
(catch python/Exception e
142-
(info :eval-exception e)
143-
(handle-error send-fn (assoc request :ns (str *ns*)) e))
146+
(debug :eval-exception e)
147+
(let [msg (->> (basilisp.lang.exception/format_exception e (type e) (.-__traceback__ e))
148+
(str/join ""))]
149+
(handle-error send-fn (assoc request :ns (str *ns*)) msg)))
144150
(finally
145151
(swap! client* assoc :eval-ns *ns*)
146152
(send-fn request {"ns" (str *ns*)
@@ -181,7 +187,7 @@
181187
*resolver*
182188
*data-readers*))))}
183189
(catch python/Exception e
184-
(info :symbol-identify-reader-error :input symbol-str :exception e)
190+
(debug :symbol-identify-reader-error :input symbol-str :exception e)
185191
{:error (str e)}))]
186192

187193
(cond
@@ -253,7 +259,7 @@
253259
status (if (and (nil? symname) (= mapping-type :eldoc) )
254260
["done" "no-eldoc"]
255261
["done"])]
256-
(debug :lookup :sym sym-str :doc doc :args arglists)
262+
(trace :lookup :sym sym-str :doc doc :args arglists)
257263
(send-fn request (assoc response :status status)))
258264
(catch python/Exception e
259265
(let [status (cond->
@@ -338,7 +344,7 @@
338344

339345
(defn- make-send-fn [socket]
340346
(fn [_request response]
341-
(debug :sending (:id _request) :response-keys (keys response))
347+
(trace :sending (:id _request) :response-keys (keys response))
342348
(try
343349
(.sendall socket (bc/encode response))
344350
(catch python/TypeError e
@@ -388,7 +394,7 @@
388394
data)
389395
[requests unprocessed] (bc/decode-all data {:keywordize-keys true
390396
:string-fn #(.decode % "utf-8")})]
391-
(debug :requests requests)
397+
(trace :requests requests)
392398
(when (not (str/blank? unprocessed))
393399
(reset! pending unprocessed))
394400
(doseq [request requests]

‎tests/basilisp/contrib/nrepl_server_test.lpy

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,9 @@
325325
{:id @id* :ns "xyz" :status ["done"]})
326326

327327
(client-send! client {:id (id-inc!) :op "eval" :code "(/ 3 0)"})
328-
(is (= {:id @id* :err "ZeroDivisionError('Fraction(3, 0)')"} (client-recv! client)))
328+
(let [{:keys [id err]} (client-recv! client)]
329+
(is (= id @id* ))
330+
(is (str/includes? err "ZeroDivisionError: Fraction(3, 0)")))
329331
(let [{:keys [id ex status ns]} (client-recv! client)]
330332
(is (= @id* id))
331333
(is (= "xyz" ns))
@@ -337,8 +339,10 @@
337339
(client-send! client {:id (id-inc!) :op "eval" :code "(println :hey)\n(/ 4 0)"})
338340
(are [response] (= response (client-recv! client))
339341
{:id @id* :out ":hey"}
340-
{:id @id* :out os/linesep}
341-
{:id @id* :err "ZeroDivisionError('Fraction(4, 0)')"})
342+
{:id @id* :out os/linesep})
343+
(let [{:keys [id err]} (client-recv! client)]
344+
(is (= @id* id))
345+
(is (str/includes? err "ZeroDivisionError: Fraction(4, 0)")))
342346
(let [{:keys [id ex status ns]} (client-recv! client)]
343347
(is (= @id* id))
344348
(is (= "xyz" ns))
@@ -347,28 +351,38 @@
347351
(are [response] (= response (client-recv! client))
348352
{:id @id* :ns "xyz" :status ["done"]})
349353

350-
(client-send! client {:id (id-inc!) :op "eval" :code "[*1 *2 *3 *e]"})
354+
(client-send! client {:id (id-inc!) :op "eval" :code "[*1 *2 *3]"})
351355
(are [response] (= response (client-recv! client))
352-
{:id @id* :ns "xyz" :value "[6 nil 4 ZeroDivisionError('Fraction(4, 0)')]"}
356+
{:id @id* :ns "xyz" :value "[6 nil 4]"}
353357
{:id @id* :ns "xyz" :status ["done"]})
354358

359+
(client-send! client {:id (id-inc!) :op "eval" :code "*e"})
360+
(let [{:keys [id value]} (client-recv! client)]
361+
(is (= @id* id))
362+
(is (str/includes? value "ZeroDivisionError: Fraction(4, 0)")))
363+
(is (= {:id @id* :ns "xyz" :status ["done"]} (client-recv! client)))
364+
355365
;; error with :file
356366
(client-send! client {:id (id-inc!) :op "eval" :file "/hey/you.lpy" :code "1\n2\n(/ 5 0)"})
357-
(are [response] (= response (client-recv! client))
358-
{:id @id* :err "ZeroDivisionError('Fraction(5, 0)')"})
367+
(let [{:keys [id err]} (client-recv! client)]
368+
(is (= @id* id))
369+
(is (str/includes? err "ZeroDivisionError: Fraction(5, 0)")))
359370
(let [{:keys [id ex status ns]} (client-recv! client)]
360371
(is (= @id* id))
361372
(is (= "xyz" ns))
362373
(is (= ["eval-error"] status))
363374
(is (not= -1 (.find ex "File \"/hey/you.lpy\", line 3")) ex))
364375
(are [response] (= response (client-recv! client))
365376
{:id @id* :ns "xyz" :status ["done"]})
366-
367377
;; error conditions
368378
(client-send! client {:id (id-inc!) :op "eval" :code "(xyz"})
369379
(let [{:keys [id err]} (client-recv! client)]
370380
(is (= @id* id))
371-
(is (str/starts-with? err "basilisp.lang.reader.SyntaxError")))
381+
(is (= [""
382+
" exception: <class 'basilisp.lang.reader.UnexpectedEOFError'>"
383+
" message: Unexpected EOF in list"
384+
" line: 1:4"]
385+
(str/split-lines err))))
372386
(let [{:keys [id ex status ns]} (client-recv! client)]
373387
(is (= @id* id))
374388
(is (= "xyz" ns))
@@ -378,7 +392,15 @@
378392
{:id @id* :ns "xyz" :status ["done"]})
379393

380394
(client-send! client {:id (id-inc!) :op "eval" :code "(+ 3 5)" :ns "not-there"})
381-
(is (= {:id @id* :err "CompilerException(msg=\"unable to resolve symbol '+' in this context\", phase=<CompilerPhase.ANALYZING: :analyzing>, filename='<nREPL Input>', form=+, lisp_ast=None, py_ast=None)"} (client-recv! client)))
395+
(let [{:keys [id err]} (client-recv! client)]
396+
(is (= id @id*))
397+
(is (= [""
398+
" exception: <class 'basilisp.lang.compiler.exception.CompilerException'>"
399+
" phase: :analyzing"
400+
" message: unable to resolve symbol '+' in this context"
401+
" form: +"
402+
" location: <nREPL Input>:1"]
403+
(str/split-lines err))))
382404
(let [{:keys [id ex status ns]} (client-recv! client)]
383405
(is (= @id* id))
384406
(is (= "not-there" ns))
@@ -402,7 +424,13 @@
402424

403425
;; bad namespace
404426
(client-send! client {:id 3 :op "eval" :code "(+ 3 5)" :ns "#,,"})
405-
(is (= {:id 3 :err "CompilerException(msg=\"unable to resolve symbol '+' in this context\", phase=<CompilerPhase.ANALYZING: :analyzing>, filename='<nREPL Input>', form=+, lisp_ast=None, py_ast=None)"} (client-recv! client)))
427+
(let [{:keys [id err]} (client-recv! client)]
428+
(is (= 3 id))
429+
(is (= [""
430+
" exception: <class 'basilisp.lang.compiler.exception.CompilerException'>"
431+
" phase: :analyzing" " message: unable to resolve symbol '+' in this context"
432+
" form: +" " location: <nREPL Input>:1"]
433+
(str/split-lines err))))
406434
(let [{:keys [id ex status ns]} (client-recv! client)]
407435
(is (= 3 id))
408436
(is (= "#,," ns))
@@ -533,7 +561,9 @@
533561

534562
(client-send! client {:id (id-inc!) :op "load-file" :ns "user" :file "(ns abc.third)\n\n(/ 3 0)"
535563
:file-name "third.lpy" :file-path "/abc/third.lpy"})
536-
(is (= {:id @id* :err "ZeroDivisionError('Fraction(3, 0)')"} (client-recv! client)))
564+
(let [{:keys [id err]} (client-recv! client)]
565+
(is (= @id* id))
566+
(is (str/includes? err "ZeroDivisionError: Fraction(3, 0)") (str/split-lines err)))
537567
(let [{:keys [id ex status ns]} (client-recv! client)]
538568
(is (= @id* id))
539569
(is (= "abc.third" ns))

0 commit comments

Comments
 (0)