Missing features in nREPL

[tonsky]

Currently nREPL middlewares allow extending nREPL to match client needs. But this approach hurts interoperability between different clients, different servers implementations and different Clojure runtimes.

What features are currently missing from nREPL base protocol that would make using vanilla nREPL with no extensions enough for most clients?

I suggest comment per feature structure, with discussion in replies to each comment.

[tonsky]

Stacktraces

Current nREPL only communicates error message, but not its stacktrace. Would be great to have platform-independent stacktrace representation as a data structure.

Another ideas:

• Drop nREPL frames when detected
• Unmunge Clojure names

[jackrusher]

👍🏻 I feel that there should be an optional key in the eval response containing the stacktrace on systems that produce them.

[bbatsov]

That'd be fine by me. I think the trickiest part would be to agree on the platform-independent format, the rest is fairly easy and we've done a lot on the subject (analyzing and filtering stacktraces) already in cider-nrepl. See https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/stacktrace.clj and https://docs.cider.mx/cider/0.23/usage/navigating_stacktraces.html

[tonsky]

Parallel eval

A way to eval next form while previous one is still executing (e.g. long-running process). I think protocol has everything for that, but implementation does not allow it.

[bbatsov]

I have to admit I'm not sure why Chas opted to serialize the eval operations in each session. Perhaps that's described somewhere in the original brainstorming docs. My best guess is that he wanted to avoid dealing with intermittent results and output. I think it won't be hard to change this in the Clojure nREPL, but I'm also not sure how much end users will gain from it, as I don't recall many people complaining about this limitation over the years.

Here are the original design document (think nREPL 0.0.1) and the list of problems that were discovered with it and (partially) addressed in the current design. They provide some useful historical context.

[jackrusher]

I suspect the reason Chas made that decision is that if the eval requests aren't serialized by default, every client will need to build a queuing mechanism to serialize them. This is also why any move to async eval as default semantics would be bad.

[tonsky]

@jackrusher you’ve already mentioned it, but can you explain why? Like, walk me through use-case?

In my head, you eval something that takes time (e.g. start server - couple of seconds). If you need your server to be up for next eval, you just wait while it finished, then eval next thing. If you want to eval something irrelevant like (+ 1 2) then you don’t wait and just eval in parallel.

This also allow you to eval blocking forms, they will stay pending forever which is ok? E.g. blocking server start. Then continue to work as usual.

What I’m curious is in which situation you’ll need a queueing mechanism?

[jackrusher]

To get the kind of async evaluation you're describing -- parallel evaluation -- you'll need to start a thread for each form that gets sent. This is bad because:

1. it changes the semantics of a number of functions when they're run in different threads, in ways almost certain to be surprising to users
2. it makes the order in which evaluations complete non-deterministic, e.g.

(def a (func-that-takes-400-msecs))

(def b (func-that-takes-400-msecs))

... becomes a race condition whose outcome is determined by the default thread execution pool. The user needing to babysit each evaluation manually to avoid this is a strictly worse experience than just evaluating them in order, which is why clients would need to use a queue to linearize execution to get behavior that doesn't feel like buggy weirdness.

Although I'm quite sympathetic to the "go do this in the background" use case, I feel that it should be an explicit thing so the user knows what they're getting, thread-wise.

[tonsky]

Report eval execution time

Relatively minor, but trivial, too. How long did the command took to execute? Very useful during interactive development.

E.g. :time-ms

[bbatsov]

That should be fairly easy to do. Not sure if we need this for all ops or just for eval, though.

[bbatsov]

To clarify - if we do this for everything it'd would make sense (at the implementation level in the Clojure nREPL at least) to make this some middleware that intercepts and times each request. Otherwise that'd be an isolated change to eval.

[tonsky]

Option to auto-close sessions

A way to specify “close this session as soon as connection is broken”. Because otherwise you might not have a change to close it properly and it will leak resources staying open forever.

:close-on-disconnect true | false?

[bbatsov]

I love this idea. I'm guessing this can be even the default behavior, as I don't see what harm can come out of it.

[jackrusher]

This is what my server already does, so of course I support it 😹

[tonsky]

This https://github.com/clojure/tools.nrepl/wiki/nREPL.Next#sessions-are-tied-to-connections has an explanation for sessions that persist between connections. Looks like auto-closing was the default but they removed it to allow clients that can’t maintain a persistent connection.

Looks like nothing stops up to re-introduce it as an option

[tonsky]

Do not capture *out*

Right now, everything your eval prints with pr/prn/print/println will be sent to you over the network. I want an option to disable this behavior and fall back to the default one: print to the original stdout, the one the java prints to (System.out).

Motivation: for nREPL clients with inline eval only, there’s no place to display async stdout messages. But because nREPL captures them, they are not visible in normal System.out output too.

Could be specified on session creation, e.g. :capture-out? true | false

[jackrusher]

