Skip to content
Snippets Groups Projects
Commit a273ee46 authored by Georges Racinet's avatar Georges Racinet
Browse files

RHGitaly: separation of repo errors into specification and load errors

As the Gitaly Comparison tests showed us in the Python case, the errors
to return on wrong `Repository` specifications depend at least on the service,
hence we need an added flexibility.

For that, we need internally to make a difference between client-side errors
(e.g., repo does not exist) and internal errors (failed to load repo, e.g., due
to a filesystem issue).
parent 0df79102
No related branches found
No related tags found
2 merge requests!151heptapod#743: making 0.36 the new stable,!144RHGitaly: implement CommitService.ListCommitsByOid
......@@ -1255,6 +1255,7 @@
version = "0.1.0"
dependencies = [
"build_const",
"derive_more",
"hg-core",
"prost",
"prost-types",
......
......@@ -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"
......
......@@ -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;
......
......@@ -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,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment