From 2c20f016d4561b66eaa30da041f24f8d6bfc7642 Mon Sep 17 00:00:00 2001
From: Pascal Hartig <realpassy@fb.com>
Date: Fri, 1 May 2020 09:41:13 -0700
Subject: [PATCH] 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
---
 packer/Cargo.lock   |  7 +++++++
 packer/Cargo.toml   |  1 +
 packer/src/error.rs | 17 +++++++----------
 packer/src/main.rs  | 12 +++++-------
 4 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/packer/Cargo.lock b/packer/Cargo.lock
index de164378b..60c47e233 100644
--- a/packer/Cargo.lock
+++ b/packer/Cargo.lock
@@ -9,6 +9,12 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "anyhow"
+version = "1.0.28"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d9a60d744a80c30fcb657dfe2c1b22bcb3e814c1a1e3674f32bf5820b570fbff"
+
 [[package]]
 name = "arrayref"
 version = "0.3.6"
@@ -143,6 +149,7 @@ dependencies = [
 name = "flipper-packer"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "clap",
  "serde",
  "serde_yaml",
diff --git a/packer/Cargo.toml b/packer/Cargo.toml
index 51480dfe8..1c5dda65d 100644
--- a/packer/Cargo.toml
+++ b/packer/Cargo.toml
@@ -10,3 +10,4 @@ shellexpand = "2.0.0"
 tar = "0.4.26"
 serde = { version = "1.0.106", features = ["derive"] }
 serde_yaml = "0.8.11"
+anyhow = "1.0.28"
diff --git a/packer/src/error.rs b/packer/src/error.rs
index 7e3f0e53f..ccf30ece8 100644
--- a/packer/src/error.rs
+++ b/packer/src/error.rs
@@ -7,27 +7,19 @@
 
 use crate::types::{PackType, Platform};
 use std::fmt;
-use std::io;
 use std::path::PathBuf;
 
 #[derive(Debug)]
 pub enum Error {
-    IOError(io::Error),
     MissingPackFile(Platform, PackType, PathBuf),
+    MissingPackDefinition(Platform, PackType),
 }
 
-impl From<io::Error> for Error {
-    fn from(e: io::Error) -> Self {
-        Error::IOError(e)
-    }
-}
+impl std::error::Error for Error {}
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use Error::*;
-
         match self {
-            IOError(e) => write!(f, "IO Error: {}", e),
             Error::MissingPackFile(platform, pack_type, path) => write!(
                 f,
                 "Couldn't open file to pack for platform {:?} and type {:?}: {}",
@@ -35,6 +27,11 @@ impl fmt::Display for Error {
                 pack_type,
                 path.to_string_lossy()
             ),
+            Error::MissingPackDefinition(platform, pack_type) => write!(
+                f,
+                "Missing packlist definition for platform {:?} and pack type {:?}.",
+                platform, pack_type,
+            ),
         }
     }
 }
diff --git a/packer/src/main.rs b/packer/src/main.rs
index be14ec588..04e90642f 100644
--- a/packer/src/main.rs
+++ b/packer/src/main.rs
@@ -8,6 +8,7 @@
 mod error;
 mod types;
 
+use anyhow::{bail, Result};
 use clap::value_t_or_exit;
 use std::collections::BTreeMap;
 use std::fs::File;
@@ -26,7 +27,7 @@ fn pack(
     dist_dir: &std::path::PathBuf,
     pack_list: &PackList,
     output_directory: &std::path::PathBuf,
-) -> Result<(), error::Error> {
+) -> Result<()> {
     let mut frameworks_path = output_directory.clone();
     frameworks_path.push("frameworks.tar");
     let mut frameworks_tar = tar::Builder::new(File::create(frameworks_path)?);
@@ -64,15 +65,12 @@ fn pack_platform(
     pack_list: &PackList,
     pack_type: &PackType,
     tar_builder: &mut tar::Builder<File>,
-) -> Result<(), error::Error> {
+) -> Result<()> {
     let pack_files = pack_list
         .0
         .get(platform)
         .and_then(|f| f.get(pack_type))
-        .expect(&format!(
-            "Missing packlist definition for platform {:?} and pack type {:?}.",
-            platform, pack_type
-        ));
+        .ok_or_else(|| error::Error::MissingPackDefinition(platform.clone(), pack_type.clone()))?;
     let base_dir = match platform {
         Platform::Mac => path::Path::new(dist_dir).join("mac"),
         // TODO: Verify this.
@@ -83,7 +81,7 @@ fn pack_platform(
     for f in pack_files {
         let full_path = path::Path::new(&base_dir).join(f);
         if !full_path.exists() {
-            return Err(error::Error::MissingPackFile(
+            bail!(error::Error::MissingPackFile(
                 platform.clone(),
                 pack_type.clone(),
                 full_path,