Re: jsonrpc.el uses "Content-Length" headers which seem specific to LSP

[João Távora]

Adam Porter <adam@alphapapa.net> writes:

>> To give you an example, in a private project I subclassed
>> jsonrpc-connection to make a websocket-based JSONRPC transport.
>> It was really easy (I can show the code if you're interested).
>
> Sure, that could be helpful.

Shown at the bottom of this message after my sig.  I'm not showing the
websocket library or the full thing, of course, but it should give you
an idea.

> Yes, we're using a network process connecting to a local TCP
> server. Indeed, we didn't want to reinvent those things.
>
>> Now, the flaw I will concede about jsonrpc-process-connection is that it
>> describes itself as a "jsonrpc connection over an Emacs process", but it
>> would be more accurately described as "JSONRPC connection over HTTP-like
>> over Emacs process".  Presumably, you want the first and the last part
>> but not the middle.  
>
> Right, the code we wrote (shared below) basically removes those middle
> parts.

After having looked at your code, I think you may be underestimating the
use of the Content-Length header.  It's not a spurious decoration.
Rather it helps processes make sense of incomplete or multiple messages
that may appear in the process buffer.  I'd say it's only in toy
examples or "lucky" cases in which you can safely jsonrpc--json-read one
single full JSON objects to completeness.

In other words, with or without refactorings or adjustments to
jsonrpc.el's design, I don't think this code can work reliably within
Emacs:

(defun hyperdrive-jsonrpc--process-filter (proc string)
  (when (buffer-live-p (process-buffer proc))
    (with-current-buffer (process-buffer proc)
      (let* ((inhibit-read-only t)
             (connection (process-get proc 'jsonrpc-connection)))
        ;; Insert the text, advancing the process marker.
        (save-excursion
          (goto-char (process-mark proc))
          (insert string)
          (set-marker (process-mark proc) (point)))
        (condition-case err
            (when-let ((json-message (jsonrpc--json-read)))
              (with-temp-buffer
                (jsonrpc-connection-receive connection json-message)))
          (error (message "hyperdrive-jsonrpc--process-filter ERROR:%S  PROC:%S 
 STRING:%S"
                          err proc string)))))))

For example, what if two full messages appear in the process buffer
suddenly and the other endpoint is waiting for a reply to the second one
before it sends more stuff?  Isn't that deadlock?  What if a very large
message appears in chunks?  Are you going to waste lots of works trying
to read it and failing each time?

I'd be careful about these things, but if you can absolutely guarantee
that this never happens or somehow change the code to deal with them
reliably, then OK.  In any case, I'm now starting to think your code
doesn't motivate any significant reworking of the class hierarchy in
jsonrpc.el.

It's true it uses some jsonrpc.el internals when it shouldn't but it's
quite OK to augment the jsonrpc.el API to export jsonrpc--json-read and
the few other internal helpers you're using.  That doesn't pose any
backward-compatibility challenge and your code is indeed quite simple.

> Of course, one of the considerations is backward compatibility with
> existing users of jsonrpc.  There don't appear to be very many yet,
> other than Eglot, so some kind of transition might be feasible,
> especially since jsonrpc.el is on GNU ELPA.

There are definitely other users, some not open source, but some are
open source.  Last time I did a GitHub code search I came up with at
least couple non-Eglot uses.  Not sure what you mean by "transition",
but any refactorings/redesigns must not introduce breaking changes.

João


(defclass nih--connection (jsonrpc-connection)
  ((repl
    :accessor nih--repl
    :documentation "REPL buffer")
   (target-info
    :reader nih--target-info :initarg :target-info
    :documentation
    "Full target info.  A JSON object returned by /json/list.
May be augmented with nih-specific fields.")
   (socket
    :reader nih--socket
    :initarg :socket
    :documentation "Open WEBSOCKET object"))
  :documentation "Represents a NIH connection.  `jsonrpc-name' is NOT
  unique.")

(cl-defmethod jsonrpc-connection-send ((conn nih--connection)
                                       &rest args
                                       &key
                                       _id
                                       method
                                       _params
                                       _result
                                       _error
                                       _partial)
  "Send MESSAGE, a JSON object, to CONNECTION."
  ;; next form clearly something to put in an :around method
  (when method
    (plist-put args :method
               (cond ((keywordp method) (substring (symbol-name method) 1))
                     ((and method (symbolp method)) (symbol-name method)))))
  (let* ((message `(;; CDP isn't technically JSONRPC, so don't send
                    ;; the `:jsonrpc' "2.0" version identifier which
                    ;; trips up node's server, for example.
                    ,@args))
         (json (jsonrpc--json-encode message)))
    (with-slots (socket) conn
      (websocket-send-text socket json))
    ;; also something for a generic :AFTER
    (jsonrpc--log-event conn message 'client)))

(cl-defmethod jsonrpc-running-p ((conn nih--connection))
  (with-slots (socket) conn (websocket-openp socket)))

(cl-defmethod jsonrpc-shutdown ((conn nih--connection))
  (with-slots (socket) conn (websocket-close socket)))

(defun nih--on-websocket-message (ws frame)
  "Called when FRAME received on NIH WS."
  (jsonrpc-connection-receive
   (websocket-client-data ws) ; the connection
   (json-parse-string
    (websocket-frame-text frame)
    :object-type 'plist
    :null-object nil
    :false-object :json-false)))