Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_bin): improve error handling for Unix domain sockets #3137

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Aug 30, 2022

Summary

This PR fixes how the domain socket file used to communicated between the daemon server and workspace clients is handled.
Specifically, since the file is left behind in the system temporary directory when the daemon process is killed, the open_socket needs to handle the ConnectionRefused error in the same way as the NotFound error and consider the daemon is not running.
I also extended this behavior to the print_socket with an additional safeguard as it will only try to connect a limited number of times before failing with the last error reported by the system.
Finally, since the server cannot bind to an existing file I added a call to remove_file on the domain socket file if it exists when the run_daemon function is called

Test Plan

I tested this manually using Ubuntu on WSL: I checked that after spawning the server process with cargo rome-cli-dev __run_server then killing it, the main CLI started with cargo rome-cli-dev format editors/vscode/src/main.ts was not using the daemon server nor failing to initialize a connection, and the server process could be started again without failing on the pre-existing socket file. I guess it would probably benefit from being tried out on a macOS machine, as well as a "physical" Linux setup instead of the "virtual" Windows subsystem

@leops leops requested a review from a team August 30, 2022 12:38
@leops leops temporarily deployed to aws August 30, 2022 12:38 Inactive
@github-actions
Copy link

@ematipico
Copy link
Contributor

I am going to test it on my machine

@ematipico
Copy link
Contributor

It seems to be working as expected:

  • I connected my VSCode extension to the built binary, and made sure that formatting worked;
  • then I closed the extension and made sure that the daemon process was still running;
  • called the format command via CLI, it worked;
  • killed the process and called the format command via CLI, it worked;
  • opened the VSCode extension, no errors and the connection got created again;
  • killed the process while the extension was open and the daemon gets spawned against successfully. Here's the logs
[Info  - 4:37:19 PM] Server initialized with PID: 40405
[Info  - 4:39:07 PM] Connection to server got closed. Server will restart.
[Info  - 4:39:07 PM] Server initialized with PID: 40451
[Info  - 4:39:36 PM] Connection to server got closed. Server will restart.
[Info  - 4:39:36 PM] Server initialized with PID: 40460

@leops leops merged commit de04db1 into main Aug 30, 2022
@leops leops deleted the fix/unix-domain-socket branch August 30, 2022 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants