Show stacktrace buffers for sync requests

[bbatsov]

Right now when something goes wrong in nrepl-send-sync-request we just dump the stacktrace in the REPL buffer and switch to it. Obviously, this is less than ideal. The reason why we're not showing our custom pretty stacktraces is that for cider middleware requests (which are generally done synchronously) *e doesn't get bound to the last exception and our stacktrace printing middleware doesn't work.

[bbatsov]

Just to add a bit more details here. I think the root cause of the issue is that we decided not to associate cider's custom middleware requests with a session. As I said I'm contemplating what's the best course of action and I definitely wouldn't mind some suggestions. @cemerick What would you recommend us to do?

[Malabarba]

Could we write a middleware that wraps around all others and sets *e when appropriate?

[bbatsov]

Could we write a middleware that wraps around all others and sets *e when appropriate?

This idea also came to my mind, but this would also require changes to the way we request the stacktrace from the Elisp code. And I was also wondering if we might need more such data in the future. So maybe it makes sense to leverage the tooling session with the middleware... Anyways, if I had really great ideas - I would have implemented something by now. I'm definitely open to any suggestion/idea whatever.

P.S. If we do some wrapper middleware it'd be nice if it did exception handling in general, so we'd avoid deadlocks on unhandled exceptions. Something like this would eliminate the need for the current notion of "sync timeout".

[bbatsov]

@Malabarba Another relatively simple idea would be require the session middleware in all the middleware we have and to set some different var (e.g. *e-middleware*) there, so we won't clobber the evaluation exceptions. Then the stacktrace requests will be checking for both.

[bbatsov]

@jeffvalk You might take a look at this, if you'd like.

[cichli]

I think the ideal solution to this would be one that doesn't rely on checking *e at all - since it could be altered from underneath us by other evaluations, and we obviously don't want to be altering it ourselves.

If we had a way of getting nREPL to convey exception values to our middleware, it could then work generically for either exceptions thrown by an eval or exceptions thrown by other middlewares. I'll open a JIRA with a proposed solution and link to it here :-).

[jeffvalk]

Haven't looked at this closely, but at one point I did bake up a middleware that wraps eval and returns errors and compiler warnings as data. If that might be useful, I'll resurrect it.

[jeffvalk]

Also, does anyone have a code example to illustrate the problem? If I just send nrepl-send-sync-request something that produces an error, like:

(nrepl-send-sync-request (list "op" "eval" "code" "(+ nil 1)")
                         (cider-current-connection))

the middleware seems to work fine.

[bbatsov]

The problem is an error that occurs in a non-eval operation - e.g. macroexpansion.

[bbatsov]

Try the macroexpand command on this (| indicates your cursor position):

(defmacro naughty-macro []
  (throw (ex-info "BANG!" {})))

(naughty-macro)|

[cichli]

Try something like:

(cider-macroexpand-expr "foo" nil)

edit: @bbatsov is too fast for me :-).

[jeffvalk]

Got it. Thanks.

[jeffvalk]

Is macroexpansion a special case? Or are there other middleware components that cause this?

Macroexpansion obviously has to handle arbitrary (possibly invalid) user code like eval. But other than these two, middleware should deal with reasonably defined inputs, I think. If so, adding session support for grabbing *e from macroexpansion seems the reasonable thing here.

Thoughts?

[bbatsov]

All non-eval middleware (e.g. completion, var-info, etc) we have suffer from this, so this should be added to all of them. Seems that the current state of things is just an oversight of ours - we didn't use the session in most middleware and this is the biggest problem...

[jeffvalk]

I see your point. But practically speaking, how would var-info or completion require presenting a stacktrace to a user? An unhandled error there is a bug in CIDER, isn't it?

[bbatsov]

Well, it might a bug and it might be some situation that we can't do anything sensible about. At any rate - it'd be better to handle such exceptions the same way we handle eval exceptions. We can really suppress them, as this would make fixing the real issues too hard, and we can't really expect that everything will always work perfectly.

[bbatsov]

Fixed by #1645