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.