feat(upgrade): better error message on failure (#25503)

Co-authored-by: crowlkats <crowlkats@toaxl.com>

6 files changed, 112 insertions, 15 deletions

diff --git a/cli/args/flags.rs b/cli/args/flags.rs
index fbf58fff2..6d2611749 100644
--- a/cli/args/flags.rs
+++ b/cli/args/flags.rs
@@ -2986,12 +2986,8 @@ The declaration file could be saved and used for typing information.",
   )
 }
 
-fn upgrade_subcommand() -> Command {
-  command(
-    "upgrade",
-    cstr!("Upgrade deno executable to the given version.
-
-<g>Latest</>
+pub static UPGRADE_USAGE: &str = cstr!(
+  "<g>Latest</>
   <bold>deno upgrade</>
 
 <g>Specific version</>
@@ -3002,7 +2998,15 @@ fn upgrade_subcommand() -> Command {
 <g>Channel</>
   <bold>deno upgrade</> <p(245)>stable</>
   <bold>deno upgrade</> <p(245)>rc</>
-  <bold>deno upgrade</> <p(245)>canary</>
+  <bold>deno upgrade</> <p(245)>canary</>"
+);
+
+fn upgrade_subcommand() -> Command {
+  command(
+    "upgrade",
+    color_print::cformat!("Upgrade deno executable to the given version.
+
+{}
 
 The version is downloaded from <p(245)>https://dl.deno.land</> and is used to replace the current executable.
 
@@ -3010,7 +3014,7 @@ If you want to not replace the current Deno executable but instead download an u
 different location, use the <c>--output</> flag:
   <p(245)>deno upgrade --output $HOME/my_deno</>
 
-<y>Read more:</> <c>https://docs.deno.com/go/cmd/upgrade</>"),
+<y>Read more:</> <c>https://docs.deno.com/go/cmd/upgrade</>", UPGRADE_USAGE),
     UnstableArgsConfig::None,
   )
   .hide(cfg!(not(feature = "upgrade")))
diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs
index 46c12ba81..826bce40a 100644
--- a/cli/tools/upgrade.rs
+++ b/cli/tools/upgrade.rs
@@ -4,6 +4,7 @@
 
 use crate::args::Flags;
 use crate::args::UpgradeFlags;
+use crate::args::UPGRADE_USAGE;
 use crate::colors;
 use crate::factory::CliFactory;
 use crate::http_util::HttpClient;
@@ -15,7 +16,6 @@ use crate::util::progress_bar::ProgressBarStyle;
 use crate::version;
 
 use async_trait::async_trait;
-use color_print::cstr;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
@@ -38,8 +38,6 @@ const RELEASE_URL: &str = "https://github.com/denoland/deno/releases";
 const CANARY_URL: &str = "https://dl.deno.land/canary";
 const RC_URL: &str = "https://dl.deno.land/release";
 
-static EXAMPLE_USAGE: &str = cstr!("Example usage:\n  <p(245)>deno upgrade | deno upgrade 1.46 | deno upgrade canary</>");
-
 pub static ARCHIVE_NAME: Lazy<String> =
   Lazy::new(|| format!("deno-{}.zip", env!("TARGET")));
 
@@ -625,6 +623,7 @@ impl RequestedVersion {
     };
     let mut maybe_passed_version = upgrade_flags.version.clone();
 
+    // TODO(bartlomieju): prefer flags first? This whole logic could be cleaned up...
     if let Some(val) = &upgrade_flags.version_or_hash_or_channel {
       if let Ok(channel) = ReleaseChannel::deserialize(&val.to_lowercase()) {
         // TODO(bartlomieju): print error if any other flags passed?
@@ -651,18 +650,19 @@ impl RequestedVersion {
     let (channel, passed_version) = if is_canary {
       if !re_hash.is_match(&passed_version) {
         bail!(
-          "Invalid commit hash passed ({})\n\n{}",
+          "Invalid commit hash passed ({})\n\nPass a semver, or a full 40 character git commit hash, or a release channel name.\n\nUsage:\n{}",
           colors::gray(passed_version),
-          EXAMPLE_USAGE
+          UPGRADE_USAGE
         );
       }
+
       (ReleaseChannel::Canary, passed_version)
     } else {
       let Ok(semver) = Version::parse_standard(&passed_version) else {
         bail!(
-          "Invalid version passed ({})\n\n{}",
+          "Invalid version passed ({})\n\nPass a semver, or a full 40 character git commit hash, or a release channel name.\n\nUsage:\n{}",
           colors::gray(passed_version),
-          EXAMPLE_USAGE
+          UPGRADE_USAGE
         );
       };
 
@@ -1123,6 +1123,8 @@ mod test {
   use std::cell::RefCell;
   use std::rc::Rc;
 
+  use test_util::assert_contains;
+
   use super::*;
 
   #[test]
@@ -1221,6 +1223,27 @@ mod test {
         "5c69b4861b52ab406e73b9cd85c254f0505cb20f".to_string()
       )
     );
+
+    upgrade_flags.version_or_hash_or_channel =
+      Some("5c69b4861b52a".to_string());
+    let err = RequestedVersion::from_upgrade_flags(upgrade_flags.clone())
+      .unwrap_err()
+      .to_string();
+    assert_contains!(err, "Invalid version passed");
+    assert_contains!(
+      err,
+      "Pass a semver, or a full 40 character git commit hash, or a release channel name."
+    );
+
+    upgrade_flags.version_or_hash_or_channel = Some("11.asd.1324".to_string());
+    let err = RequestedVersion::from_upgrade_flags(upgrade_flags.clone())
+      .unwrap_err()
+      .to_string();
+    assert_contains!(err, "Invalid version passed");
+    assert_contains!(
+      err,
+      "Pass a semver, or a full 40 character git commit hash, or a release channel name."
+    );
   }
 
   #[test]
diff --git a/tests/specs/upgrade/invalid_version/__test__.jsonc b/tests/specs/upgrade/invalid_version/__test__.jsonc
new file mode 100644
index 000000000..1fa81c107
--- /dev/null
+++ b/tests/specs/upgrade/invalid_version/__test__.jsonc
@@ -0,0 +1,19 @@
+{
+  "tests": {
+    "canary": {
+      "args": "upgrade --canary asdfasdf",
+      "output": "canary.out",
+      "exitCode": 1
+    },
+    "version": {
+      "args": "upgrade --version asdfasdf",
+      "output": "version.out",
+      "exitCode": 1
+    },
+    "shorthand": {
+      "args": "upgrade asdfasdf",
+      "output": "shorthand.out",
+      "exitCode": 1
+    }
+  }
+}
diff --git a/tests/specs/upgrade/invalid_version/canary.out b/tests/specs/upgrade/invalid_version/canary.out
new file mode 100644
index 000000000..60a7b9f09
--- /dev/null
+++ b/tests/specs/upgrade/invalid_version/canary.out
@@ -0,0 +1,17 @@
+error: Invalid commit hash passed (asdfasdf)
+
+Pass a semver, or a full 40 character git commit hash, or a release channel name.
+
+Usage:
+Latest
+  deno upgrade
+
+Specific version
+  deno upgrade 1.45.0
+  deno upgrade 1.46.0-rc.1
+  deno upgrade 9bc2dd29ad6ba334fd57a20114e367d3c04763d4
+
+Channel
+  deno upgrade stable
+  deno upgrade rc
+  deno upgrade canary
diff --git a/tests/specs/upgrade/invalid_version/shorthand.out b/tests/specs/upgrade/invalid_version/shorthand.out
new file mode 100644
index 000000000..134284cab
--- /dev/null
+++ b/tests/specs/upgrade/invalid_version/shorthand.out
@@ -0,0 +1,17 @@
+error: Invalid version passed (asdfasdf)
+
+Pass a semver, or a full 40 character git commit hash, or a release channel name.
+
+Usage:
+Latest
+  deno upgrade
+
+Specific version
+  deno upgrade 1.45.0
+  deno upgrade 1.46.0-rc.1
+  deno upgrade 9bc2dd29ad6ba334fd57a20114e367d3c04763d4
+
+Channel
+  deno upgrade stable
+  deno upgrade rc
+  deno upgrade canary
diff --git a/tests/specs/upgrade/invalid_version/version.out b/tests/specs/upgrade/invalid_version/version.out
new file mode 100644
index 000000000..134284cab
--- /dev/null
+++ b/tests/specs/upgrade/invalid_version/version.out
@@ -0,0 +1,17 @@
+error: Invalid version passed (asdfasdf)
+
+Pass a semver, or a full 40 character git commit hash, or a release channel name.
+
+Usage:
+Latest
+  deno upgrade
+
+Specific version
+  deno upgrade 1.45.0
+  deno upgrade 1.46.0-rc.1
+  deno upgrade 9bc2dd29ad6ba334fd57a20114e367d3c04763d4
+
+Channel
+  deno upgrade stable
+  deno upgrade rc
+  deno upgrade canary