@imrekoszo is correct. To expand: the REPL is an artifact of an interpreter written to work with a c. 1960 teletype. What's needed to support a modern IDE-like experience is a bit stronger than that, thus being able to send off an eval request while still being able to get completions and arglists, for example.

[jackrusher]

@bbatsov 💯 support being careful about breaking everyone's environment

[cursive-ide]

@jackrusher Sure, I get the eval server idea (in fact, it was me arguing with Rich in that thread). But surely the in/out part of the nREPL spec is required if you're connecting to a remote machine and want to see the output of what you're evaluating?

[jackrusher]

I mean...
https://clojuredocs.org/clojure.core/with-out-str

[jackrusher]

(Less flippantly, your functions can return the values you care about. You don't need a stdout sideband.)

[tonsky]

Auto-require namespace if not loaded

Might seem controversial, but when I send

{"op":   "eval",
 "code": "(reverse \"abc\")",
 "ns":   "my-ns"}

this code breaks if my-ns is not loaded. This could be addressed by loading namespace then sending it again (in other words, in client), but maybe we could have an option that checks if ns is loaded in the server? Then it would be a single operation always.

E.g. require-ns-not-loaded? true | false

[bbatsov]

We also have to consider here that the ns form has changed (e.g. you added some new requires there) and your eval will explode because you forgot to eval it. That's obviously solvable but would require tracking last evaled ns form and comparing it with whatever is currently in the source file. (That's how I solved this in CIDER at least) My main concern here is that for this to work we'll have to make the server interact with the source files, which adds some degree of complexity.

[jackrusher]

We cannot assume the server has access to the source files. I would much rather the client handle this.

[cursive-ide]

I agree that the client should handle this.

[tonsky]

I agree with all, this might be too brittle

[cursive-ide]

Better specify completion information

Currently the doc states:

Returns: :completions A list of completion candidates. Each candidate is a map with :candidate and :type keys. Vars also have a :ns key.

This is pretty minimal information. The doc makes some vague references to possible extra metadata, but doesn't specify them at all. There's lots of extra info that would be useful to put in completions:

1. For vars, in addition to the ns, the arglists if the var contains a function, as well as e.g. type hints for returns and parameters.
2. For JVM methods, parameter types, return type, containing class.
3. For JVM fields, type and containing class.
4. For aliases, which ns is aliased.
5. For JS functions, containing object, perhaps containing package or library, arglists and type info if available.
6. and so on.

I'm not sure if we would want to specify ahead of time all the types that might be returned and the specific information for each one - I'm open to suggestions on that. But I'd like to see a lot more information returned in general.

[bbatsov]

Yep, I totally agree that should be specified better, but it's also a bit tricky as we have to separate what's universal (goes to the protocol requirements) from what's Clojure-specific (goes beyond the protocol and is needed only for Clojure nREPL implementations).

[jackrusher]

Totally agree about the spec. It was vague enough that I had to observe what other servers did to get a sense of what I should do.

[jackrusher]

A small thing that I would like to see is the addition of an optional key/value pair in the eval response for mimetype. This could be safely left out by servers and/or ignored by clients, but would make it easier to build richer experiences on the client side when supported.

[bbatsov]

Something that emulates cider-nrepl's content-type middleware? (see https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/content_type.clj)

[jackrusher]

Same intention, different mechanism that leaves implementation decisions open to the clients.

I think there are a fair few things like this that date to a time when JVM clojure + emacs was really the only client/server pair. Now that we have a bunch of different clients and a growing list of non-JVM server types, it would be nice to avoid relying on middleware for these extension points.

[bbatsov]

No argument from me. I was just trying to understand how you envision the mimetype support in general.

[yuhan0]

Should the Nrepl protocol have some concept of "tagged literals / collections" in order to represent values in the source language that don't have an equivalent in the client or message transporting protocol?

This comes from some sort of impedance mismatch I've noticed recently, between Cider's representation of Clojure values in Emacs lisp and their lossy encoding over the Nrepl protocol.

The most glaring example I see is the fact that you can't encode true or false in a message.
Eg. the cider-nrepl format-code op takes an optional map to pass to cljfmt, but there's no way to encode the config map {:indentation? false}, because of Elisp nil being equivalent to an empty list and Bencode having no concept of a Boolean type.

[yuhan0]

Apologies if this isn't the right place to post it, I was thinking of creating an issue in Cider/ cider-nrepl but it seemed like a more fundamental problem.
One could get around it by having all values sent over the wire as strings, just as it's done with the eval op, but that would require the additional load of writing and parsing on either end

[bbatsov]

That's a problem I've been thinking for a while in the context of CIDER, and it certainly affects to some extent all nREPL clients, so I think that's the right place to discuss it.

[bbatsov]

I recall there were many discussions over the years, but all I could find quickly were those tickets:

• Improve handling of boolean values in responses #196
• [nrepl-client] Automatically disjoin false/nil keys from requests clojure-emacs/cider#2854
• Document the best practices for middleware development #144 (comment)