compliment can cause cider-nrepl to err out

[expez]

Expected behavior

Errors in compliment only result in missing completions

Actual behavior

Input is interrupted and repl is filled with stacktracke:

java.lang.NullPointerException: null
 at java.util.concurrent.ConcurrentHashMap.get (ConcurrentHashMap.java:936)
    clojure.lang.Namespace.find (Namespace.java:188)
    clojure.core$find_ns.invoke (core.clj:3976)
    clojure.core$the_ns.invoke (core.clj:4008)
    clojure.core$ns_map.invoke (core.clj:4022)
    cider.inlined_deps.compliment.v0v2v7.compliment.sources.ns_mappings$candidates.invoke (ns_mappings.clj:53)
    clojure.lang.Var.invoke (Var.java:388)
    cider.inlined_deps.compliment.v0v2v7.compliment.core$eval52698$completions__52701$iter__52703__52707$fn__52708.invoke (core.clj:95)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.RT.seq (RT.java:507)
    clojure.core/seq (core.clj:137)
    clojure.core$tree_seq$walk__5045$fn__5046.invoke (core.clj:4739)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.LazySeq.more (LazySeq.java:85)
    clojure.lang.RT.more (RT.java:683)
    clojure.core/rest (core.clj:73)
    clojure.core$flatten.invoke (core.clj:6851)
    cider.inlined_deps.compliment.v0v2v7.compliment.core$eval52698$completions__52701.invoke (core.clj:91)
    cider.nrepl.middleware.complete$complete.invoke (complete.clj:19)
    cider.nrepl.middleware.complete$complete_reply.invoke (complete.clj:34)
    cider.nrepl.middleware.complete$wrap_complete$fn__53008.invoke (complete.clj:55)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.ns$wrap_ns$fn__51752.invoke (ns.clj:110)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.trace$wrap_trace$fn__59014.invoke (trace.clj:65)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.macroexpand$wrap_macroexpand$fn__58340.invoke (macroexpand.clj:220)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.apropos$wrap_apropos$fn__51782.invoke (apropos.clj:100)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    refactor_nrepl.middleware$wrap_refactor$fn__68899.invoke (middleware.clj:135)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.classpath$wrap_classpath$fn__51806.invoke (classpath.clj:30)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__40547.invoke (interruptible_eval.clj:247)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.inspect$wrap_inspect$fn__53217.invoke (inspect.clj:108)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.debug$wrap_debug$fn__54863.invoke (debug.clj:461)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    clojure.tools.nrepl.middleware.load_file$wrap_load_file$fn__40667.invoke (load_file.clj:79)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    user$eval72797$fn__72798$fn__72800.invoke (form-init6400695034319109191.clj:1)
    cider.nrepl.middleware.enlighten$wrap_enlighten$fn__54905.invoke (enlighten.clj:86)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    clojure.tools.nrepl.middleware.session$add_stdin$fn__40626.invoke (session.clj:238)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.pprint$wrap_pprint$fn__54554.invoke (pprint.clj:106)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.track_state$wrap_tracker$fn__59277.invoke (track_state.clj:200)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    clojure.tools.nrepl.middleware.pr_values$pr_values$fn__40465.invoke (pr_values.clj:22)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.test$wrap_test$fn__58741.invoke (test.clj:286)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.resource$wrap_resource$fn__58596.invoke (resource.clj:49)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.stacktrace$wrap_stacktrace$fn__54632.invoke (stacktrace.clj:175)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.pprint$wrap_pprint_fn$fn__54528.invoke (pprint.clj:53)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.info$wrap_info$fn__58225.invoke (info.clj:304)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.undef$wrap_undef$fn__59304.invoke (undef.clj:30)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    cider.nrepl.middleware.out$wrap_out$fn__58426.invoke (out.clj:96)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    clojure.tools.nrepl.middleware.session$session$fn__40611.invoke (session.clj:192)
    clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__40282.invoke (middleware.clj:22)
    clojure.tools.nrepl.server$handle_STAR_.invoke (server.clj:19)
    clojure.tools.nrepl.server$handle$fn__40682.invoke (server.clj:28)
    clojure.core$binding_conveyor_fn$fn__4444.invoke (core.clj:1916)
    clojure.lang.AFn.call (AFn.java:18)
    java.util.concurrent.FutureTask.run (FutureTask.java:266)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1142)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:617)
    java.lang.Thread.run (Thread.java:745)

