Better stop cleanup (#23)

- Always remove pid files at the end of stop - Swallow more no such process Fixes DO-841
mentimeter · Aug 28, 2023 · 2209908 · 2209908
1 parent a395ea7
commit 2209908
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 27 deletions.
diff --git a/linkup-cli/src/background_tunnel.rs b/linkup-cli/src/background_tunnel.rs
@@ -11,7 +11,7 @@ use url::Url;
 
 use crate::signal::send_sigint;
 
-use crate::stop::stop_tunnel;
+use crate::stop::stop_pid_file;
 use crate::{linkup_file_path, CliError};
 use crate::{LINKUP_CLOUDFLARED_PID, LINKUP_LOCALSERVER_PORT};
 
@@ -133,7 +133,7 @@ fn try_start_tunnel() -> Result<Url, CliError> {
     match rx.recv_timeout(Duration::from_secs(TUNNEL_START_WAIT)) {
         Ok(result) => result,
         Err(e) => {
-            stop_tunnel()?;
+            stop_pid_file(LINKUP_CLOUDFLARED_PID)?;
             Err(CliError::StartLocalTunnel(format!(
                 "Failed to obtain tunnel URL within {} seconds: {}",
                 TUNNEL_START_WAIT, e

diff --git a/linkup-cli/src/stop.rs b/linkup-cli/src/stop.rs
@@ -10,21 +10,14 @@ use crate::{
 };
 
 pub fn stop() -> Result<(), CliError> {
-    let local_stopped = match get_pid(LINKUP_LOCALSERVER_PID_FILE) {
-        Ok(pid) => send_sigint(&pid).map_err(|e| {
-            CliError::StopErr(format!(
-                "Could not send SIGINT to local server pid {}: {}",
-                pid, e
-            ))
-        }),
-        Err(PidError::NoPidFile(_)) => Ok(()),
-        Err(e) => Err(CliError::StopErr(format!(
-            "Could not get local server pid: {}",
-            e
-        ))),
-    };
-
-    let tunnel_stopped = stop_tunnel();
+    let local_stopped = stop_pid_file(LINKUP_LOCALSERVER_PID_FILE);
+    if local_stopped.is_ok() {
+        let _ = std::fs::remove_file(LINKUP_LOCALSERVER_PID_FILE);
+    }
+    let tunnel_stopped = stop_pid_file(LINKUP_CLOUDFLARED_PID);
+    if tunnel_stopped.is_ok() {
+        let _ = std::fs::remove_file(LINKUP_CLOUDFLARED_PID);
+    }
 
     let state = get_state()?;
     for service in &state.services {
@@ -48,23 +41,23 @@ pub fn stop() -> Result<(), CliError> {
     }
 }
 
-pub fn stop_tunnel() -> Result<(), CliError> {
-    match get_pid(LINKUP_CLOUDFLARED_PID) {
+pub fn stop_pid_file(pid_file: &str) -> Result<(), CliError> {
+    match get_pid(pid_file) {
         Ok(pid) => {
             match send_sigint(&pid) {
                 Ok(_) => Ok(()),
                 // If we're trying to stop it but it's already died, that's fine
                 Err(PidError::NoSuchProcess(_)) => Ok(()),
                 Err(e) => Err(CliError::StopErr(format!(
-                    "Could not send SIGINT to cloudflared pid {}: {}",
-                    pid, e
+                    "Could not send SIGINT to {} pid {}: {}",
+                    pid_file, pid, e
                 ))),
             }
         }
         Err(PidError::NoPidFile(_)) => Ok(()),
         Err(e) => Err(CliError::StopErr(format!(
-            "Could not get cloudflared pid: {}",
-            e
+            "Could not get {} pid: {}",
+            pid_file, e
         ))),
     }
 }

diff --git a/linkup/src/lib.rs b/linkup/src/lib.rs
@@ -319,10 +319,7 @@ mod tests {
 
         // Origin
         let mut origin_headers: HashMap<String, String> = HashMap::new();
-        origin_headers.insert(
-            "origin".to_string(),
-            format!("http://{}.example.com", name),
-        );
+        origin_headers.insert("origin".to_string(), format!("http://{}.example.com", name));
         sessions
             .get_request_session("example.com".to_string(), origin_headers)
             .await