bug#70724: 29.2.50; eglot-reconnect errors when the project is deleted

[Eli Zaretskii]

Ping!  Can we make some progress here?

> Cc: app-emacs-dev@janestreet.com
> Date: Sat, 4 May 2024 04:09:48 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> Hi Spencer,
> 
> On 02/05/2024 22:37, Spencer Baugh wrote:
>  >
>  > In some project /home/foo/proj, with pretty much any LSP server:
>  >
>  > 1. In /home/foo/proj, M-x eglot, starting some LSP server
>  >
>  > 2. Delete the directory /home/foo/proj
>  >
>  > 3. The LSP server will crash/exit
>  >
>  > 4. The process sentinel for the server will run, running
>  >     eglot--on-shutdown which by default will call eglot-reconnect
>  >
>  > 5. eglot-reconnect extracts the saved project instance out of the
>  >     server, which has a root directory which no longer exists, and calls
>  >     eglot--connect with it
>  >
>  > 6. eglot--connect calls project-name on a nonexistent project instance,
>  >     which may fail with an error depending on the project implementation
>  >     (I have a custom project implementation, but I think this can happen
>  >     with project-vc too)
>  >
>  > 7. This causes the process sentinel to error.
>  >
>  > I think the right fix is probably for eglot--on-shutdown (or maybe
>  > eglot-reconnect) to call (project-current nil (project-root pr)) to find
>  > the new project instance.  If that returns nil, the project has
>  > disappeared, and eglot should just not try to reconnect.  This also
>  > would make eglot behave better if the project layout changes (e.g. if
>  > there are nested projects).
> 
> I think I like this solution (as long as the nil value returned by 
> project-current on this step is appropriately handled).
> 
> Something like:
> 
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 6896baf30ce..7b2461c3ce6 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -1426,11 +1426,15 @@ eglot-reconnect
>     (interactive (list (eglot--current-server-or-lose) t))
>     (when (jsonrpc-running-p server)
>       (ignore-errors (eglot-shutdown server interactive nil 
> 'preserve-buffers)))
> -  (eglot--connect (eglot--major-modes server)
> -                  (eglot--project server)
> -                  (eieio-object-class-name server)
> -                  (eglot--saved-initargs server)
> -                  (eglot--language-ids server))
> +  (let* ((root (project-root (eglot--project server)))
> +         (project (project-current nil root)))
> +    (if (not project)
> +        (eglot--error "Project in `%s' is gone!" root)
> +      (eglot--connect (eglot--major-modes server)
> +                      project
> +                      (eieio-object-class-name server)
> +                      (eglot--saved-initargs server)
> +                      (eglot--language-ids server))))
>     (eglot--message "Reconnected!"))
> 
>   (defvar eglot--managed-mode) ; forward decl
> 
> 
> Though it also raises a question about the caching strategy for VC-Aware 
> project backend. At the moment is associates a project with a directory 
> more or less indefinitely, and this is a case to watch out for.
> 
>  > Alternatively, maybe eglot--on-shutdown shouldn't automatically
>  > reconnect in the first place?  Maybe reconnection should happen
>  > automatically only when some specific buffer tries to interact with the
>  > LSP - then it can run project-current in the context of that specific
>  > buffer, and see there's no project, and fail.  Plus, if the user kills
>  > all the buffers in the project (possibly with project-kill-buffers)
>  > before deleting it, this approach would entirely avoid the unnecessary
>  > eglot reconnection attempt.
> 
> This also sounds good, though it'd probably require more changes 
> overall. Additionally, perhaps I'd change the association from (server 
> -> project) to (server -> project-root), relying on the project 
> backends' internal caches to fetch the project value whenever it's 
> needed. That might be the most reliable approach. Perhaps the slowest in 
> theory, but hopefully not noticeably so.
> 
> 
> 
>