Use anyhow for error handling

Summary:
This is a nice solution if you don't want to spend
too much time thinking about error handling.
You have one common return type and can basically use `?`
everywhere while still maintaining the flexibility to
create custom error types where needed.

Reviewed By: jknoxville

Differential Revision: D21349046

fbshipit-source-id: 073539ce8422cdb3e0141886e95321052bc0c7a3
This commit is contained in:
Pascal Hartig
2020-05-01 09:41:13 -07:00
committed by Facebook GitHub Bot
parent 6f9d82117e
commit 2c20f016d4
4 changed files with 20 additions and 17 deletions

7
packer/Cargo.lock generated
View File

@@ -9,6 +9,12 @@ dependencies = [
"winapi", "winapi",
] ]
[[package]]
name = "anyhow"
version = "1.0.28"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d9a60d744a80c30fcb657dfe2c1b22bcb3e814c1a1e3674f32bf5820b570fbff"
[[package]] [[package]]
name = "arrayref" name = "arrayref"
version = "0.3.6" version = "0.3.6"
@@ -143,6 +149,7 @@ dependencies = [
name = "flipper-packer" name = "flipper-packer"
version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"anyhow",
"clap", "clap",
"serde", "serde",
"serde_yaml", "serde_yaml",

View File

@@ -10,3 +10,4 @@ shellexpand = "2.0.0"
tar = "0.4.26" tar = "0.4.26"
serde = { version = "1.0.106", features = ["derive"] } serde = { version = "1.0.106", features = ["derive"] }
serde_yaml = "0.8.11" serde_yaml = "0.8.11"
anyhow = "1.0.28"

View File

@@ -7,27 +7,19 @@
use crate::types::{PackType, Platform}; use crate::types::{PackType, Platform};
use std::fmt; use std::fmt;
use std::io;
use std::path::PathBuf; use std::path::PathBuf;
#[derive(Debug)] #[derive(Debug)]
pub enum Error { pub enum Error {
IOError(io::Error),
MissingPackFile(Platform, PackType, PathBuf), MissingPackFile(Platform, PackType, PathBuf),
MissingPackDefinition(Platform, PackType),
} }
impl From<io::Error> for Error { impl std::error::Error for Error {}
fn from(e: io::Error) -> Self {
Error::IOError(e)
}
}
impl fmt::Display for Error { impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use Error::*;
match self { match self {
IOError(e) => write!(f, "IO Error: {}", e),
Error::MissingPackFile(platform, pack_type, path) => write!( Error::MissingPackFile(platform, pack_type, path) => write!(
f, f,
"Couldn't open file to pack for platform {:?} and type {:?}: {}", "Couldn't open file to pack for platform {:?} and type {:?}: {}",
@@ -35,6 +27,11 @@ impl fmt::Display for Error {
pack_type, pack_type,
path.to_string_lossy() path.to_string_lossy()
), ),
Error::MissingPackDefinition(platform, pack_type) => write!(
f,
"Missing packlist definition for platform {:?} and pack type {:?}.",
platform, pack_type,
),
} }
} }
} }

View File

@@ -8,6 +8,7 @@
mod error; mod error;
mod types; mod types;
use anyhow::{bail, Result};
use clap::value_t_or_exit; use clap::value_t_or_exit;
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::fs::File; use std::fs::File;
@@ -26,7 +27,7 @@ fn pack(
dist_dir: &std::path::PathBuf, dist_dir: &std::path::PathBuf,
pack_list: &PackList, pack_list: &PackList,
output_directory: &std::path::PathBuf, output_directory: &std::path::PathBuf,
) -> Result<(), error::Error> { ) -> Result<()> {
let mut frameworks_path = output_directory.clone(); let mut frameworks_path = output_directory.clone();
frameworks_path.push("frameworks.tar"); frameworks_path.push("frameworks.tar");
let mut frameworks_tar = tar::Builder::new(File::create(frameworks_path)?); let mut frameworks_tar = tar::Builder::new(File::create(frameworks_path)?);
@@ -64,15 +65,12 @@ fn pack_platform(
pack_list: &PackList, pack_list: &PackList,
pack_type: &PackType, pack_type: &PackType,
tar_builder: &mut tar::Builder<File>, tar_builder: &mut tar::Builder<File>,
) -> Result<(), error::Error> { ) -> Result<()> {
let pack_files = pack_list let pack_files = pack_list
.0 .0
.get(platform) .get(platform)
.and_then(|f| f.get(pack_type)) .and_then(|f| f.get(pack_type))
.expect(&format!( .ok_or_else(|| error::Error::MissingPackDefinition(platform.clone(), pack_type.clone()))?;
"Missing packlist definition for platform {:?} and pack type {:?}.",
platform, pack_type
));
let base_dir = match platform { let base_dir = match platform {
Platform::Mac => path::Path::new(dist_dir).join("mac"), Platform::Mac => path::Path::new(dist_dir).join("mac"),
// TODO: Verify this. // TODO: Verify this.
@@ -83,7 +81,7 @@ fn pack_platform(
for f in pack_files { for f in pack_files {
let full_path = path::Path::new(&base_dir).join(f); let full_path = path::Path::new(&base_dir).join(f);
if !full_path.exists() { if !full_path.exists() {
return Err(error::Error::MissingPackFile( bail!(error::Error::MissingPackFile(
platform.clone(), platform.clone(),
pack_type.clone(), pack_type.clone(),
full_path, full_path,