Steps to reproduce the problem

Not sure. Should be trivial to solve with code inspection.

Environment & Version information

cider-nrepl version

Latest snapshot

Java version

1.8

Operating system

OSX, El capitan

[bbatsov]

Seems to me the bug is in compliment, but I also see you've already reported it there.

[expez]

Yes, but we should wrap our call to compliment in a try/catch. Every time I pressed a key, in a context triggering this bug, the repl would be focused and a stacktrace produced. That is not an acceptable failure mode.

[bbatsov]

As I mentioned in the other ticket - ideally compliment should raise some more specific exceptions which we would handle. As it stands this is way too generic.

[expez]

@bbatsov We should fail gracefully imo. Making CIDER completely unusable just because there's a problem with completions seems extreme.

As it is now you can't type when something goes wrong because the focus is changed, from whatever window you're typing in, to the repl on each keystroke.

Can we at least make it so the repl is spammed without changing focus?

[sanjayl]

@expez I was able to replicate the behavior, but I had a very difficult time doing so naturally -- I only finally saw the stack traces when I patched the compliment code itself to throw a null pointer exception. I'm not exactly sure how the nil got into that function since the elisp code always sends a non-nil namespace over (as far as I can see). Would you be able to give me a list of the namespaces you typically use? Perhaps a lein deps :tree? Any unicode characters or anything like that in your namespaces? My best guess is that it's related to a regex, so I needed some test data to throw at the problem.

In the meantime, I'm getting a PR ready that would swallow these types of errors in the completion middleware, returning an empty completion and sending back some error information.

[sanjayl]

oh, and also @expez , could you give me the full string that you get when you do an M-x cider-version?

[expez]

@sanjayl There's no debugging required here, I think. A bug was fixed in compliment and we just need to wrap the call to compliment in a try/catch to avoid making such an inconvenience out of any future problems.

I think we're waiting for @alexander-yakushev to wrap his own code in a try/catch and then rethrow a more specific exception we can depend on, to avoid catching Exception and thus swallowing our own exceptions as well.

Fwiw I don't have any unicode namespaces, it's all ASCII. lein deps :tree will unfortunately list most of clojars and a substantial chunk of maven central, but that's another problem entirely :(

[sanjayl]

@expez , while we're waiting for the relevant patches, I put up a branch on my repo that should swallow the errors you're seeing and prevent any interruption in your workflow until things are ironed out a bit better. You can do a local install with ./build install and it should work the next time you jack-in.

I do still think that there's a need to debug this further. I'm having a tough time wrapping my head around how that nil even got close to the compliment code. I spend a lot of time in CIDER and I haven't run across it yet, so I just know it's going to stick in my craw until I can figure it out. Probably the most helpful thing would be a copy of the relevant communication in the *nrepl-messages cider-nrepl*buffer at the time of an error.

Thanks!

[alexander-yakushev]

I think we're waiting for @alexander-yakushev to wrap his own code in a try/catch and then rethrow a more specific exception we can depend on, to avoid catching Exception and thus swallowing our own exceptions as well.

I don't mind adding a few try-catches, but I really don't understand the logic behind rethrowing exceptions in this case. It's a very Javaesque practice. Why would catch Exception swallow CIDER's own exceptions if it is put exactly around calls to Compliment?

[sanjayl]

@alexander-yakushev , agreed. I think figuring out how this error is happening will have the most utility.

