Adding error handling to telemetry clean up.

rust-lang · Apr 21, 2016 · a00f9bc · a00f9bc
1 parent 783f0d6
commit a00f9bc
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 20 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,4 @@
 /Cargo.lock
 /.settings
 *~
+/**/target
diff --git a/src/rustup/command.rs b/src/rustup/command.rs
@@ -88,15 +88,23 @@ fn telemetry_rustc<S: AsRef<OsStr>>(mut cmd: Command, args: &[S], cfg: &Cfg) ->
             let te = TelemetryEvent::RustcRun { duration_ms: ms, 
                                                 exit_code: exit_code,
                                                 errors: e };
-            t.log_telemetry(te);
+
+            let _ = t.log_telemetry(te).map_err(|xe| {
+                cfg.notify_handler.call(Notification::TelemetryCleanupError(&xe));
+            });
+
             process::exit(exit_code);
         },
         Err(e) => {
             let exit_code = e.raw_os_error().unwrap_or(1);
             let te = TelemetryEvent::RustcRun { duration_ms: ms,
                                                 exit_code: exit_code,
                                                 errors: None };
-            t.log_telemetry(te);
+
+            let _ = t.log_telemetry(te).map_err(|xe| {
+                cfg.notify_handler.call(Notification::TelemetryCleanupError(&xe));
+            });
+
             Err(rustup_utils::Error::RunningCommand {    
                 name: args[0].as_ref().to_owned(),
                 error: rustup_utils::raw::CommandError::Io(e),

diff --git a/src/rustup/errors.rs b/src/rustup/errors.rs
@@ -34,6 +34,7 @@ pub enum Notification<'a> {
     UpgradeRemovesToolchains,
     MissingFileDuringSelfUninstall(PathBuf),
     SetTelemetry(&'a str),
+    TelemetryCleanupError(&'a Error),
 }
 
 #[derive(Debug)]
@@ -66,6 +67,7 @@ pub enum Error {
         id: String,
         desc: String,
     },
+    TelemetryCleanupError(io::Error),
 }
 
 pub type Result<T> = ::std::result::Result<T, Error>;
@@ -94,7 +96,8 @@ impl<'a> Notification<'a> {
             UpdatingToolchain(_) |
             ReadMetadataVersion(_) |
             InstalledToolchain(_) |
-            UpdateHashMatches => NotificationLevel::Verbose,
+            UpdateHashMatches | 
+            TelemetryCleanupError(_) => NotificationLevel::Verbose,
             SetDefaultToolchain(_) |
             SetOverrideToolchain(_, _) |
             UsingExistingToolchain(_) |
@@ -155,7 +158,8 @@ impl<'a> Display for Notification<'a> {
             MissingFileDuringSelfUninstall(ref p) => {
                 write!(f, "expected file does not exist to uninstall: {}", p.display())
             }
-            SetTelemetry(telemetry_status) => write!(f, "telemetry set to '{}'", telemetry_status)
+            SetTelemetry(telemetry_status) => write!(f, "telemetry set to '{}'", telemetry_status),
+            TelemetryCleanupError(e) => write!(f, "unable to remove old telemetry files: '{}'", e),
         }
     }
 }
@@ -188,6 +192,7 @@ impl error::Error for Error {
             SelfUpdateFailed => "self-updater failed to replace multirust executable",
             ReadStdin => "unable to read from stdin for confirmation",
             Custom { ref desc, .. } => desc,
+            TelemetryCleanupError(_) => "unable to remove old telemetry files"
         }
     }
 
@@ -199,6 +204,7 @@ impl error::Error for Error {
             Temp(ref e) => Some(e),
             UpgradeIoError(ref e) => Some(e),
             WindowsUninstallMadness(ref e) => Some(e),
+            TelemetryCleanupError(ref e) => Some(e),
             UnknownMetadataVersion(_) |
             InvalidEnvironment |
             NoDefaultToolchain |
@@ -270,6 +276,7 @@ impl Display for Error {
             SelfUpdateFailed => write!(f, "{}", self.description()),
             ReadStdin => write!(f, "{}", self.description()),
             Custom { ref desc, .. } => write!(f, "{}", desc),
+            TelemetryCleanupError(ref e) => write!(f, "Unable to delete telemetry files {}", e.description()),
         }
     }
 }
diff --git a/src/rustup/telemetry.rs b/src/rustup/telemetry.rs
@@ -1,3 +1,4 @@
+use errors::*;
 use time;
 use rustup_utils::{raw, utils};
 use rustc_serialize::json;
