From 8d18911a9a57233cf4b9c2a151210fa8ef41ddbe Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Thu, 7 Nov 2024 16:51:40 -0800 Subject: [PATCH] feat: Improve error reporting from typo and review This commit modifies the error reporting type in the SDK to enable more expressive errors from plugins, and updates 'typo' and 'review' to use it. Signed-off-by: Andrew Lilley Brinker --- Cargo.lock | 5 +++-- plugins/review/Cargo.toml | 1 + plugins/review/src/main.rs | 10 +++------- plugins/typo/src/main.rs | 30 +++++++++++------------------- sdk/rust/Cargo.toml | 3 ++- sdk/rust/src/error.rs | 24 +++++++++++++++++++++++- 6 files changed, 43 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0da07d2b..18d62932 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -158,9 +158,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.91" +version = "1.0.93" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c042108f3ed77fd83760a5fd79b53be043192bb3b9dba91d8c574c0ada7850c8" +checksum = "4c95c10ba0b00a02636238b814946408b1322d5ac4760326e6fb8ec956d85775" [[package]] name = "arbitrary" @@ -2594,6 +2594,7 @@ dependencies = [ name = "review" version = "0.1.0" dependencies = [ + "anyhow", "clap", "hipcheck-sdk", "log", diff --git a/plugins/review/Cargo.toml b/plugins/review/Cargo.toml index 45bcd05e..188783fc 100644 --- a/plugins/review/Cargo.toml +++ b/plugins/review/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" publish = false [dependencies] +anyhow = "1.0.93" clap = { version = "4.5.18", features = ["derive"] } hipcheck-sdk = { path = "../../sdk/rust", features = ["macros"] } log = "0.4.22" diff --git a/plugins/review/src/main.rs b/plugins/review/src/main.rs index 702d10d9..005e74bd 100644 --- a/plugins/review/src/main.rs +++ b/plugins/review/src/main.rs @@ -2,6 +2,7 @@ //! Plugin for querying what percentage of pull requests were merged without review +use anyhow::Context as _; use clap::Parser; use hipcheck_sdk::{prelude::*, types::Target}; use schemars::JsonSchema; @@ -38,6 +39,7 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result> { log::error!("target repository does not have a remote repository URL"); return Err(Error::UnexpectedPluginQueryInputFormat); }; + let Some(known_remote) = remote.known_remote else { log::error!("target repository is not a GitHub repository or else is missing GitHub repo information"); return Err(Error::UnexpectedPluginQueryInputFormat); @@ -47,13 +49,7 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result> { let value = engine .query("mitre/github/pr_reviews", known_remote) .await - .map_err(|e| { - log::error!( - "failed to get pull request reviews from GitHub for review query: {}", - e - ); - Error::UnspecifiedQueryState - })?; + .context("failed to get pull request reviews from GitHub")?; let pull_requests: Vec = serde_json::from_value(value).map_err(Error::InvalidJsonInQueryOutput)?; diff --git a/plugins/typo/src/main.rs b/plugins/typo/src/main.rs index e43b430d..a8617d7d 100644 --- a/plugins/typo/src/main.rs +++ b/plugins/typo/src/main.rs @@ -11,6 +11,7 @@ use crate::{ languages::TypoFile, types::{Lang, NpmDependencies}, }; +use anyhow::{anyhow, Context as _}; use clap::Parser; use hipcheck_sdk::{prelude::*, types::Target}; use serde::Deserialize; @@ -66,34 +67,25 @@ impl TryFrom for Config { async fn typo(engine: &mut PluginEngine, value: Target) -> Result> { log::debug!("running typo query"); - let local = value.local; - - // Get the typo file - let typo_file = TYPOFILE.get().ok_or(Error::UnspecifiedQueryState)?; + // Get the typo file. + let typo_file = TYPOFILE + .get() + .ok_or_else(|| anyhow!("could not find typo file"))?; // Get the repo's dependencies let value = engine - .query("mitre/npm/dependencies", local) + .query("mitre/npm/dependencies", value.local) .await - .map_err(|e| { - log::error!("failed to get dependencies for typo query: {}", e); - Error::UnspecifiedQueryState - })?; + .context("failed to get dependencies")?; + let dependencies: NpmDependencies = serde_json::from_value(value).map_err(Error::InvalidJsonInQueryOutput)?; // Get the dependencies with identified typos let typo_deps = match dependencies.language { - Lang::JavaScript => languages::typos_for_javascript(typo_file, dependencies.clone()) - .map_err(|e| { - log::error!("{}", e); - Error::UnspecifiedQueryState - }), - Lang::Unknown => { - log::error!("failed to identify a known language"); - Err(Error::UnexpectedPluginQueryInputFormat) - } - }?; + Lang::JavaScript => languages::typos_for_javascript(typo_file, dependencies.clone())?, + Lang::Unknown => Err(anyhow!("failed to identify a known language"))?, + }; // Generate a boolean list of depedencies with and without typos let typos = dependencies diff --git a/sdk/rust/Cargo.toml b/sdk/rust/Cargo.toml index 12eae6c1..b2ee24d9 100644 --- a/sdk/rust/Cargo.toml +++ b/sdk/rust/Cargo.toml @@ -5,10 +5,11 @@ version = "0.1.0" edition = "2021" [dependencies] +anyhow = "1.0.93" thiserror = "1.0.65" futures = "0.3.31" indexmap = "2.6.0" -jiff = { version = "0.1.13", features=["serde"]} +jiff = { version = "0.1.13", features = ["serde"] } prost = "0.13.3" rand = "0.8.5" serde = { version = "1.0.214", features = ["derive"] } diff --git a/sdk/rust/src/error.rs b/sdk/rust/src/error.rs index ab11a27b..f7b10fb0 100644 --- a/sdk/rust/src/error.rs +++ b/sdk/rust/src/error.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::proto::{ConfigurationStatus, InitiateQueryProtocolResponse, SetConfigurationResponse}; -use std::{convert::Infallible, ops::Not, result::Result as StdResult}; +use std::{convert::Infallible, error::Error as StdError, ops::Not, result::Result as StdResult}; use tokio::sync::mpsc::error::SendError as TokioMpscSendError; use tonic::Status as TonicStatus; @@ -59,8 +59,30 @@ pub enum Error { #[error("invalid format for QueryTarget")] InvalidQueryTargetFormat, + + #[error(transparent)] + Unspecified { source: DynError }, +} + +impl From for Error { + fn from(value: anyhow::Error) -> Self { + Error::Unspecified { + source: value.into(), + } + } } +impl Error { + pub fn any(source: E) -> Self { + Error::Unspecified { + source: Box::new(source), + } + } +} + +/// A thread-safe error trait object. +pub type DynError = Box; + // this will never happen, but is needed to enable passing QueryTarget to PluginEngine::query impl From for Error { fn from(_value: Infallible) -> Self {