Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance error handling with thiserror #1447

Merged
merged 1 commit into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 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 api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ log = "0.4.8"
serde_json = "1.0.53"
toml = "0.5"

thiserror = "1.0.30"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move it to line 15.

backtrace = { version = "0.3", optional = true }
dbs-uhttp = { version = "0.3.0", optional = true }
http = { version = "0.2.1", optional = true }
Expand Down
33 changes: 17 additions & 16 deletions api/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ use std::sync::mpsc::{RecvError, SendError};

use serde::Deserialize;
use serde_json::Error as SerdeError;
use thiserror::Error;

use crate::BlobCacheEntry;

/// Errors related to Metrics.
#[derive(Debug)]
#[derive(Error, Debug)]
pub enum MetricsError {
/// Non-exist counter.
#[error("no counter found for the metric")]
NoCounter,
/// Failed to serialize message.
Serialize(SerdeError),
#[error("failed to serialize metric: {0:?}")]
Serialize(#[source] SerdeError),
}

/// Mount a filesystem.
Expand Down Expand Up @@ -145,25 +146,25 @@ pub enum MetricsErrorKind {
Stats(MetricsError),
}

#[derive(Debug)]
#[derive(Error, Debug)]
#[allow(clippy::large_enum_variant)]
pub enum ApiError {
/// Daemon internal error
#[error("daemon internal error: {0:?}")]
DaemonAbnormal(DaemonErrorKind),
/// Failed to get events information
#[error("daemon events error: {0}")]
Events(String),
/// Failed to get metrics information
#[error("metrics error: {0:?}")]
Metrics(MetricsErrorKind),
/// Failed to mount filesystem
#[error("failed to mount filesystem: {0:?}")]
MountFilesystem(DaemonErrorKind),
/// Failed to send request to the API service
RequestSend(SendError<Option<ApiRequest>>),
/// Unrecognized payload content
#[error("failed to send request to the API service: {0:?}")]
RequestSend(#[from] SendError<Option<ApiRequest>>),
#[error("failed to parse response payload type")]
ResponsePayloadType,
/// Failed to receive response from the API service
ResponseRecv(RecvError),
/// Failed to send wakeup notification
Wakeup(io::Error),
#[error("failed to receive response from the API service: {0:?}")]
ResponseRecv(#[from] RecvError),
#[error("failed to wake up the daemon: {0:?}")]
Wakeup(#[source] io::Error),
}

/// Specialized `std::result::Result` for API replies.
Expand Down
2 changes: 1 addition & 1 deletion rafs/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl Rafs {
let cache_cfg = cfg.get_cache_config().map_err(RafsError::LoadConfig)?;
let rafs_cfg = cfg.get_rafs_config().map_err(RafsError::LoadConfig)?;
let (sb, reader) = RafsSuper::load_from_file(path, cfg.clone(), false)
.map_err(RafsError::FillSuperblock)?;
.map_err(RafsError::FillSuperBlock)?;
let blob_infos = sb.superblock.get_blob_infos();
let device = BlobDevice::new(cfg, &blob_infos).map_err(RafsError::CreateDevice)?;

Expand Down
35 changes: 18 additions & 17 deletions rafs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ extern crate nydus_storage as storage;

use std::any::Any;
use std::borrow::Cow;
use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
use std::fmt::Debug;
use std::fs::File;
use std::io::{BufWriter, Error, Read, Result, Seek, SeekFrom, Write};
use std::os::unix::io::AsRawFd;
Expand All @@ -56,37 +56,38 @@ pub mod metadata;
pub mod mock;

/// Error codes for rafs related operations.
#[derive(Debug)]
#[derive(thiserror::Error, Debug)]
pub enum RafsError {
#[error("Operation is not supported.")]
Unsupported,
#[error("Rafs is not initialized.")]
Uninitialized,
#[error("Rafs is already mounted.")]
AlreadyMounted,
#[error("Failed to read metadata: {0}`")]
ReadMetadata(Error, String),
#[error("Failed to load config: {0}`")]
LoadConfig(Error),
ParseConfig(serde_json::Error),
#[error("Failed to parse config: {0}`")]
ParseConfig(#[source] serde_json::Error),
#[error("Failed to create swap backend: {0}`")]
SwapBackend(Error),
FillSuperblock(Error),
#[error("Failed to fill superBlock: {0}`")]
FillSuperBlock(Error),
#[error("Failed to create device: {0}`")]
CreateDevice(Error),
#[error("Failed to prefetch data: {0}`")]
Prefetch(String),
#[error("Failed to configure device: {0}`")]
Configure(String),
#[error("Incompatible RAFS version: `{0}`")]
Incompatible(u16),
#[error("Illegal meta struct, type is `{0:?}` and content is `{1}`")]
IllegalMetaStruct(MetaType, String),
#[error("Invalid image data")]
InvalidImageData,
}

impl std::error::Error for RafsError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
None
}
}

impl Display for RafsError {
fn fmt(&self, f: &mut Formatter) -> FmtResult {
write!(f, "{:?}", self)?;
Ok(())
}
}

#[derive(Debug)]
pub enum MetaType {
Regular,
Expand Down
2 changes: 1 addition & 1 deletion rafs/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ impl RafsSuper {
pub fn update(&self, r: &mut RafsIoReader) -> RafsResult<()> {
if self.meta.is_v5() {
self.skip_v5_superblock(r)
.map_err(RafsError::FillSuperblock)?;
.map_err(RafsError::FillSuperBlock)?;
}

self.superblock.update(r)
Expand Down
8 changes: 1 addition & 7 deletions service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub enum Error {
#[error("RAFS failed to handle request, {0}")]
Rafs(#[from] RafsError),
#[error("VFS failed to handle request, {0:?}")]
Vfs(VfsError),
Vfs(#[from] VfsError),

// fusedev
#[error("failed to create FUSE server, {0}")]
Expand Down Expand Up @@ -130,12 +130,6 @@ impl From<Error> for io::Error {
}
}

impl From<VfsError> for Error {
fn from(e: VfsError) -> Self {
Error::Vfs(e)
}
}

impl From<Error> for DaemonErrorKind {
fn from(e: Error) -> Self {
use Error::*;
Expand Down
1 change: 1 addition & 0 deletions utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ repository = "https://github.com/dragonflyoss/image-service"
edition = "2018"

[dependencies]
thiserror = "1.0.30"
blake3 = "1.3"
httpdate = "1.0"
lazy_static = "1.4"
Expand Down
4 changes: 3 additions & 1 deletion utils/src/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use std::time::SystemTime;

use serde::Serialize;
use serde_json::{error::Error, value::Value};
use thiserror::Error;

impl Display for TraceClass {
fn fmt(&self, f: &mut Formatter) -> FmtResult {
Expand Down Expand Up @@ -51,8 +52,9 @@ pub enum TraceClass {
}
}

#[derive(Debug)]
#[derive(Error, Debug)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about using #[derive(thiserror::Error, Debug)], it's more informative.

pub enum TraceError {
#[error("serialize error: {0}")]
Serde(Error),
}

Expand Down