Skip to content
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

Display debug-on-exception messages #3366

Merged
merged 3 commits into from
Mar 25, 2024
Merged

Conversation

yuhan0
Copy link
Contributor

@yuhan0 yuhan0 commented Jul 5, 2023

Support for clojure-emacs/cider-nrepl#772

A quick PR without the proper changelogs / updates to manual, etc. Changes untested after being rebased on master branch, but I've been dogfooding them on a messy local branch with many (hopefully unrelated) changes for the last month or two. Might be able to clean things up a bit this weekend.

Does not provide feature for auto-inserting #dbg! macros or instrumenting defns (eg. with negative argument) - see #3337


Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

Thanks!

If you're just starting out to hack on CIDER you might find this section of its
manual
extremely useful.

@bbatsov bbatsov mentioned this pull request Jul 6, 2023
6 tasks
@bbatsov
Copy link
Member

bbatsov commented Jul 11, 2023

many (hopefully unrelated) changes

Looking forward to seeing some of those unrelated changes become PRs as well. ;-)

@@ -154,15 +157,17 @@ configure `cider-debug-prompt' instead."
#("." 0 1 (display (left-fringe right-triangle)))
"Used as an overlay's before-string prop to place a fringe arrow.")

(defun cider--debug-display-result-overlay (value)
(defun cider--debug-display-result-overlay (value caught)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to mention this in the docstring to make checkdoc happy.

@bbatsov
Copy link
Member

bbatsov commented Aug 4, 2023

Might be able to clean things up a bit this weekend.

No rush. CIDER 1.8 is still in development in probably won't be released for a week or two more. If this PR can make there it'd be great, but if not - not a big deal.

@vemv
Copy link
Member

vemv commented Aug 10, 2023

Hey @yuhan0 - do you have the chance to revisit this one?

Else it seems quite minor - could give it a shot myself today/tomorrow.

@yuhan0
Copy link
Contributor Author

yuhan0 commented Aug 13, 2023

Else it seems quite minor - could give it a shot myself today/tomorrow.

@vemv That would be great if you could 🙏
Sorry for the long absence but I've been away from OSS / Clojure-related work for the past month or so.. Glad to see such activity around Cider 1.8 happening in the meantime, but realistically I won't have time to pull these new changes and integrate/test against them in next couple of days.
Feel free to adapt this PR however you like!

@vemv
Copy link
Member

vemv commented Aug 20, 2023

@yuhan0 if you'd like to re-take this PR soon let us know, I plan to work on misc stuff for the following few days

@yuhan0
Copy link
Contributor Author

yuhan0 commented Aug 20, 2023

Thanks - I'm tentatively back from that long-ish absence, currently in the midst of piecing together a broken Emacs config from the 29.1 update 😅

But yes, I'll be able to revisit this PR as soon as I re-familiarize myself with the issue and the recent updates to Cider and friends :)

@vemv
Copy link
Member

vemv commented Aug 20, 2023

Cheers. Much better like that 🍻

@bo-tato
Copy link

bo-tato commented Sep 1, 2023

great to see yuhan0 has put a lot of work on this 😄
I was reading some about jvm and Debug Wire Protocol, and it seems you are able to have it break on uncaught exceptions. The only catch being that if there is native code in the call stack, it will sometimes break on an "uncaught exception" that really will be caught by the native code but the jvm can't know that. There is also the PopFrames command that allows to restart execution from some other frame higher up in the stack. It seems a near Common Lisp level experience is possible with clojure on the JVM, to break on uncaught exceptions and inspect local variables in all stack frames and fix code and then restart from some stack frame.
There is this project that uses jvm debug wire protocol to implement a clojure debugger but it is long since abandoned, and cider and flowstorm debugger took a different approach of instrumenting code, which is great but has limitations that you have to first replicate the issue, and then instrument the code and rerun it, rather than having the debugger already present when you first run into the issue. It seems jdwt has almost no overhead when you don't have breakpoints set, so it could even be possible to leave running in production, and log local variables and function arguments and everything at times of exception to easily reproduce whatever exceptions your application throws in production. It seems there is some commercial product for the jvm that does this but I couldn't find anything open source.
It seems it would be promising to build a clojure debugger on top of jdwp that would allow much better experience regarding breaking on exceptions. Does anyone know why cdt was discontinued and the clojure debuggers that came after took a different approach, are there limitations or difficulties to debugging clojure code with jdwp?

@bbatsov
Copy link
Member

bbatsov commented Sep 1, 2023

@bo-tato I can answer your questions, but I'd suggest to move this to a discussion topic first as I don't think it belongs here.

@bo-tato
Copy link

bo-tato commented Sep 1, 2023

you're right it's getting off topic from the PR, I opened a discussion here

@yuhan0
Copy link
Contributor Author

yuhan0 commented Mar 25, 2024

Sorry for delaying this feature this past 6 months..! I must have completely forgot about this PR 👀

Rebased onto master and the error overlays seem to be working as expected, also fixed the lint warnings from last time.

image

@bbatsov bbatsov merged commit bf26bc6 into clojure-emacs:master Mar 25, 2024
35 checks passed
@bbatsov
Copy link
Member

bbatsov commented Mar 25, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants