Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix remote server connection #842

Merged

Conversation

norris-young
Copy link
Contributor

@norris-young norris-young commented Jan 28, 2024

Add a heartbeat mechanism ffd0b98 for keeping remote connection alive to resovle #831 .
In the process of analyzing #831 ,I found some other bugs to fix here:

  1. 431171c lsp-bridge-is-remote-file needs synchronization with the server, so it is not real-time. We should not send requests for these remote files in updating to local servers or we will get unexpected errors. Before remote sync done, we just skip requests for these files.
  2. c328cc8 We should call sync_tramp_remote with pure text buffer name.
  3. ceb2f18 When we have both opened remote files and local files and do lsp-bridge-restart-process in a local file buffer, we would get errors if we do not remove remote files on indexing.

Besides, the code for LspBridge remote server seems to be in a mess and hard to understand. So I tried to do a refactor.
6ce0a1a 99476ec b1200f0

When a tramp file is opened, its information is updated by calling
'lsp-bridge-sync-tramp-remote' and will be finished in
'lsp-bridge-update-tramp-file-info'. Before it is done,
'lsp-bridge-remote-file-flag' is nil and related remote file info
variables are not set. If we call file api this time, it will send the
request to local lsp server(local epc call). Because the file is a
remote file in fact, we won't get right result frome local server and it
might leads to some unpredicatable exception. So we add check in
'lsp-bridge-call-file-api' to skip the call if update is not finished.
'buffer-file-name' might have properties and we should only pass text
string without any properties. Or we will get an error fo wrong number
of arguments.
Calling search_file_words_index_files with remote file names brings
error.
@norris-young norris-young force-pushed the fix.remote.connection branch from 63d589e to 3d60688 Compare January 28, 2024 08:41
Just move functions to do classification and fix a typo.
1. Implement FileSyncServer/FileCommandServer/FileElispServer inherited
from RmoeteFileServer which was only used by FILE_SYNC_CHANNEL.
2. Replace set_* functions with set_lsp_bridge.
3. Move call_remote_rpc into FileElispServer.
4. Replace remote_eval_socket with FileCommandServer.
5. Add remote_file_elisp_sender/receiver_queue to unify handlers for remote
messages from 3 servers and disable these when running in server.
6. Unify send_remote_message/receive_remote_message and their dispatchers.
If client does not send messages to the server for more than a specified
time(e.g. 2h12min for me), the server will close the socket while the
client can still send messages without error. However, the server will
not receive and handle any more messages.
In order to solve this issue, we add a mechanism to send heartbeats to
the server to keep the connection alive.
@norris-young norris-young force-pushed the fix.remote.connection branch from 3d60688 to b1200f0 Compare January 28, 2024 08:45
@manateelazycat manateelazycat merged commit f973018 into manateelazycat:master Jan 28, 2024
1 check passed
@manateelazycat
Copy link
Owner

thanks for patch

@norris-young norris-young deleted the fix.remote.connection branch January 28, 2024 09:03
@manateelazycat
Copy link
Owner

我要紧急撤回这一批补丁, 这一批补丁升级以后, 导致 Python 语言无法补全。

请把您要提交的补丁, 按照每一个功能, 单独提交, 这样更容易测试, 也更容易合并。

@manateelazycat
Copy link
Owner

我已经提交了 5baf390 , 撤销了这个PR的所有代码。

请把功能单开拆分提交, 一个功能一个补丁, 并保障远程补全可以工作的情况下, 本地代码补全也要测试。

@norris-young
Copy link
Contributor Author

好的 大概已经找到问题了 我再测试测试 有空再分开提过来

@norris-young
Copy link
Contributor Author

已经尽量拆分开了
没有依赖关系的patch我先推了 #847 #848 #849 #850
有依赖关系的后面一个一个推过来

@manateelazycat
Copy link
Owner

好的,我这几天堵在高速上,等我到酒店有空了,我来看这些补丁,辛苦了,辛苦了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants