-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add #exc reader to break on exception #769
Conversation
I've been using this a bit now and it seems to be working fine as a quick and easy way to inspect local variables and eval in repl at point where exception is thrown. For completeness the only other change I made was on the cider side, in
to
|
Sorry about the slow response. The idea looks appealing to me, so let's polish it a bit and ship it! |
Lisp is truly special indeed! I took a closer look at the code and it definitely looks good. Seems most professional devs are hesitant to touch the debugger code themselves, so you're a real hero in my book. :-) |
@bo-tato ping :-) |
thanks :) sorry I got busy with work again and haven't had much time for learning and experimenting with clojure. One thing to polish is deciding how exactly it should behave after stopping at exception. One possibility is to have three options, quit, continue, and inject. Inject continues with the value you injected as the return value of the expression that failed with exception. Continue will rerun the expression (presumably you've edited and reloaded some function so it will work now instead of throwing exception again), and quit is self-explanatory. I'm not sure if this is worth it, cause continue is only useful if you can actually edit and reload code, which is hard at present as cider debug keybindings conflict with trying to edit. The way I was able to edit was by using clone-indirect-buffer before starting the debugger, then start the debugger in one window and can edit and reload code in the other, but you have to do that before starting the debugger. Also this would have to be well documented as it could be confusing and have undesired behaviour especially if the expression has side effects, people probably don't expect continue to rerun code that has already run. And when trying to implement this I got errors as I don't understand exactly how instrumentation works on loop/recur forms. So the way I implemented it now is on continue it just tries to rerun once and if that throws an exception again it just crashes with the exception. Or at least that's how I thought it behaved, when I tried just now it seems it actually catches the exception twice before crashing. Which also gives the buggy behaviour that if you do quit it catches the exception "Cannot invoke java.lang.Thread.stop()" and you have to quit again to actually quit. So maybe the simplest choice is to simply break on exception and have the chance to inspect locals or eval code in that context, and then just exit, without the option to inject or continue. I'm not sure if inject and continue are actually useful in practice, a workflow I liked more was combining this with scope-capture and scope-capture-nrepl. I bound one key to:
one key to:
and one key to:
That way when it breaks at exception and it's some complex case I want to edit and re-eval stuff in that context, I can save the local variables with spy, exit the debugger and then use |
I'm fine with making this max simple. Something is infinitely better than nothing anyways. :-) |
Just tried this out and it works wonderfully :) One small issue - at the moment this does not catch assertions, since the (defn g [y]
{:pre [(even? y)]}
(+ 2 y))
#dbgexn
(defn f
[x]
(inc (g x)))
(f 1) ; <- throws AssertionError The wrapping form should probably catch It might even be helpful to implement in the future a debugging mode which automatically wraps all assertions in these |
`(try ~form | ||
(catch Exception ex# | ||
(let [exn-message# (.getMessage ex#) | ||
break-result# (expand-break exn-message# ~dbg-state ~original-form)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be nice if we could overload the s
->:stacktrace
and p
->:inspect
commands here,
using Cider's stacktrace analyser to make better sense of the stacktrace, or the inspector to traverse the ex-info map.
Currently the result being returned here is only exn-message#
, so calling the :inspect command simply inspects the error string.
Maybe a better method is to return the exception object ex#
itself? On the client end we could process the response to pretty-print the message alone as an overlay. Inspecting would then work automatically, and we could add an condition in debug-stacktrace
to analyse the current value if (instance? Throwable value)
agreed :) better just make something basic and working and then others or me in the future can improve it. I kept the option of injecting a value as that seems pretty straightforward without problems, but for continue I just made it rethrow the exception. thanks for feedback @yuhan0 ! I changed it to catch Throwable,
It displays as all one line in the overlay, and the inspector isn't pretty either:
sounds like a good idea |
my clojure-lsp makes some spaces different than the clj-fmt that ci wants
I opened a parallel pull request clojure-emacs/cider#3337 |
So I tried returning the exception and parsing it client-side, which allows for using an appropriate Here's the modified code in cider-debug (defun cider--debug-parse-error (value)
(when (string-match "#error {" value)
(with-temp-buffer
(insert value)
(goto-char (point-min))
(search-forward-regexp ":cause \"" nil 'noerror)
(read (thing-at-point 'string)))))
(defun cider--debug-display-result-overlay (value)
"Place an overlay at point displaying VALUE."
(when cider-debug-use-overlays
;; This is cosmetic, let's ensure it doesn't break the session no matter what.
(ignore-errors
;; Result
(let ((err (cider--debug-parse-error value)))
(cider--make-result-overlay (or err (cider-font-lock-as-clojure value))
:where (point-marker)
:type 'debug-result
:prepend-face (if err 'cider-error-overlay-face
'cider-result-overlay-face)
'before-string cider--fringe-arrow-string))
;; Code
(cider--make-overlay (save-excursion (clojure-backward-logical-sexp 1) (point))
(point) 'debug-code
'face 'cider-debug-code-overlay-face
;; Higher priority than `show-paren'.
'priority 2000)))) A side effect of this is any exception objects being debugged normally will also be displayed in this manner, and also I believe the |
I agree that using (defn foo [n]
#exn (inc n))
(try
(foo nil)
(catch Exception e
,,,)) |
nice! now that it is returning the full exception, maybe you can also open the cider-error buffer showing it? so then we wouldn't need to rely on continue and rethrowing it to see it in the cider-error buffer which won't work as you say in cases where the code catches it higher up |
I've made a few commits on top of this PR in https://github.com/yuhan0/cider-nrepl/tree/debug-exn, with this you can press "s" to bring up the stacktrace viewer, and it adds a Just to bikeshed a little on the naming - how about Or they could be metadata keys on top of the existing reader macros, maybe #break ^:exn (...)
#dbg ^{:break/when (pred ...) :on/ex true} (...) I imagine it would combine disjunctively with the existing |
I like this idea. Seems more "user-friendly" to me. At any rate - I'll merge the PR in the current state so @yuhan0 can open a follow-up PR with his improvements. |
I added malabarba's idea for simple break on exception from here clojure-emacs/cider#2754 (comment)
Basically when you run something and get an exception, you can click in the error buffer the location in your source closest to the top of the stacktrace, it takes you to the source location, you add a
![screenshot](https://user-images.githubusercontent.com/122528427/218280543-27eac0d9-7f6b-4c6d-af3e-f038c3dd73c3.png)
#exc
before the form, and rerun whatever caused the exception, now the debugger will break there showing the exception and you can inspect local variables, eval stuff in the local context etc.Of course this is basic and limited, as noted in that issue:
this is just a convenience macro for manually wrapping in try form and setting breakpoint on except, but I think the little extra convenience makes it more pleasant. If there's interest in merging this then I'll do the rest to make this a proper pull request (adding tests and documentation)