Debugger breaks on quit when debugging function called via http

[danielneal]

Expected behavior

Pressing q (for quit) when debugging a function called indirectly via http should quit the debugger without causing future issues.

Actual behavior

Pressing q (for quit) when debugging a function called indirectly via http causes an error:
Execution error (NullPointerException) at cider.nrepl.middleware.debug/abort! (debug.clj:143)

After this error occurs, all future attempts to invoke the debugger fail silently.

Steps to reproduce the problem

1. Set up a simple ring server e.g. git clone https://github.com/danielneal/cider-debugger-issue
2. Connect to the problem e.g. using cider-jack-in
3. Run cider-debug-defun-at-point with the point in the body of the handler function.
4. Start the server by running the start-server command in the comment block.
5. Call the server through http, using the http/get command in the comment block.
6. The cider debugger will appear, press quit. This will cause an error.
7. Repeat step 4, the cider debugger will no longer appear.

Environment & Version information

cider-nrepl version

0.25.5

Java version

1.8

Operating system

Mac OS X 10.15.7 Catalina

[jumarko]

I can add that pressing C leads to a similarly broken state where the breakpoints cannot be hit anymore

[danielneal]

Even if fix is not easy, a workaround would be very useful. If for example, there's a function to call that would reset debugger state? It takes quite a long time for our system to restart, and killing the repl and restarting everything is the only way I know to get the debugger working again once this state is hit.

[expez]

@bbatsov any idea how to work around this? The debugger essentially broken for all of us that work on webapps.

If anyone else sees this and is looking for an alternative, I've reverted to using the updated clj-commons version of spyscope. I also use these convenience functions to add/remove the reader macro:

(defun toggle-spy (p)
  (interactive "P")
  (if current-prefix-arg
      (unspy)
    (insert "#spy/d "))
  (cider-eval-defun-at-point))

(defun unspy ()
  (save-excursion
    (let ((start (progn (cljr--goto-toplevel) (point)))
          (end (progn (paredit-forward) (point))))
      (replace-regexp "#spy/d " "" nil start end))))

[bbatsov]

(defn- abort!
  "Stop current eval thread.
  This does not quit the repl, it only interrupts an eval operation."
  []
  (if (map? *msg*)
    (do
      (transport/send (:transport *msg*) (response-for *msg* :value 'QUIT))
      (.stop ^Thread (:thread (meta (:session *msg*)))))
    ;; We can't really abort if there's no *msg*, so we do our best
    ;; impression of that. This is only used in some panic situations,
    ;; the user won't be offered the :quit option if there's no *msg*.
    (skip-breaks! :all)))

It might be something to do with this .stop here. I've started to wonder if that's not some regression from the changes to the threading model that we made in nREPL 0.6. I can't offer any workarounds and I can't make any promises about when I'll be able to take a closer look at this, as I'm overwhelmed with work. Perhaps @vspinu and @Malabarba migth be able to check this problem faster than me.

[danielneal]

@expez workaround: making sure to press c for continue rather than C or q allows the debugger to continue without breaking. Just make sure not to press q instinctively ;)

[bbatsov]

I recall that @grammati and @yuhan0 were also familiars with the internals of the debugger. I've checked that this function hasn't been updated since 2015, which reinforces my belief this was probably working fine before nREPL 0.6 and it's not a problem that was always there.

[bbatsov]

Looking at the session metadata in the source it doesn't seem like it has a thread key anymore (this is code from nREPL):

(defn- create-session
  "Returns a new atom containing a map of bindings as per
  `clojure.core/get-thread-bindings`. *in* is obtained using `session-in`, *ns*
  defaults to 'user, and other bindings as optionally provided in
  `session` are merged in."
  ([{:keys [transport session out-limit] :as msg}]
   (let [id (uuid)
         {:keys [input-queue stdin-reader]} (session-in id transport)
         the-session (atom (into (or (some-> session deref) {})
                                 {#'*in* stdin-reader
                                  #'*ns* (create-ns 'user)})
                           :meta {:id id
                                  :out-limit (or out-limit (:out-limit (meta session)))
                                  :stdin-reader stdin-reader
                                  :input-queue input-queue
                                  :exec default-exec})
         msg {:code "" :session the-session}]
     ;; to fully initialize bindings
     (binding [*msg* msg]
       (evaluate msg))
     the-session)))

I guess one of you can play with this to check if this is the problem.

[Cartmanishere]

Here's a guess on what's happening based on some debugging.

I think the reason for this behavior with HTTP handlers is the (skip-breaks! :all).

The same behavior with HTTP handlers can be observed with :continue-all and :quit.
Because both of these commands set the *skip-breaks* dynamic atom to {:mode :all}.

This is not a problem with normal functions in debug mode because each new invocation of that function starts with a new context bindings. So each new call to debuggable function starts with *skip-breaks* as nil.

But in case of a HTTP handler, the web context is persistent.
Once the *skip-breaks* is set to {:mode :all}, it does not change even if you re-instrument the HTTP handler function.

For e.g this works, even if you :quit from inside a HTTP handler fn.

(defn test-debugger
  []
  (println "*skip-break*: " @cider.nrepl.middleware.debug/*skip-breaks*)
  (let [a (rand-int 1000)
        b (rand-int 2000)]
    (str "Random generated: " (* a b)))
  (swap! cider.nrepl.middleware.debug/*skip-breaks* (constantly nil)))