For anyone interested in similar types of issues in CIDER, I think it's important to note that the CIDER code already has a try/catch around the compliment call. It has had a try/catch around the compliment code since May of 2014 infact! It might seem like the exceptions are currently uncaught or re-thrown since the stacktrace is appearing in the REPL, but the CIDER middleware code is actually:

1. Catching the exception in the complete-reply method
2. Creating a digestible error response packet out of it in the u/err-info function
3. This error packet gets transported over the wire from the clojure code in the JVM to the elisp code in emacs
4. The elisp response handler then recognizes that it got an error packet, extracts the stacktrace, and prints the stacktrace in the REPL in a controlled fashion.

Having a stacktrace show up in the REPL is clearly not the optimal or most user friendly solution, but it is more user friendly than the behavior of having the exception go uncaught -- which completely hangs the system and requires a C-c C-b to interrupt the hung evaluation thread!

I'm still trying to grab all the low hanging fruit WRT error handling (i.e., prevent hangs due to exceptions), but I do intend to try and improve the user experience and find a happy medium between silently swallowing an exception and getting a stacktrace thrown in your face. I'm about half way done with the easy stuff, so stay tuned...

[alexander-yakushev]

In that case, the correct solution would be to catch and rethrow the exception here: https://github.com/clojure-emacs/cider-nrepl/blob/master/src/cider/nrepl/middleware/complete.clj#L19

By arbitrary rethrowing errors inside Compliment I would force hypothetical developers that might want to wrap Compliment in their own libraries (not in the user-facing solution that CIDER is) to manually unpack my rethrown exception, or to read a folded stacktrace.

[expez]

I just had this happen to me again. This time it happened because the completion was triggered in file which hadn't been loaded yet:

java.lang.NullPointerException: null
 at java.util.concurrent.ConcurrentHashMap.get (ConcurrentHashMap.java:936)
    clojure.lang.Namespace.find (Namespace.java:188)
    clojure.core$find_ns.invoke (core.clj:3976)
    clojure.core$the_ns.invoke (core.clj:4008)
    clojure.core$ns_map.invoke (core.clj:4022)
    cider.inlined_deps.compliment.v0v2v7.compliment.sources.ns_mappings$candidates.invoke (ns_mappings.clj:53)

Obviously find-ns is going to fail pretty hard here in this situation.

[jstaffans]

I am running into this quite often, usually after doing a cider-refresh which due to some bug results in a compilation error. When I go back to the file to fix the error, I get this same stack trace as I start typing.

Is there any way to work around this for now? It makes working with CIDER pretty much impossible.

[sanjayl]

@jstaffans: if you clone the cider-nrepl repo, make the single change shown with the src/cider/nrepl/middleware/complete.clj file shown on my branch related to this bug, and then run ./build.sh install, it will install a patched version of cider-nrepl that will swallow these errors. let me know how this goes

[bbatsov]

I am running into this quite often, usually after doing a cider-refresh which due to some bug results in a compilation error. When I go back to the file to fix the error, I get this same stack trace as I start typing.

Is there any way to work around this for now? It makes working with CIDER pretty much impossible.

What's the exact exception your getting? As I said before I'm against just ignoring all exceptions as this would masquerade real issues with the code. I'm still open to idea regarding this, but as I said before - it's not wise to suppress generic exceptions that might occur for 10 different reasons. If there was something more specific we could handle - it would certainly help.

[jstaffans]

I get the exact same stack trace as mentioned in the original bug report for this issue.

@sanjayl I'll try that out when I have more time, for now I switched editors.

[alexander-yakushev]

cider-refresh is the hint. Can it destroy namespaces if it fails?

@bbatsov There is 0.2.8-SNAPSHOT Compliment that you can use. It contains a possible fix for this issue, you can try to use it and we see if anything changes for @jstaffans and others.

[bbatsov]

