From 25d107185b2adeb30fa6d9a7728db302375fcf4f Mon Sep 17 00:00:00 2001 From: Nilirad Date: Tue, 8 Sep 2026 10:30:39 +0200 Subject: [PATCH] Rehome error types to their layer --- AGENTS.md | 2 +- src/domain/accept_header.rs | 2 +- src/domain/api_version.rs | 2 +- src/domain/branch_name.rs | 2 +- src/domain/commit_hash.rs | 2 +- src/domain/event_type.rs | 2 +- src/domain/mod.rs | 2 + src/domain/non_empty_string.rs | 2 +- src/domain/repo_url.rs | 2 +- src/domain/target_repo.rs | 2 +- src/domain/validation_error.rs | 11 +++ src/error.rs | 126 +-------------------------------- src/http.rs | 1 + src/http/error.rs | 45 ++++++++++++ src/http/handler.rs | 2 +- src/http/handler/create.rs | 2 +- src/http/handler/delete.rs | 2 +- src/http/handler/get.rs | 2 +- src/http/handler/list.rs | 2 +- src/http/handler/update.rs | 2 +- src/polling/branch.rs | 6 +- src/polling/error.rs | 71 +++++++++++++++++++ src/polling/git.rs | 2 +- src/polling/mod.rs | 3 +- src/test_utils.rs | 2 +- 25 files changed, 155 insertions(+), 144 deletions(-) create mode 100644 src/domain/validation_error.rs create mode 100644 src/http/error.rs diff --git a/AGENTS.md b/AGENTS.md index 58dc090..e3fad9b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ This file contains crucial context for AI agents working in this repository. - **Execution Flow**: `src/main.rs` initializes an `axum` router and spawns two decoupled background `tokio` tasks: 1. `polling/`: Periodically checks remote git repositories for updates. 2. `trigger/`: Receives update events from the polling engine via `mpsc` channels and triggers GitHub Action workflows on target repositories. -- **Error Handling**: Use domain-specific error enums (`HandlerError`, `FatalError`) defined in `src/error.rs` using the `thiserror` crate. Ensure `IntoResponse` is implemented for any errors that bubble up to Axum handlers. +- **Error Handling**: Use `thiserror` for domain-specific error enums, defined in the layer they belong to: `HandlerError` in `src/http/error.rs`, repository errors in `src/repository/error.rs`, engine errors in `src/{polling,trigger}/error.rs`, and value-validation errors in `src/domain/`. Boot-time errors (`FatalError`, `SetupError`) live in `src/error.rs`. Implement `IntoResponse` for any error that bubbles up to an Axum handler. ## Reviews diff --git a/src/domain/accept_header.rs b/src/domain/accept_header.rs index cff1739..9267013 100644 --- a/src/domain/accept_header.rs +++ b/src/domain/accept_header.rs @@ -1,6 +1,6 @@ //! Domain type to represent an HTTP Accept header. -use crate::error::ValidationError; +use crate::domain::ValidationError; use http::header::HeaderValue; use serde::{Deserialize, Serialize}; diff --git a/src/domain/api_version.rs b/src/domain/api_version.rs index 13a917d..c3b1d3f 100644 --- a/src/domain/api_version.rs +++ b/src/domain/api_version.rs @@ -1,6 +1,6 @@ //! Domain type to represent a GitHub API version in YYYY-MM-DD format. -use crate::error::ValidationError; +use crate::domain::ValidationError; use chrono::NaiveDate; use serde::{Deserialize, Serialize}; diff --git a/src/domain/branch_name.rs b/src/domain/branch_name.rs index e8770cc..ca200d0 100644 --- a/src/domain/branch_name.rs +++ b/src/domain/branch_name.rs @@ -1,6 +1,6 @@ //! Domain type to represent a Git branch name. -use crate::error::ValidationError; +use crate::domain::ValidationError; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use validator::Validate; diff --git a/src/domain/commit_hash.rs b/src/domain/commit_hash.rs index 115baa9..6a9481d 100644 --- a/src/domain/commit_hash.rs +++ b/src/domain/commit_hash.rs @@ -1,6 +1,6 @@ //! Domain type to represent a git commit hash. -use crate::error::ValidationError; +use crate::domain::ValidationError; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/src/domain/event_type.rs b/src/domain/event_type.rs index 5e9b6a5..015293d 100644 --- a/src/domain/event_type.rs +++ b/src/domain/event_type.rs @@ -1,6 +1,6 @@ //! Domain type to represent a GitHub's `repository_dispatch` `event_type`. -use crate::error::ValidationError; +use crate::domain::ValidationError; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use validator::Validate; diff --git a/src/domain/mod.rs b/src/domain/mod.rs index 22fc34f..39fbf95 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -8,6 +8,7 @@ pub mod event_type; pub mod non_empty_string; pub mod repo_url; pub mod target_repo; +pub mod validation_error; pub use accept_header::AcceptHeader; pub use api_version::ApiVersion; @@ -17,6 +18,7 @@ pub use event_type::EventType; pub use non_empty_string::NonEmptyString; pub use repo_url::RepoUrl; pub use target_repo::TargetRepo; +pub use validation_error::ValidationError; /// Derives `sqlx` trait implementations for a type that implements `TryFrom`. /// diff --git a/src/domain/non_empty_string.rs b/src/domain/non_empty_string.rs index 5946389..0c049c6 100644 --- a/src/domain/non_empty_string.rs +++ b/src/domain/non_empty_string.rs @@ -1,6 +1,6 @@ //! Domain type to represent a non-empty string. -use crate::error::ValidationError; +use crate::domain::ValidationError; use serde::{Deserialize, Serialize}; use validator::Validate; diff --git a/src/domain/repo_url.rs b/src/domain/repo_url.rs index 408b60c..7063684 100644 --- a/src/domain/repo_url.rs +++ b/src/domain/repo_url.rs @@ -1,6 +1,6 @@ //! Domain type to represent a GitHub repository URL. -use crate::error::ValidationError; +use crate::domain::ValidationError; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use validator::Validate; diff --git a/src/domain/target_repo.rs b/src/domain/target_repo.rs index 771a648..357ab12 100644 --- a/src/domain/target_repo.rs +++ b/src/domain/target_repo.rs @@ -1,6 +1,6 @@ //! Domain type to represent a target repository hosted on GitHub. -use crate::error::ValidationError; +use crate::domain::ValidationError; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/src/domain/validation_error.rs b/src/domain/validation_error.rs new file mode 100644 index 0000000..d8b41ac --- /dev/null +++ b/src/domain/validation_error.rs @@ -0,0 +1,11 @@ +//! Validation error type for domain values. + +use thiserror::Error; + +/// Validation error. +#[derive(Debug, Error)] +pub enum ValidationError { + /// Invalid field value. + #[error("Validation error: {0}")] + InvalidValue(String), +} diff --git a/src/error.rs b/src/error.rs index eac57af..43fc291 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,64 +1,10 @@ -//! Definitions for common error types. +//! Definitions for fatal and setup errors. use crate::repository::RepositoryError; -use axum::{ - http::StatusCode, - response::{IntoResponse, Response}, -}; use config::ConfigError; -use rovo::aide::OperationOutput; use thiserror::Error; use validator::ValidationErrors; -/// Validation error. -#[derive(Debug, Error)] -pub enum ValidationError { - /// Invalid field value. - #[error("Validation error: {0}")] - InvalidValue(String), -} - -impl IntoResponse for ValidationError { - fn into_response(self) -> Response { - (StatusCode::UNPROCESSABLE_ENTITY, self.to_string()).into_response() - } -} - -/// An error happened inside an Axum handler. -#[derive(Debug, Error)] -pub enum HandlerError { - /// Database query execution failure. - #[error("Repository Error: {0}")] - DbQuery(RepositoryError), - - /// Requested resource not found. - #[error("Not Found")] - NotFound, -} - -impl From for HandlerError { - fn from(err: RepositoryError) -> Self { - match err { - RepositoryError::NotFound => HandlerError::NotFound, - other => HandlerError::DbQuery(other), - } - } -} - -impl OperationOutput for HandlerError { - type Inner = (); -} - -impl IntoResponse for HandlerError { - fn into_response(self) -> Response { - let status = match self { - HandlerError::DbQuery(_) => StatusCode::INTERNAL_SERVER_ERROR, - HandlerError::NotFound => StatusCode::NOT_FOUND, - }; - (status, self.to_string()).into_response() - } -} - /// An error that requires the server to be shut down. #[derive(Debug, Error)] pub enum FatalError { @@ -147,73 +93,3 @@ impl From for FatalError { #[derive(Debug, Error)] #[error(transparent)] pub struct ClientCreationError(#[from] reqwest::Error); - -/// Error in retrieving a commit or its info using `git ls-remote`. -#[derive(Debug, Error)] -pub enum CommitHashError { - /// Validation error. - #[error("Validation error: {0}")] - Validation(#[from] ValidationError), - - /// I/O error while spawning the process. - #[error("I/O error in `git ls-remote`: {0}")] - Io(#[from] std::io::Error), - - /// Unexpected exit status. - #[error("Unexpected `git ls-remote` exit status: {0}")] - UnexpectedStatus(String), - - /// Unexpected output format. - #[error( - "Unexpected `git ls-remote` output format. Repo: {repo_url}; Branch: {branch}; Stdout: {stdout}" - )] - UnexpectedOutput { - /// The process output text. - stdout: String, - /// The relevant git repository URL. - repo_url: String, - /// The relevant git branch. - branch: String, - }, - - /// Failed to find remote. - #[error("Failed to find remote: {0}")] - // Error is boxed because it is very large - RemoteAt(Box), - - /// Failed to connect to remote. - #[error("Failed to connect to remote: {0}")] - // Error is boxed because it is very large - Connect(Box), - - /// Failed to map refs. - #[error("Failed to map refs: {0}")] - // Error is boxed because it is very large - RefMap(Box), - - /// Failed to parse refspec. - #[error("Failed to parse refspec: {0}")] - RefSpecParse(#[from] gix::refspec::parse::Error), - - /// Git operation failed using gix. - #[error("Git operation failed: {0}")] - Git(String), -} - -impl From for CommitHashError { - fn from(e: gix::remote::init::Error) -> Self { - CommitHashError::RemoteAt(Box::new(e)) - } -} - -impl From for CommitHashError { - fn from(e: gix::remote::connect::Error) -> Self { - CommitHashError::Connect(Box::new(e)) - } -} - -impl From for CommitHashError { - fn from(e: gix::remote::ref_map::Error) -> Self { - CommitHashError::RefMap(Box::new(e)) - } -} diff --git a/src/http.rs b/src/http.rs index 404d2af..6593168 100644 --- a/src/http.rs +++ b/src/http.rs @@ -1,5 +1,6 @@ //! HTTP networking: router wiring, server runtime, and the outbound HTTP client. +pub mod error; pub mod handler; pub mod router; pub(crate) mod server; diff --git a/src/http/error.rs b/src/http/error.rs new file mode 100644 index 0000000..d94c26a --- /dev/null +++ b/src/http/error.rs @@ -0,0 +1,45 @@ +//! Error types for the HTTP layer. + +use axum::{ + http::StatusCode, + response::{IntoResponse, Response}, +}; +use rovo::aide::OperationOutput; +use thiserror::Error; + +use crate::repository::RepositoryError; + +/// An error happened inside an Axum handler. +#[derive(Debug, Error)] +pub enum HandlerError { + /// Database query execution failure. + #[error("Repository Error: {0}")] + DbQuery(RepositoryError), + + /// Requested resource not found. + #[error("Not Found")] + NotFound, +} + +impl From for HandlerError { + fn from(err: RepositoryError) -> Self { + match err { + RepositoryError::NotFound => HandlerError::NotFound, + other => HandlerError::DbQuery(other), + } + } +} + +impl OperationOutput for HandlerError { + type Inner = (); +} + +impl IntoResponse for HandlerError { + fn into_response(self) -> Response { + let status = match self { + HandlerError::DbQuery(_) => StatusCode::INTERNAL_SERVER_ERROR, + HandlerError::NotFound => StatusCode::NOT_FOUND, + }; + (status, self.to_string()).into_response() + } +} diff --git a/src/http/handler.rs b/src/http/handler.rs index 8843e66..0eddaf7 100644 --- a/src/http/handler.rs +++ b/src/http/handler.rs @@ -55,7 +55,7 @@ mod tests { use super::list::{ListSubscriptionsQuery, list_subscriptions_inner}; use super::update::update_subscription_inner; use crate::domain::{BranchName, EventType, RepoUrl, TargetRepo}; - use crate::error::HandlerError; + use crate::http::error::HandlerError; use crate::http::state::AppState; use crate::model::{CreateSubscription, UpdateSubscription}; use crate::test_utils::create_test_db; diff --git a/src/http/handler/create.rs b/src/http/handler/create.rs index 9eef579..0b515f1 100644 --- a/src/http/handler/create.rs +++ b/src/http/handler/create.rs @@ -1,7 +1,7 @@ //! Create a new subscription handler. use super::map_to_hal; -use crate::error::HandlerError; +use crate::http::error::HandlerError; use crate::http::state::AppState; use crate::model::{CreateSubscription, SubscriptionHal}; use crate::repository::subscription::SubscriptionRepository; diff --git a/src/http/handler/delete.rs b/src/http/handler/delete.rs index 78f6da0..f8031f0 100644 --- a/src/http/handler/delete.rs +++ b/src/http/handler/delete.rs @@ -1,6 +1,6 @@ //! Delete a subscription handler. -use crate::error::HandlerError; +use crate::http::error::HandlerError; use crate::http::state::AppState; use crate::repository::subscription::SubscriptionRepository; use axum::extract::{Path, State}; diff --git a/src/http/handler/get.rs b/src/http/handler/get.rs index 9872d14..2e6fe56 100644 --- a/src/http/handler/get.rs +++ b/src/http/handler/get.rs @@ -1,7 +1,7 @@ //! Get a single subscription handler. use super::map_to_hal; -use crate::error::HandlerError; +use crate::http::error::HandlerError; use crate::http::state::AppState; use crate::model::SubscriptionHal; use crate::repository::subscription::SubscriptionRepository; diff --git a/src/http/handler/list.rs b/src/http/handler/list.rs index b217854..898d9d7 100644 --- a/src/http/handler/list.rs +++ b/src/http/handler/list.rs @@ -1,7 +1,7 @@ //! List subscriptions handler. use super::map_to_hal; -use crate::error::HandlerError; +use crate::http::error::HandlerError; use crate::http::state::AppState; use crate::model::{HalLink, SubscriptionHal, SubscriptionPage, SubscriptionPageLinks}; use crate::repository::subscription::SubscriptionRepository; diff --git a/src/http/handler/update.rs b/src/http/handler/update.rs index 8a5ab67..6e627d8 100644 --- a/src/http/handler/update.rs +++ b/src/http/handler/update.rs @@ -1,7 +1,7 @@ //! Update an existing subscription handler. use super::map_to_hal; -use crate::error::HandlerError; +use crate::http::error::HandlerError; use crate::http::state::AppState; use crate::model::{SubscriptionHal, UpdateSubscription}; use crate::repository::subscription::SubscriptionRepository; diff --git a/src/polling/branch.rs b/src/polling/branch.rs index cd11ff1..5d539ff 100644 --- a/src/polling/branch.rs +++ b/src/polling/branch.rs @@ -1,6 +1,10 @@ //! Utilities for checking whether a branch has updated. -use crate::{domain::CommitHash, error::CommitHashError, model::Branch, polling::git::GitFetcher}; +use crate::{ + domain::CommitHash, + model::Branch, + polling::{CommitHashError, git::GitFetcher}, +}; /// Enables comparison between a git branch row, and the newly fetched branch. pub(super) struct BranchInfo { diff --git a/src/polling/error.rs b/src/polling/error.rs index 22cb58c..039f276 100644 --- a/src/polling/error.rs +++ b/src/polling/error.rs @@ -1,6 +1,7 @@ //! Handling for errors specific to the polling engine. use crate::context::SharedContext; +use crate::domain::ValidationError; use crate::repository::RepositoryError; use thiserror::Error; @@ -64,3 +65,73 @@ async fn handle_sqlx_error(error: sqlx::Error, ctx: &SharedContext) { } } } + +/// Error in fetching the latest commit hash of a remote branch. +#[derive(Debug, Error)] +pub enum CommitHashError { + /// Validation error. + #[error("Validation error: {0}")] + Validation(#[from] ValidationError), + + /// I/O error. + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), + + /// Unexpected failure while fetching the commit hash. + #[error("Unexpected failure while fetching the commit hash: {0}")] + UnexpectedStatus(String), + + /// Unexpected output while fetching the commit hash. + #[error( + "Unexpected output while fetching the commit hash. Repo: {repo_url}; Branch: {branch}; Output: {stdout}" + )] + UnexpectedOutput { + /// The unexpected output text. + stdout: String, + /// The relevant git repository URL. + repo_url: String, + /// The relevant git branch. + branch: String, + }, + + /// Failed to find remote. + #[error("Failed to find remote: {0}")] + // Error is boxed because it is very large + RemoteAt(Box), + + /// Failed to connect to remote. + #[error("Failed to connect to remote: {0}")] + // Error is boxed because it is very large + Connect(Box), + + /// Failed to map refs. + #[error("Failed to map refs: {0}")] + // Error is boxed because it is very large + RefMap(Box), + + /// Failed to parse refspec. + #[error("Failed to parse refspec: {0}")] + RefSpecParse(#[from] gix::refspec::parse::Error), + + /// Git operation failed using gix. + #[error("Git operation failed: {0}")] + Git(String), +} + +impl From for CommitHashError { + fn from(e: gix::remote::init::Error) -> Self { + CommitHashError::RemoteAt(Box::new(e)) + } +} + +impl From for CommitHashError { + fn from(e: gix::remote::connect::Error) -> Self { + CommitHashError::Connect(Box::new(e)) + } +} + +impl From for CommitHashError { + fn from(e: gix::remote::ref_map::Error) -> Self { + CommitHashError::RefMap(Box::new(e)) + } +} diff --git a/src/polling/git.rs b/src/polling/git.rs index 2185397..b8603ff 100644 --- a/src/polling/git.rs +++ b/src/polling/git.rs @@ -1,6 +1,6 @@ //! Operations to fetch and extract git branch data from remote repositories. -use crate::{domain::CommitHash, error::CommitHashError}; +use crate::{domain::CommitHash, polling::CommitHashError}; use async_trait::async_trait; use gix::progress::Discard; use gix::remote::Direction; diff --git a/src/polling/mod.rs b/src/polling/mod.rs index 1038ed0..9238df2 100644 --- a/src/polling/mod.rs +++ b/src/polling/mod.rs @@ -7,7 +7,6 @@ use tracing::{info, warn}; use crate::{ context::SharedContext, engine::AsyncEngine, - error::CommitHashError, polling::{ branch::BranchInfo, error::{PollingError, handle_polling_error}, @@ -19,6 +18,8 @@ mod branch; mod error; pub mod git; +pub use error::CommitHashError; + /// Runs an asynchronous task /// that periodically polls git branches in remote repositories. pub struct PollingEngine { diff --git a/src/test_utils.rs b/src/test_utils.rs index 6718f4f..34675fb 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -26,7 +26,7 @@ impl GitFetcher for MockGitFetcher { &self, _repo: &str, _branch: &str, - ) -> Result { + ) -> Result { Ok(self.hash.clone()) } }