Better debugger support about emacs-websocket

[ahyatt]

That should be possible by separating the decoding of frames and callbacks until the very end of the network processing. I'll look into it, thanks for the suggestion.

[ahyatt]

I've done by best to have a reasonable solution in my latest change, 14e94a4.

However, it doesn't completely solve your problem. After thinking about the issue, I haven't found any acceptable design that would allow errors to be thrown safely. If several frames come in at once, and the first one generates an error on the client side, we just can't keep processing the remaining frames immediately after allowing the user to debug the first error.

The current design I've implemented should give you the flexibility to debug when you want to, though. So hopefully it will help you out.

[tkf]

Hi, thanks for the change. However, I am not sure if I explained what I meant well enough. What I meant was something like deferred:debug-on-signal and deferred:condition-case (https://github.com/kiwanami/emacs-deferred/blob/master/deferred.el#L158), where you can go into the *Backtrace* buffer if you are in "debugging mode" when the error occurs. If I understand your change correctly, websocket.el still hides stack trace from Emacs debugger, right? To pass the trace to the debugger, I believe there should be no error handler in the condition-case calls. Right, you can re-raise the error with the error function, but I think trace back information is gone (I am not sure about this. Probably there is a way to do it. I would like to know if so.).

For example, if you execute a "plain" lisp

(append (list (car 1)) (list 1 2))

you get trace back like this:

Debugger entered--Lisp error: (wrong-type-argument listp 1)
  car(1)
  (list (car 1))
  (append (list (car 1)) (list 1 2))
  eval((append (list (car 1)) (list 1 2)) nil)
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

You can see where the error occurs clearly, because you can see car's callers.

However, if you re-raise it like this (I guess this is what you have in mind for the on-error use case)

(condition-case err
    (append (list (car 1)) (list 1 2))
  (error
   (error err)))

the original traceback information is gone. You will get this:

Debugger entered--Lisp error: (wrong-type-argument stringp (wrong-type-argument listp 1))
  format((wrong-type-argument listp 1))
  apply(format (wrong-type-argument listp 1))
  error((wrong-type-argument listp 1))
  (condition-case err (append (list (car 1)) (list 1 2)) (error (error err)))
  eval((condition-case err (append (list (car 1)) (list 1 2)) (error (error err))) nil)
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

[ahyatt]

The problem is that if I don't wrap the error handling in a condition case, it's hard to avoid messing up processing when emacs gets control before our code has finished processing all frames.

Fortunately, a solution exists: just have your program call "(debug)" in your error handler. I think that should solve your problem. Probably worth mentioning in the docs, too...

[tkf]

Ah! I didn't know calling (debug) in handler clause of condition-case can show original trace! I thought it just discards the original trace (like (error (error err)) in my previous comment). Thank you very much.

This brings me another question: can't you do this even without on-error callback? I mean, what is the difference between the following two:

;; Case 1

(defun my-on-message (...)
  (my-do-something))

(defun my-on-error (...)
  (when my-debug
    (debug))
  (my-unwind-something))

and

;; Case 2

(defun my-on-message (...)
  (unwind-protect
    (condition-case nil
       (my-do-something)
     (error (when my-debug
              (debug))))
   (my-unwind-something)))

[ahyatt]

There's no theoretical difference. However, I believe a good API should be hard to misuse. If we relied on clients to protect against errors in their on-message callbacks, then they may fail to do so, which will cause clients issues when errors do happen. If we do the right thing ourselves by default, and they have to take special action to do something dangerous like rethrow or debug an error, then the API becomes much more robust.

[tkf]

Thanks for the clarification. I agree with you about API. It's nice if you can save all user from writing complicated function like case 2. Also, I'm +1 for adding the tips in document.

[tkf]

Hi, it seems I was wrong about that (debug) shows original stack trace. In fact, original original stack trace is gone in the condition-case handler:

Case 1 (w/o condition-case)

(let ((a 1))
  (list (car a)))

gives:

Debugger entered--Lisp error: (wrong-type-argument listp 1)
  car(1)
  (list (car a))
  (let ((a 1)) (list (car a)))
  eval((let ((a 1)) (list (car a))) nil)
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

Case 2 (with condition-case)

(condition-case nil
    (let ((a 1))
      (list (car a)))
  (error (debug)))

gives:

Debugger entered: nil
  (condition-case nil (let ((a 1)) (list (car a))) (error (debug)))
  eval((condition-case nil (let ((a 1)) (list (car a))) (error (debug))) nil)
  eval-last-sexp-1(nil)
  eval-last-sexp(nil)
  call-interactively(eval-last-sexp nil nil)

Note that in the case 1, you can type e a RET to see what is inside of the variable a, whereas it is impossible in the case 2. It is very useful feature when you don't know where is the bug and why it happens.

I still think it is possible to save parser state using unwind-protect and continue the filter function after debugging. But maybe it is better not to have this complexity to avoid unnecessary bugs. Probably it is better if somebody writes better debugger which can save any stack trace like ERT does.

[ahyatt]

I'm still thinking about this. Evidently if we replace (error (debug)) with ((debug error)), then the client gets to debug with the correct stack. Also, wrapping the whole thing with ignore-errors means that we can get the flow to resume, which means that we can have the best of both worlds. For example...

(progn
  (message "1")
  (ignore-errors
    (condition-case nil
        (let ((a 1))
          (list (car a)))
      ((debug error) (message "In error"))))
  (message "2"))

This seems ideal to me. Not perfect, since a user that is in the debugger may hit "q" instead of "c", and quit the debugger, messing up the websocket state.

[tkf]

Wow, this trick is cool. I see the manual mentioning this. I don't know why I missed that.

unwind-protect does not work against "q"? Anyway, I agree that this is the best solution we've got.

[ahyatt]

OK, added a function based around the above idea in commit f728631. Now just set websocket-callback-debug-on-error to t to get proper debugging.

[tkf]

Thank you very much! This is awesome! It just helped me finding a problem in JSON sent from IPython server. It was possible because I could stop JSON parser using debug-on-error and look at the parser state. I can imagine that it would be very hard to find the problem without the debugger support!