address PR feedback

Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
vercel · Nov 30, 2022 · 9b4ef7c · 9b4ef7c
1 parent 7b7c64c
commit 9b4ef7c
Show file tree

Hide file tree

Showing 8 changed files with 218 additions and 206 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/update-notifier/Cargo.toml b/crates/update-notifier/Cargo.toml
@@ -2,14 +2,12 @@
 name = "update-notifier"
 version = "0.1.0"
 edition = "2021"
-description = "Output update notification to the console when new version on npm package is available"
+description = "Display update notification when new version of npm package is available"
 license = "MPL-2.0"
 publish = false
 
-
 [dependencies]
 colored            = "2.0"
-env_logger         = "0.10"
 log                = "0.4"
 reqwest            = { version = "0.11", features = ["json"] }
 semver             = "1.0"

diff --git a/crates/update-notifier/src/fetch.rs b/crates/update-notifier/src/fetch.rs
@@ -12,31 +12,20 @@ struct NpmVersionData {
 
 const REGISTRY_URL: &str = "https://registry.npmjs.org";
 const DEFAULT_TAG: &str = "latest";
-const TIMEOUT: Duration = Duration::from_millis(800);
+const DEFAULT_TIMEOUT: Duration = Duration::from_millis(800);
 
 pub async fn get_latest_version(
     package: &str,
     tag: Option<&str>,
     timeout: Option<Duration>,
 ) -> Result<String, UpdateNotifierError> {
     log::debug!("fetching latest version");
-    let tag = tag.unwrap_or_else(|| DEFAULT_TAG);
-    let timeout = timeout.unwrap_or_else(|| TIMEOUT);
+    let tag = tag.unwrap_or(DEFAULT_TAG);
+    let timeout = timeout.unwrap_or(DEFAULT_TIMEOUT);
     let client: Client = reqwest::Client::new();
     let url = format!("{}/{}/{}", REGISTRY_URL, package, tag);
     log::debug!("fetching {:?}", url);
-    let resp = client.get(url).timeout(timeout).send().await;
-    match resp {
-        Ok(r) => {
-            let json_result = r.json::<NpmVersionData>().await;
-            match json_result {
-                Ok(v) => Ok(v.version.to_string()),
-                Err(err) => Err(UpdateNotifierError::FetchError(err)),
-            }
-        }
-        Err(err) => {
-            log::error!("failed to fetch latest version {:?}", err);
-            Err(UpdateNotifierError::FetchError(err))
-        }
-    }
+    let resp = client.get(url).timeout(timeout).send().await?;
+    let json_result = resp.json::<NpmVersionData>().await?;
+    Ok(json_result.version.to_string())
 }
diff --git a/crates/update-notifier/src/lib.rs b/crates/update-notifier/src/lib.rs
@@ -13,7 +13,7 @@ mod ui;
 mod utils;
 
 // default interval to check for new updates (one per day)
-const INTERVAL: Duration = Duration::from_secs(60 * 60 * 24);
+const DEFAULT_INTERVAL: Duration = Duration::from_secs(60 * 60 * 24);
 
 #[derive(Debug)]
 pub struct UpdateNotifier {
@@ -39,6 +39,10 @@ pub enum UpdateNotifierError {
     ParseError(#[from] SemVerError),
     #[error("Failed to parse JSON")]
     JsonError(#[from] SerdeError),
+    #[error("Failed to write to terminal")]
+    DisplayLengthError(#[from] ui::utils::GetDisplayLengthError),
+    #[error("Unknown Error")]
+    Internal(),
 }
 
 impl UpdateNotifier {
@@ -66,36 +70,37 @@ impl UpdateNotifier {
         }
     }
 
-    fn update_message(&self) {
+    fn update_message(&self) -> Result<(), UpdateNotifierError> {
         let turbo = "@turborepo";
         let turbo_gradient = turbo.gradient([RGB::new(0, 153, 247), RGB::new(241, 23, 18)]);
 
         let latest_version = match &self.config.latest_version {
             Some(v) => v,
             None => {
                 log::error!("no latest version found in local config");
-                return;
+                return Err(UpdateNotifierError::Internal());
             }
         };
 
         let msg = format!(
             "
             Update available {} ≫ {}
-            Changelog: https://github.com/vercel/turbo/releases/tag/v{}Run \
-             \"{}\" to update
+            Changelog: {}/releases/tag/v{}
+            Run \"{}\" to update
 
             Follow {} for updates: {}
             ",
             &self.config.current_version.dimmed(),
             &latest_version.green().bold(),
+            "https://github.com/vercel/turbo",
             &latest_version,
             // TODO: make this package manager aware
             "npm i -g turbo".cyan().bold(),
             turbo_gradient,
             "https://twitter.com/turborepo",
         );
 
-        ui::rectangle(&msg);
+        return ui::message(&msg);
     }
 
     fn first_run(package: String, tag: Option<String>, interval: Option<Duration>) -> Self {
@@ -106,7 +111,7 @@ impl UpdateNotifier {
                 latest_version: None,
                 last_checked: None,
             },
-            interval: interval.unwrap_or(INTERVAL),
+            interval: interval.unwrap_or(DEFAULT_INTERVAL),
             package,
             tag,
         }
@@ -121,7 +126,7 @@ impl UpdateNotifier {
             let file = match file {
                 Ok(f) => f,
                 Err(_) => {
-                    log::debug!("failed to open local config, writing first version");
+                    log::debug!("failed to read local config, writing first version");
                     return Self::first_run(package, tag, interval);
                 }
             };
@@ -138,7 +143,7 @@ impl UpdateNotifier {
                         },
                         package,
                         tag,
-                        interval: interval.unwrap_or(INTERVAL),
+                        interval: interval.unwrap_or(DEFAULT_INTERVAL),
                     }
                 }
                 Err(_) => {
@@ -173,15 +178,7 @@ impl UpdateNotifier {
     #[tokio::main]
     async fn update(&mut self) -> Result<(), UpdateNotifierError> {
         let latest_version =
-            fetch::get_latest_version(&self.package, self.tag.as_deref(), None).await;
-        let latest_version = match latest_version {
-            Ok(v) => v,
-            Err(err) => {
-                log::debug!("failed to fetch latest version {:?}", err);
-                return Err(err);
-            }
-        };
-
+            fetch::get_latest_version(&self.package, self.tag.as_deref(), None).await?;
         let current_version = String::from(utils::get_version());
         let now = utils::ms_since_epoch();
 
@@ -207,7 +204,7 @@ impl UpdateNotifier {
             log::debug!("checking if {} > {}", latest_version, current_version);
             if latest_version > current_version {
                 log::debug!("update available");
-                self.update_message();
+                self.update_message()?;
                 return Ok(());
             } else {
                 log::debug!("no update available");