From 036595b8ab0b97353b846b067d2b40ce6c4bc183 Mon Sep 17 00:00:00 2001
From: itowlson <ivan.towlson@fermyon.com>
Date: Tue, 17 Sep 2024 13:29:02 +1200
Subject: [PATCH] Less meaningless error messages

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
---
 .../src/environment_definition.rs             | 29 +++++++++++++------
 crates/environments/src/lib.rs                |  9 ++++--
 crates/environments/src/loader.rs             |  4 +--
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/crates/environments/src/environment_definition.rs b/crates/environments/src/environment_definition.rs
index 5e576b2cdd..49ea4f91b9 100644
--- a/crates/environments/src/environment_definition.rs
+++ b/crates/environments/src/environment_definition.rs
@@ -3,25 +3,33 @@ use anyhow::Context;
 pub async fn load_environment(env_id: &str) -> anyhow::Result<TargetEnvironment> {
     use futures_util::TryStreamExt;
 
-    let (pkg_name, pkg_ver) = env_id.split_once('@').unwrap();
+    let (pkg_name, pkg_ver) = env_id.split_once('@').with_context(|| format!("Failed to parse target environment {env_id} as package reference - is the target correct?"))?;
 
-    let mut client = wasm_pkg_loader::Client::with_global_defaults()?;
+    // TODO: this requires wkg configuration which shouldn't be on users:
+    // is there a better way to handle it?
+    let mut client = wasm_pkg_loader::Client::with_global_defaults()
+        .context("Failed to create a package loader from your global settings")?;
 
-    let package = pkg_name.to_owned().try_into().context("pkg ref parse")?;
-    let version = wasm_pkg_loader::Version::parse(pkg_ver).context("pkg ver parse")?;
+    let package = pkg_name
+        .to_owned()
+        .try_into()
+        .with_context(|| format!("Failed to parse environment name {pkg_name} as package name"))?;
+    let version = wasm_pkg_loader::Version::parse(pkg_ver).with_context(|| {
+        format!("Failed to parse environment version {pkg_ver} as package version")
+    })?;
 
     let release = client
         .get_release(&package, &version)
         .await
-        .context("get release")?;
+        .with_context(|| format!("Failed to get {env_id} release from registry"))?;
     let stm = client
         .stream_content(&package, &release)
         .await
-        .context("stream content")?;
+        .with_context(|| format!("Failed to get {env_id} package from registry"))?;
     let bytes = stm
         .try_collect::<bytes::BytesMut>()
         .await
-        .context("collect stm")?
+        .with_context(|| format!("Failed to get {env_id} package data from registry"))?
         .to_vec();
 
     TargetEnvironment::new(env_id.to_owned(), bytes)
@@ -37,13 +45,16 @@ pub struct TargetEnvironment {
 
 impl TargetEnvironment {
     fn new(name: String, bytes: Vec<u8>) -> anyhow::Result<Self> {
-        let decoded = wit_component::decode(&bytes).context("decode wasm")?;
+        let decoded = wit_component::decode(&bytes)
+            .with_context(|| format!("Failed to decode package for environment {name}"))?;
         let package_id = decoded.package();
         let package = decoded
             .resolve()
             .packages
             .get(package_id)
-            .context("should had a package")?
+            .with_context(|| {
+                format!("The {name} environment is invalid (no package for decoded package ID)")
+            })?
             .clone();
 
         Ok(Self {
diff --git a/crates/environments/src/lib.rs b/crates/environments/src/lib.rs
index 2997d96417..6771c3a6ec 100644
--- a/crates/environments/src/lib.rs
+++ b/crates/environments/src/lib.rs
@@ -1,4 +1,4 @@
-use anyhow::anyhow;
+use anyhow::{anyhow, Context};
 
 mod environment_definition;
 mod loader;
@@ -36,7 +36,9 @@ async fn validate_application_against_environments(
         load_and_resolve_all(app, ts, resolution_context)
             .map(|css| css.map(|css| (ty.to_owned(), css)))
     });
-    let components_by_trigger_type = join_all_result(components_by_trigger_type_futs).await?;
+    let components_by_trigger_type = join_all_result(components_by_trigger_type_futs)
+        .await
+        .context("Failed to prepare components for target environment checking")?;
 
     for (trigger_type, component) in components_by_trigger_type {
         for component in &component {
@@ -126,7 +128,8 @@ async fn validate_wasm_against_world(
     "#
     );
 
-    let doc = wac_parser::Document::parse(&wac_text)?;
+    let doc = wac_parser::Document::parse(&wac_text)
+        .context("Internal error constructing WAC document for target checking")?;
 
     // TODO: if we end up needing the registry, we need to do this dance
     // for things we are providing separately, or the registry will try to
diff --git a/crates/environments/src/loader.rs b/crates/environments/src/loader.rs
index 0cd229bcd7..b97d2d9d46 100644
--- a/crates/environments/src/loader.rs
+++ b/crates/environments/src/loader.rs
@@ -1,6 +1,6 @@
 use std::path::Path;
 
-use anyhow::anyhow;
+use anyhow::{anyhow, Context};
 use spin_common::ui::quoted_path;
 
 pub(crate) struct ComponentToValidate<'a> {
@@ -73,7 +73,7 @@ async fn load_and_resolve_one<'a>(
 
     let loader = ComponentSourceLoader::new(resolution_context.wasm_loader());
 
-    let wasm = spin_compose::compose(&loader, &component).await?;
+    let wasm = spin_compose::compose(&loader, &component).await.with_context(|| format!("Spin needed to compose dependencies for {id} as part of target checking, but composition failed"))?;
 
     Ok(ComponentToValidate {
         id,