@@ -39,7 +40,7 @@ impl Telemetry {
         Telemetry { telemetry_dir: telemetry_dir }
     }
 
-    pub fn log_telemetry(&self, event: TelemetryEvent) {
+    pub fn log_telemetry(&self, event: TelemetryEvent) -> Result<()> {
         let current_time = time::now_utc();
         let ln = LogMessage { log_time_s: current_time.to_timespec().sec,
                               event: event,
@@ -52,14 +53,25 @@ impl Telemetry {
         // Check for the telemetry file. If it doesn't exist, it's a new day.
         // If it is a new day, then attempt to clean the telemetry directory.
         if !raw::is_file(&self.telemetry_dir.join(&filename)) {
-            self.clean_telemetry_dir();
+            try!(self.clean_telemetry_dir());
         }
 
-        let _ = utils::append_file("telemetry", &self.telemetry_dir.join(&filename), &json);
+        let _ = utils::append_file("telemetry", 
+                                   &self.telemetry_dir.join(&filename), 
+                                   &json);
+
+        Ok(())
     }
 
-    pub fn clean_telemetry_dir(&self) {
-        let contents = self.telemetry_dir.read_dir().unwrap();
+    pub fn clean_telemetry_dir(&self) -> Result<()> {
+        let telemetry_dir_contents = self.telemetry_dir.read_dir();
+
+        let contents = match telemetry_dir_contents {
+            Ok(c) => c,
+            Err(e) => {
+                return Err(Error::TelemetryCleanupError(e));
+            }
+        };
 
         let mut telemetry_files: Vec<PathBuf> = Vec::new();
 
@@ -71,10 +83,8 @@ impl Telemetry {
             }
         }
 
-        println!("found {:?} files to delete", telemetry_files.len());
-
         if telemetry_files.len() < MAX_TELEMETRY_FILES {
-            return;
+            return Ok(());
         }
 
         let dl: usize = telemetry_files.len() - MAX_TELEMETRY_FILES;
@@ -85,7 +95,11 @@ impl Telemetry {
 
         for i in 0..dl {
             let i = i as usize;
-            let _ = fs::remove_file(&telemetry_files[i]);
+            try!(fs::remove_file(&telemetry_files[i]).map_err(|e| {
+                Error::TelemetryCleanupError(e)
+            }));
         }
+
+        Ok(())
     }
 }
diff --git a/src/rustup/toolchain.rs b/src/rustup/toolchain.rs
@@ -166,13 +166,20 @@ impl<'a> Toolchain<'a> {
             Ok(us) => {
                 let te = TelemetryEvent::ToolchainUpdate { toolchain: self.name().to_string() ,
                                                            success: true };
-                self.telemetry.log_telemetry(te);
-                Ok(us)
+                match self.telemetry.log_telemetry(te) {
+                    Ok(_) => Ok(us),
+                    Err(e) => { 
+                        self.cfg.notify_handler.call(Notification::TelemetryCleanupError(&e));
+                        Ok(us)
+                    }
+                }
             } 
             Err(e) => {
                 let te = TelemetryEvent::ToolchainUpdate { toolchain: self.name().to_string() ,
                                                            success: true };
-                self.telemetry.log_telemetry(te);
+                let _ = self.telemetry.log_telemetry(te).map_err(|xe| {
+                    self.cfg.notify_handler.call(Notification::TelemetryCleanupError(&xe));
+                });
                 Err(e)
             }
         }
@@ -399,15 +406,21 @@ impl<'a> Toolchain<'a> {
                 let te = TelemetryEvent::ToolchainUpdate { toolchain: self.name.to_owned(), 
                                                            success: true };
 
-                self.telemetry.log_telemetry(te);
-
-                Ok(())
+                match self.telemetry.log_telemetry(te) {
+                    Ok(_) => Ok(()),
+                    Err(e) => { 
+                        self.cfg.notify_handler.call(Notification::TelemetryCleanupError(&e));
+                        Ok(())
+                    }
+                }
             },
             Err(e) => {
                 let te = TelemetryEvent::ToolchainUpdate { toolchain: self.name.to_owned(), 
                                                            success: false };
 
-                self.telemetry.log_telemetry(te);
+                let _ = self.telemetry.log_telemetry(te).map_err(|xe| {
+                    self.cfg.notify_handler.call(Notification::TelemetryCleanupError(&xe));
+                });
                 Err(e)
             }
         }