So, I've updated compliment - let me know if this fixes the bug or not.

[jstaffans]

Thanks, I'll keep you posted.

[jstaffans]

I still get the same stacktrace and REPL focus using CIDER 0.12.0snapshot (package: 20160330.3). Is that the version that the fix should be on? I'm an Emacs newbie.

[alexander-yakushev]

@jstaffans Dependes whether cider-nrepl snapshot was re-downloaded or not.

Regardless, it seems that you have a quite reproducible case. It will be extremely helpful if you write down the exact steps that lead to the said stacktrace printed. Thank you!

[sanjayl]

@alexander-yakushev : Maybe there needs to be a check in ensure-ns to make sure that *ns* hasn't gotten set to nil somehow by the refresh code? I don't even know if refresh could even conceivably do this, but line 54 in this function is the only place I can see a nil creeping it's way into the system??

[alexander-yakushev]

@sanjayl It could be so, but when *ns* is set to nil, broken Compliment is one of minor issues to worry about. I don't think Clojure is designed to work without a namespace. I could put a post-condition on ensure-ns, but it won't change much in this case. If this is true, we have to find who can clear the *ns*.

[bbatsov]

If this is true, we have to find who can clear the ns.

You're right. If only someone could reproduce this reliably...

[bbatsov]

On a related note, we should probably add an option to suppress internal middleware errors, so their presence won't be so disruptive to users in the absence of a proper solution.

[alexander-yakushev]

Why don't we redirect them to cider-debug or cider-error buffer? I can't check right now but I think we have something like that, right?

[bbatsov]

Why don't we redirect them to cider-debug or cider-error buffer? I can't check right now but I think we have something like that, right?

We're about to remove this functionality completely in this release - @sanjayl is working on a solution that would allow us to display regular stacktrace buffers in case of such errors (as opposed to dumping some stacktrace in the REPL). We should just add an option not to display/select them automatically IMO.

[expez]

I think stacktrace buffers are OK for errors that happen once in a blue moon, but when there's a bug, like the one which caused me to open this issue, the stacktrace buffer isn't an improvement.

What I'd like:

1. A setting for showing unexpected errors in a stacktrace buffer controlled by a var (defaulting to t) like @bbatsov suggested.
2. A button in the stracktrace buffer, for middleware related errors, which suppresses future errors of exactly this type, and only this type, for the current session.

[bbatsov]

We'll implement something like 1 for sure. As for 2 - I like the idea, but I'm not sure we'll find someone to work on something like this any time soon.

[sanjayl]

Once CIDER PR #1645 lands (which sends all exceptions to a stacktrace buffer instead of spamming the REPL), I think option 1 will work out of the box. There's a variable named cider-show-error-buffer in cider-interaction that controls whether newly rendered stacktrace buffers pop to the front or not. If this is set to nil then all errors will be rendered in a stacktrace buffer but will stay in the background.

Customizing this behavior further as required by option 2 or something similar is possible but will require slightly broadening the scope of some of the internal plumbing. I need to dig a bit deeper into the code to see how that can be done best.

[bbatsov]

Once CIDER PR #1645 lands (which sends all exceptions to a stacktrace buffer instead of spamming the REPL), I think option 1 will work out of the box. There's a variable named cider-show-error-buffer in cider-interaction that controls whether newly rendered stacktrace buffers pop to the front or not. If this is set to nil then all errors will be rendered in a stacktrace buffer but will stay in the background.

Still, you'd want to treat internal errors differently from eval errors, so it's not as simple as this. But you're right - at least people will be able to workaround the problem.

[sanjayl]

Here's a quick screencast of what I have so far, I think it satisfies both No. 1 and 2 from above:
https://youtu.be/gHWvsdSMX6o
Submitting as a PR to CIDER

[bbatsov]

Excellent. I have a few remarks about the UI, but I'll mention them on the Elisp PR when you file it.

[bbatsov]

Guess we can close this.