node: clean up `UploadError`

This change better separates the different errors that can occur during an
upload. It does this by:
- Separating the authorization error into its own enum and having a variant
  simply for that
- The `PacketLine` error was never constructed, so the error returned from the
  `upload_pack::pktline::git_request` is now wrapped in this variant.
- The error from `upload_pack::upload_pack` is now wrapped in an `UploadPack`
  variant.
- The `Io` variant is removed.
- Other redundant variants are also removed.
This commit is contained in:
Fintan Halpenny 2025-05-13 15:11:01 +02:00
parent db3b3b0548
commit e965d9a2c9
1 changed files with 15 additions and 13 deletions

View File

@ -72,24 +72,26 @@ impl FetchError {
pub enum UploadError { pub enum UploadError {
#[error("error parsing git command packet-line: {0}")] #[error("error parsing git command packet-line: {0}")]
PacketLine(io::Error), PacketLine(io::Error),
#[error("error while performing git upload-pack: {0}")]
UploadPack(io::Error),
#[error(transparent)] #[error(transparent)]
Io(#[from] io::Error), Authorization(#[from] AuthorizationError),
}
#[derive(thiserror::Error, Debug)]
pub enum AuthorizationError {
#[error("{0} is not authorized to fetch {1}")] #[error("{0} is not authorized to fetch {1}")]
Unauthorized(NodeId, RepoId), Unauthorized(NodeId, RepoId),
#[error(transparent)] #[error(transparent)]
Storage(#[from] radicle::storage::Error), PolicyStore(#[from] radicle::node::policy::store::Error),
#[error(transparent)]
Identity(#[from] radicle::identity::DocError),
#[error(transparent)] #[error(transparent)]
Repository(#[from] radicle::storage::RepositoryError), Repository(#[from] radicle::storage::RepositoryError),
#[error(transparent)]
PolicyStore(#[from] radicle::node::policy::store::Error),
} }
impl UploadError { impl UploadError {
/// Check if it's an end-of-file error. /// Check if it's an end-of-file error.
pub fn is_eof(&self) -> bool { pub fn is_eof(&self) -> bool {
matches!(self, UploadError::Io(e) if e.kind() == io::ErrorKind::UnexpectedEof) matches!(self, UploadError::UploadPack(e) if e.kind() == io::ErrorKind::UnexpectedEof)
} }
} }
@ -243,7 +245,7 @@ impl Worker {
Err(e) => { Err(e) => {
return FetchResult::Responder { return FetchResult::Responder {
rid: None, rid: None,
result: Err(e.into()), result: Err(UploadError::PacketLine(e)),
} }
} }
}; };
@ -252,7 +254,7 @@ impl Worker {
if let Err(e) = self.is_authorized(remote, header.repo) { if let Err(e) = self.is_authorized(remote, header.repo) {
return FetchResult::Responder { return FetchResult::Responder {
rid: Some(header.repo), rid: Some(header.repo),
result: Err(e), result: Err(e.into()),
}; };
} }
@ -267,7 +269,7 @@ impl Worker {
timeout, timeout,
) )
.map(|_| ()) .map(|_| ())
.map_err(|e| e.into()); .map_err(UploadError::UploadPack);
log::debug!(target: "worker", "Upload process on stream {stream} exited with result {result:?}"); log::debug!(target: "worker", "Upload process on stream {stream} exited with result {result:?}");
FetchResult::Responder { FetchResult::Responder {
@ -278,18 +280,18 @@ impl Worker {
} }
} }
fn is_authorized(&self, remote: NodeId, rid: RepoId) -> Result<(), UploadError> { fn is_authorized(&self, remote: NodeId, rid: RepoId) -> Result<(), AuthorizationError> {
let policy = self.policies.seed_policy(&rid)?.policy; let policy = self.policies.seed_policy(&rid)?.policy;
// Check policy first, since if we're blocking then we likely don't have // Check policy first, since if we're blocking then we likely don't have
// the repository. // the repository.
if policy.is_block() { if policy.is_block() {
return Err(UploadError::Unauthorized(remote, rid)); return Err(AuthorizationError::Unauthorized(remote, rid));
} }
let repo = self.storage.repository(rid)?; let repo = self.storage.repository(rid)?;
let doc = repo.identity_doc()?; let doc = repo.identity_doc()?;
if !doc.is_visible_to(&remote.into()) { if !doc.is_visible_to(&remote.into()) {
Err(UploadError::Unauthorized(remote, rid)) Err(AuthorizationError::Unauthorized(remote, rid))
} else { } else {
Ok(()) Ok(())
} }