fix: graceful closing of other plugins on initialization failure

Now that we parse all default expressions from plugins, we discovered
that the `activity` plugin had a malformed expression, and when the
parsing failed and `hc` exited, other plugin processes were left
orphaned. This commit makes sure we use up the JoinSet that performs
plugin initialization, thus trying to initialize all plugins instead of
stopping as soon as the first error occurs. By doing so, we prevent
orphaned processes and report all errors which makes for a better user
experience.

Signed-off-by: jlanson <[email protected]>

hipcheck/src/plugin/mod.rs

@@ -8,9 +8,9 @@ mod plugin_manifest;
 mod retrieval;
 mod types;
 
-use crate::error::Result;
 pub use crate::plugin::{get_plugin_key, manager::*, plugin_id::PluginId, types::*};
 use crate::policy_exprs::Expr;
+use crate::{error::Result, hc_error};
 pub use arch::{get_current_arch, try_set_arch, Arch};
 pub use download_manifest::{ArchiveFormat, DownloadManifest, HashAlgorithm, HashWithDigest};
 use hipcheck_common::types::{Query, QueryDirection};
@@ -19,7 +19,7 @@ pub use plugin_manifest::{
 };
 pub use retrieval::retrieve_plugins;
 use serde_json::Value;
-use std::collections::HashMap;
+use std::{collections::HashMap, ops::Not};
 use tokio::sync::Mutex;
 
 pub async fn initialize_plugins(
@@ -34,12 +34,31 @@ pub async fn initialize_plugins(
 		set.spawn(p.initialize(c));
 	}
 
-	let mut out: Vec<PluginTransport> = vec![];
+	let mut inited: Vec<PluginTransport> = vec![];
+	let mut failures: Vec<_> = vec![];
+
 	while let Some(res) = set.join_next().await {
-		out.push(res??);
+		// @Todo - what is the cleanup if the tokio func fails?
+		let init_res = res?;
+
+		// Instead of immediately erroring, we need to finish
+		// initializing all plugins so they shut down properly
+		match init_res {
+			Ok(pt) => inited.push(pt),
+			Err(e) => failures.push(e),
+		};
 	}
 
-	Ok(out)
+	if failures.is_empty().not() {
+		let mut err = "Failures occurred during plugin initialization:".to_owned();
+		for x in failures {
+			err += "\n";
+			err += x.to_string().as_str();
+		}
+		Err(hc_error!("{}", err))
+	} else {
+		Ok(inited)
+	}
 }
 
 #[derive(Debug)]

hipcheck/src/plugin/types.rs

@@ -288,10 +288,19 @@ impl PluginContext {
 
 		self.set_configuration(&config).await?.as_result()?;
 
-		let opt_default_policy_expr = self
-			.get_default_policy_expression()
-			.await?
-			.map(|s| std_parse(s.as_str()))
+		let opt_str = self.get_default_policy_expression().await?;
+		// This is where we turn the `std_parse` error into a user-facing message
+		let opt_default_policy_expr = opt_str
+			.map(|s| {
+				std_parse(s.as_str()).map_err(|e| {
+					hc_error!(
+						"Plugin '{}' has bad default policy expression '{}': {}",
+						self.plugin.name,
+						s,
+						e
+					)
+				})
+			})
 			.transpose()?;
 
 		let opt_explain_default_query = self.explain_default_query().await?;

plugins/activity/src/main.rs

@@ -76,7 +76,7 @@ impl Plugin for ActivityPlugin {
 			return Err(Error::UnspecifiedQueryState);
 		};
 
-		Ok(format!("lte $ P{}w", conf.weeks.unwrap_or(71)))
+		Ok(format!("(lte $ P{}w)", conf.weeks.unwrap_or(71)))
 	}
 
 	fn explain_default_query(&self) -> Result<Option<String>> {