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

Report problems of type java.lang.Error back to the nREPL client #185

Merged
merged 1 commit into from
Jan 22, 2017

Conversation

plexus
Copy link
Contributor

@plexus plexus commented Jan 22, 2017

Error inherits directly from Throwable, so Errors are not Exceptions, they need
to be handled separately. This category includes RuntimeError,
StackOverflowError, and NoClassDefFoundError.

In regular applications you would not catch these, but in our case letting them
bubble means the nREPL handler dies without returning a response, causing the
client to assume that the operation is taking too long and time out, without any
useful feedback.

This is bad UX, and it makes it hard to diagnose and debug problems like #184.
At least this way the user will see a stack trace that hints at the problem.

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

  • All tests are passing (run lein do clean, test)
  • Code inlining with mranderson works and tests pass with inlined code (run ./build.sh install -- takes a long time)
  • You've updated the changelog (if adding/changing user-visible functionality)

Thanks!

Error inherits directly from Throwable, so Errors are not Exceptions, they need
to be handled separately. This category includes RuntimeError,
StackOverflowError, and NoClassDefFoundError.

In regular applications you would not catch these, but in our case letting them
bubble means the nREPL handler dies without returning a response, causing the
client to assume that the operation is taking too long and time out, without any
useful feedback.

This is bad UX, and it makes it hard to diagnose and debug problems like clojure-emacs#184.
At least this way the user will see a stack trace that hints at the problem.
@expez expez merged commit df24f30 into clojure-emacs:master Jan 22, 2017
@expez
Copy link
Member

expez commented Jan 22, 2017

Sweet, 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.

2 participants