manateelazycat / lsp-bridge

A blazingly fast LSP client for Emacs
GNU General Public License v3.0
1.42k stars 205 forks source link

enable restart remote process and reconnect to remote process #825

Closed nohzafk closed 8 months ago

nohzafk commented 8 months ago

I'm very happy to introduce to you this new functionality 😃 The update includes the ability to:

While I have thoroughly tested it in a local environment, I'm aware that it is a significant modification. Please take the necessary time to review and test. Ideally, I would have divided this update into several smaller PRs for ease of review; however, they are tightly coupling together, such a split would cost me too much time.

following I will explain what I did in detail.

Fixes and Renaming

fix lsp-bridge--with-file-buffer

this macro is missing a comma before filehost it should be ,filehost, so it always result in an error and fall to the second clause of the pcase

`(when-let ((buffer (pcase ,filehost

fix get_ssh_password error

when try to connect_ssh in RemoteFileClient, running get_ssh_password on macOS will throw an error:

epc.handler.ReturnError: [Symbol('error'), 'The macOS Keychain auth-source backend doesn’t support creation yet']

so it must be moved inside the try...except block

rename get/set_remote_tramp_method to get/set_remote_tramp_connection_info

tramp_method is a standard terminology of emacs TRAMP which is something like ssh docker, see File name syntax TRAMP manual.

previously in lsp_bridge.py sync_tramp_remote use this name tramp_method and its value is like /ssh:user@ip#port:

        tramp_file_split = filename.rsplit(":", 1)
        tramp_method = tramp_file_split[0] + ":"
        file_path = tramp_file_split[1]

the name tramp_method used here was very misleading, here all related names are renamed to tramp_connection_info to avoid any confusion, and its value is not changed, just the name.

tramp_connection_info = tramp_file_name.rsplit(":", 1)[0] + ":"

enabling auto start remote process

Current implementation of auto start remote process DO NOT work

lsp-bridge-start-remote-process is running on local emacs and use of make-process only create a process on local machine, not on the remote server. So related code along with lsp-bridge-remote-process-alist are removed.

refactor get_socket_client and RemoteFileClient

To enable auto start process on remote server, I've carefully refactored get_socket_client and RemoteFileClient, there will be 3 major exceptions to handle with:

The first 2 exceptions are handled inside get_socket_client, when lsp-bridge-remote-start-automatically is set to t, RemoteFileClient().start_lsp_bridge_process(() is called and it use self.ssh.exec_command to start the remote bridge process correctly.

enabling auto reconnect to remote process when connection lost

This scenario is more complex, and I will try my best to explain it.

enhance the handling logic inside send_message_dispatcher

When a socket OSError is throwed inside the loop of send_message_dispatcher

    while True:
        data = queue.get(True)

        server_host = data["host"]
        client = self.get_socket_client(server_host, port)
        try:
            client.send_message(data["message"])
        except SendMessageException as e:
           # lsp-bridge process might has been restarted, making the orignal socket no longer valid.
            logger.exception("Channel %s is broken, message %s, error %s", f"{server_host}:{port}", data["message"], e)
            # remove all the clients for server_host from client_dict
            # client will be created again when get_socket_client is called
            self.client_dict.pop(f"{server_host}:{REMOTE_FILE_SYNC_CHANNEL}", None)
            self.client_dict.pop(f"{server_host}:{REMOTE_FILE_COMMAND_CHANNEL}", None)
            self.client_dict.pop(f"{server_host}:{REMOTE_FILE_ELISP_CHANNEL}", None)

            message_emacs(f"try to recreate channel to {server_host}:{port}")
            try:
                client = self.get_socket_client(server_host, port)
            except Exception as e:
                # unable to restore the channel
                logger.exception(e)
            else:
                # try to send the message
                client.send_message(data["message"])
                eval_in_emacs('lsp-bridge-remote-reconnect', server_host)
        except Exception as e:
            logger.exception(e)
        finally:
                    queue.task_done()

We have addressed the two scenarios of SSH login failed, or remote process does not eixist within the get_socket_client function.

When we encounter a SendMessageException while invoking client.send_message, this implies that the client’s channel has become invalid, the remote process might has been restarted.

We remove all the clients connected to the server from self.client_dict. This action forces the affected client to be re-created ensuring the re-establishment of the channel in other threads.

But for the message that triggered the exception, we need to resend it. We retrieve the client once more and send the message again.

And then we call emacs to evaluate lsp-bridge-remote-reconnect

edge case of starting order of remote_request_dispatcher and remote_file_elisp_dispatcher

firstly, in remote_file_elisp_dispatcer this check is removed

if get_remote_rpc_socket() is None:

allowing init_search_backends to be executed every time a client connects, otherwise it will only executed once when the process is started the first time.

normally when the remote process starts, no client is connecting, and init_serach_backends finished really quick, no error happens.

However in the reconnecting scenario, there will be chance that client connect to remote_request to send message before client connect to remote_file_elisp and init_search_backends finish execution, this will lead to an error like

Traceback (most recent call last):
  File "/workspaces/xxxx/lsp-bridge/lsp_bridge.py", line 624, in event_dispatcher
    getattr(self, func_name)(*func_args)
    ^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'LspBridge' object has no attribute '_signature_help'

these method are set by init_search_backends, so ideally we must wait for init_search_backends to finish execution before we start to handle remote request, this is done by using a threading event to synchronize these threads.

   # remote_file_elisp_dispatcher
    self.init_search_backends()
    log_time("init_search_backends finish")
    # Signal that init_search_backends is done
    self.init_search_backends_complete_event.set()
    # remote_request_dispatcher
    while True:
        # Record server host when lsp-bridge running in remote server.
        client_socket, client_address = self.remote_request.accept()
        # we wait for init_search_backends to finish execution
        # before start thread to handle remote request
        log_time("wait for init_search_backends to finsih execution")
        self.init_search_backends_complete_event.wait()

unify the logic handling for buffer created by lsp-bridge-open-remote-file and TRAMP buffer

we call emacs to evaluate lsp-bridge-remote-reconnect when reconnecting a remote file (TRAMP remote file or buffer created by lsp-bridge-open-remote-file.

Buffer created by lsp-bridge-opne-remote-file is NOT a tramp buffer, so in order to reuse the logic of lsp-bridge-sync-tramp-remote, we will try to construct a tramp-file-name like name to be used when (buffer-file-name) is called inside lsp-bridge-sync-tramp-remote, this requires us to add some buffer local variables to the buffer

these variables are added to buffer for lsp-bridge-construct-tramp-file-name to use to override buffer-file-name in lsp-bridge-remote-reconnect.

and finally a new macro lsp-bridge--conditional-update-tramp-file-info is used to unify the logic of setting tramp file inof of TRAMP buffer and lsp-bridge-open-remote-file buffer

Thanks

I'm working on using lsp-bridge inside container. This feature is a big milestone towrards the goal.

As container is created and restarted on the fly, lsp-bridge must support restart and reconnect remote process.

I hope I've explained myself well and thanks for your reading.

manateelazycat commented 8 months ago

Thanks for great path, looks forward container feature. ;)