Skip to content

Commit

Permalink
feat: Improve error reporting from typo and review
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
alilleybrinker committed Nov 8, 2024
1 parent 468e99a commit 8d18911
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 30 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions plugins/review/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 3 additions & 7 deletions plugins/review/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -38,6 +39,7 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result<Vec<bool>> {
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);
Expand All @@ -47,13 +49,7 @@ async fn review(engine: &mut PluginEngine, value: Target) -> Result<Vec<bool>> {
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<PullRequest> =
serde_json::from_value(value).map_err(Error::InvalidJsonInQueryOutput)?;
Expand Down
30 changes: 11 additions & 19 deletions plugins/typo/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,34 +67,25 @@ impl TryFrom<RawConfig> for Config {
async fn typo(engine: &mut PluginEngine, value: Target) -> Result<Vec<bool>> {
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
Expand Down
3 changes: 2 additions & 1 deletion sdk/rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
24 changes: 23 additions & 1 deletion sdk/rust/src/error.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -59,8 +59,30 @@ pub enum Error {

#[error("invalid format for QueryTarget")]
InvalidQueryTargetFormat,

#[error(transparent)]
Unspecified { source: DynError },
}

impl From<anyhow::Error> for Error {
fn from(value: anyhow::Error) -> Self {
Error::Unspecified {
source: value.into(),
}
}
}

impl Error {
pub fn any<E: StdError + 'static + Send + Sync>(source: E) -> Self {
Error::Unspecified {
source: Box::new(source),
}
}
}

/// A thread-safe error trait object.
pub type DynError = Box<dyn StdError + 'static + Send + Sync>;

// this will never happen, but is needed to enable passing QueryTarget to PluginEngine::query
impl From<Infallible> for Error {
fn from(_value: Infallible) -> Self {
Expand Down

0 comments on commit 8d18911

Please sign in to comment.