diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 0df79102a3e88c8f326139d124c329efd35c12d6_cnVzdC9DYXJnby5sb2Nr..a273ee467f4927a582d4e64af0f7ec34fc072146_cnVzdC9DYXJnby5sb2Nr 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1255,6 +1255,7 @@ version = "0.1.0" dependencies = [ "build_const", + "derive_more", "hg-core", "prost", "prost-types", diff --git a/rust/rhgitaly/Cargo.toml b/rust/rhgitaly/Cargo.toml index 0df79102a3e88c8f326139d124c329efd35c12d6_cnVzdC9yaGdpdGFseS9DYXJnby50b21s..a273ee467f4927a582d4e64af0f7ec34fc072146_cnVzdC9yaGdpdGFseS9DYXJnby50b21s 100644 --- a/rust/rhgitaly/Cargo.toml +++ b/rust/rhgitaly/Cargo.toml @@ -14,6 +14,7 @@ tracing = "0.1.37" tracing-subscriber = "0.3.16" build_const = "0.2" +derive_more = "0.99.17" [build-dependencies] tonic-build = "0.8" diff --git a/rust/rhgitaly/src/repository.rs b/rust/rhgitaly/src/repository.rs index 0df79102a3e88c8f326139d124c329efd35c12d6_cnVzdC9yaGdpdGFseS9zcmMvcmVwb3NpdG9yeS5ycw==..a273ee467f4927a582d4e64af0f7ec34fc072146_cnVzdC9yaGdpdGFseS9zcmMvcmVwb3NpdG9yeS5ycw== 100644 --- a/rust/rhgitaly/src/repository.rs +++ b/rust/rhgitaly/src/repository.rs @@ -4,5 +4,6 @@ // GNU General Public License version 2 or any later version. // SPDX-License-Identifier: GPL-2.0-or-later use std::path::PathBuf; + use tonic::Status; @@ -7,5 +8,7 @@ use tonic::Status; +use hg::repo::RepoError; + use super::config::Config; use super::gitaly::Repository; @@ -9,4 +12,8 @@ use super::config::Config; use super::gitaly::Repository; +/// Represent errors that are due to a wrong repository specification. +/// +/// In terms of gRPC methods, the specifications is usually enclosed in a [`Repository`] message +/// and these errors are considered to be client errors. #[derive(Debug, PartialEq, Eq)] @@ -12,3 +19,4 @@ #[derive(Debug, PartialEq, Eq)] -pub enum PathError { +pub enum RepoSpecError { + MissingSpecification, UnknownStorage(String), @@ -14,3 +22,12 @@ UnknownStorage(String), + RepoNotFound(PathBuf), +} + +/// Represent errors loading a repository (bad specification or internal errors) +#[derive(Debug, derive_more::From)] // TODO add PartialEq, but do it in core for RepoError first +pub enum RepoLoadError { + #[from] + SpecError(RepoSpecError), + LoadError(RepoError), } @@ -15,13 +32,33 @@ } -impl From<PathError> for Status { - fn from(value: PathError) -> Self { - match value { - PathError::UnknownStorage(storage) => Status::invalid_argument(format!( - "GetStorageByName: no such storage: \"{}\"", - storage - )), +impl From<RepoError> for RepoLoadError { + fn from(value: RepoError) -> Self { + if let RepoError::NotFound { at } = value { + return Self::SpecError(RepoSpecError::RepoNotFound(at)); + } + Self::LoadError(value) + } +} + +/// Default conversion of ['RepoSpecError'] into a gRPC ['Status'] +/// +/// This function does not care to precisely match the error details, focusing on the error +/// codes instead. +/// +/// The resulting codes match the most common behaviour of Gitaly, which actually behaves more +/// and more like this with time (e.g., as internal Git error get catched and handled). +pub fn default_repo_spec_error_status(err: RepoSpecError) -> Status { + match err { + RepoSpecError::MissingSpecification => { + Status::invalid_argument("missing repository specification") + } + RepoSpecError::UnknownStorage(storage) => Status::invalid_argument(format!( + "GetStorageByName: no such storage: \"{}\"", + storage + )), + RepoSpecError::RepoNotFound(at) => { + Status::not_found(format!("repository at \"{}\" not found", at.display())) } } } @@ -24,6 +61,6 @@ } } } -pub fn repo_path(config: &Config, repo: &Repository) -> Result<PathBuf, PathError> { +pub fn repo_path(config: &Config, repo: &Repository) -> Result<PathBuf, RepoSpecError> { if repo.storage_name != "default" { @@ -29,5 +66,5 @@ if repo.storage_name != "default" { - return Err(PathError::UnknownStorage(repo.storage_name.clone())); + return Err(RepoSpecError::UnknownStorage(repo.storage_name.clone())); } let root = &config.repositories_root; diff --git a/rust/rhgitaly/src/service/repository.rs b/rust/rhgitaly/src/service/repository.rs index 0df79102a3e88c8f326139d124c329efd35c12d6_cnVzdC9yaGdpdGFseS9zcmMvc2VydmljZS9yZXBvc2l0b3J5LnJz..a273ee467f4927a582d4e64af0f7ec34fc072146_cnVzdC9yaGdpdGFseS9zcmMvc2VydmljZS9yZXBvc2l0b3J5LnJz 100644 --- a/rust/rhgitaly/src/service/repository.rs +++ b/rust/rhgitaly/src/service/repository.rs @@ -11,7 +11,7 @@ use crate::config::Config; use crate::gitaly::repository_service_server::{RepositoryService, RepositoryServiceServer}; use crate::gitaly::{RepositoryExistsRequest, RepositoryExistsResponse}; -use crate::repository::repo_path; +use crate::repository::{default_repo_spec_error_status, repo_path}; #[derive(Debug)] pub struct RepositoryServiceImpl { @@ -28,7 +28,7 @@ info!("Processing"); if let Some(gl_repo) = &request.get_ref().repository { - let path = repo_path(&self.config, gl_repo)?; + let path = repo_path(&self.config, gl_repo).map_err(default_repo_spec_error_status)?; let res = match fs::metadata(path).await { Ok(md) => md.is_dir(), Err(_) => false,