Merge pull request #44 from esp-rs/better-errors

improved error messages

Author: icewind1991

cargo-espflash/Cargo.toml

@@ -24,11 +24,12 @@ categories = [
 ]
 
 [dependencies]
-anyhow = "1.0"
+miette = "2"
 cargo_metadata = "0.14"
 clap = "2.33"
 espflash = { version = "*", path = "../espflash" }
 guess_host_triple = "0.1"
 serde = { version = "1.0", features = ["derive"] }
 serial = "0.4"
 toml = "0.5"
+thiserror = "1"

cargo-espflash/src/error.rs

@@ -0,0 +1,29 @@
+use miette::Diagnostic;
+use thiserror::Error;
+
+#[derive(Error, Debug, Diagnostic)]
+#[non_exhaustive]
+pub enum Error {
+    #[error("No executable artifact found")]
+    #[diagnostic(
+        code(cargo_espflash::no_artifact),
+        help("If you're trying to run an example you need to specify it using the `--example` argument")
+    )]
+    NoArtifact,
+    #[error("'build-std' not configured")]
+    #[diagnostic(
+        code(cargo_espflash::build_std),
+        help(
+            "cargo currently requires the unstable 'build-std' feature, ensure \
+            that .cargo/config{{.toml}} has the appropriate options.\n  \
+            \tSee: https://doc.rust-lang.org/cargo/reference/unstable.html#build-std"
+        )
+    )]
+    NoBuildStd,
+    #[error("Multiple build artifacts found")]
+    #[diagnostic(
+        code(cargo_espflash::multiple_artifacts),
+        help("Please specify which artifact to flash using --bin")
+    )]
+    MultipleArtifacts,
+}

cargo-espflash/src/main.rs

@@ -1,7 +1,8 @@
-use anyhow::{anyhow, bail, Context};
 use cargo_metadata::Message;
 use clap::{App, Arg, SubCommand};
+use error::Error;
 use espflash::{Config, Flasher, PartitionTable};
+use miette::{IntoDiagnostic, Result, WrapErr};
 use serial::{BaudRate, SerialPort};
 
 use std::{
@@ -12,9 +13,11 @@ use std::{
 };
 
 mod cargo_config;
+mod error;
+
 use cargo_config::has_build_std;
 
-fn main() -> anyhow::Result<()> {
+fn main() -> Result<()> {
     let mut app = App::new(env!("CARGO_PKG_NAME"))
         .bin_name("cargo")
         .subcommand(
@@ -84,7 +87,7 @@ fn main() -> anyhow::Result<()> {
     let matches = match matches.subcommand_matches("espflash") {
         Some(matches) => matches,
         None => {
-            app.print_help()?;
+            app.print_help().into_diagnostic()?;
             exit(0);
         }
     };
@@ -99,7 +102,7 @@ fn main() -> anyhow::Result<()> {
     } else if let Some(serial) = config.connection.serial {
         serial
     } else {
-        app.print_help()?;
+        app.print_help().into_diagnostic()?;
         exit(0);
     };
 
@@ -120,15 +123,19 @@ fn main() -> anyhow::Result<()> {
     // Attempt to open the serial port and set its initial baud rate.
     println!("Serial port: {}", port);
     println!("Connecting...\n");
-    let mut serial = serial::open(&port).context(format!("Failed to open serial port {}", port))?;
-    serial.reconfigure(&|settings| {
-        settings.set_baud_rate(BaudRate::Baud115200)?;
-        Ok(())
-    })?;
+    let mut serial = serial::open(&port)
+        .map_err(espflash::Error::from)
+        .wrap_err_with(|| format!("Failed to open serial port {}", port))?;
+    serial
+        .reconfigure(&|settings| {
+            settings.set_baud_rate(BaudRate::Baud115200)?;
+            Ok(())
+        })
+        .into_diagnostic()?;
 
     // Parse the baud rate if provided as as a command-line argument.
     let speed = if let Some(speed) = matches.value_of("speed") {
-        let speed = speed.parse::<usize>()?;
+        let speed = speed.parse::<usize>().into_diagnostic()?;
         Some(BaudRate::from_speed(speed))
     } else {
         None
@@ -145,8 +152,8 @@ fn main() -> anyhow::Result<()> {
     // If the '--bootloader' option is provided, load the binary file at the
     // specified path.
     let bootloader = if let Some(path) = matches.value_of("bootloader") {
-        let path = fs::canonicalize(path)?;
-        let data = fs::read(path)?;
+        let path = fs::canonicalize(path).into_diagnostic()?;
+        let data = fs::read(path).into_diagnostic()?;
         Some(data)
     } else {
         None
@@ -155,19 +162,16 @@ fn main() -> anyhow::Result<()> {
     // If the '--partition-table' option is provided, load the partition table from
     // the CSV at the specified path.
     let partition_table = if let Some(path) = matches.value_of("partition_table") {
-        let path = fs::canonicalize(path)?;
-        let data = fs::read_to_string(path)?;
-
-        match PartitionTable::try_from_str(data) {
-            Ok(t) => Some(t),
-            Err(e) => bail!("{}", e),
-        }
+        let path = fs::canonicalize(path).into_diagnostic()?;
+        let data = fs::read_to_string(path).into_diagnostic()?;
+        let table = PartitionTable::try_from_str(data)?;
+        Some(table)
     } else {
         None
     };
 
     // Read the ELF data from the build path and load it to the target.
-    let elf_data = fs::read(path.unwrap())?;
+    let elf_data = fs::read(path.unwrap()).into_diagnostic()?;
     if matches.is_present("ram") {
         flasher.load_elf_to_ram(&elf_data)?;
     } else {
@@ -183,16 +187,12 @@ fn board_info(flasher: &Flasher) {
     println!("Flash size: {}", flasher.flash_size());
 }
 
-fn build(release: bool, example: Option<&str>, features: Option<&str>) -> anyhow::Result<PathBuf> {
+fn build(release: bool, example: Option<&str>, features: Option<&str>) -> Result<PathBuf> {
     // The 'build-std' unstable cargo feature is required to enable
     // cross-compilation. If it has not been set then we cannot build the
     // application.
     if !has_build_std(".") {
-        bail!(
-            "cargo currently requires the unstable 'build-std' feature, ensure \
-            that .cargo/config{.toml} has the appropriate options.\n  \
-            See: https://doc.rust-lang.org/cargo/reference/unstable.html#build-std"
-        );
+        return Err(Error::NoBuildStd.into());
     };
 
     // Build the list of arguments to pass to 'cargo build'.
@@ -219,8 +219,10 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> anyhow
         .args(&["--message-format", "json-diagnostic-rendered-ansi"])
         .stdout(Stdio::piped())
         .stderr(Stdio::inherit())
-        .spawn()?
-        .wait_with_output()?;
+        .spawn()
+        .into_diagnostic()?
+        .wait_with_output()
+        .into_diagnostic()?;
 
     // Parse build output.
     let messages = Message::parse_stream(&output.stdout[..]);
@@ -229,12 +231,11 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> anyhow
     let mut target_artifact = None;
 
     for message in messages {
-        match message? {
+        match message.into_diagnostic()? {
             Message::CompilerArtifact(artifact) => {
                 if artifact.executable.is_some() {
                     if target_artifact.is_some() {
-                        // We found multiple binary artifacts, so we don't know which one to use.
-                        bail!("Multiple artifacts found, please specify one with --bin");
+                        return Err(Error::MultipleArtifacts.into());
                     } else {
                         target_artifact = Some(artifact);
                     }
@@ -258,16 +259,9 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> anyhow
     }
 
     // If no target artifact was found, we don't have a path to return.
-    if target_artifact.is_none() {
-        bail!("Artifact not found");
-    }
+    let target_artifact = target_artifact.ok_or(Error::NoArtifact)?;
 
-    let artifact_path = PathBuf::from(
-        target_artifact
-            .unwrap()
-            .executable
-            .ok_or_else(|| anyhow!("artifact executable path is missing"))?,
-    );
+    let artifact_path = target_artifact.executable.unwrap().into();
 
     Ok(artifact_path)
 }

espflash/Cargo.toml

@@ -33,6 +33,7 @@ strum_macros = "0.21.1"
 csv = "1.1.6"
 regex = "1.5.4"
 flate2 = "1"
+miette = "2"
 
 [dev-dependencies]
 pretty_assertions = "0.7.1"

espflash/src/chip/esp8266.rs

@@ -4,6 +4,7 @@ use super::{ChipType, EspCommonHeader, SegmentHeader, ESP_MAGIC};
 use crate::{
     chip::{Chip, SpiRegisters},
     elf::{update_checksum, CodeSegment, FirmwareImage, RomSegment, ESP_CHECKSUM_MAGIC},
+    error::FlashDetectError,
     flasher::FlashSize,
     Error, PartitionTable,
 };
@@ -96,7 +97,7 @@ impl ChipType for Esp8266 {
     }
 }
 
-fn encode_flash_size(size: FlashSize) -> Result<u8, Error> {
+fn encode_flash_size(size: FlashSize) -> Result<u8, FlashDetectError> {
     match size {
         FlashSize::Flash256Kb => Ok(0x10),
         FlashSize::Flash512Kb => Ok(0x00),
@@ -105,7 +106,7 @@ fn encode_flash_size(size: FlashSize) -> Result<u8, Error> {
         FlashSize::Flash4Mb => Ok(0x40),
         FlashSize::Flash8Mb => Ok(0x80),
         FlashSize::Flash16Mb => Ok(0x90),
-        FlashSize::FlashRetry => Err(Error::UnsupportedFlash(size as u8)),
+        FlashSize::FlashRetry => Err(FlashDetectError::from(size as u8)),
     }
 }
 

espflash/src/chip/mod.rs

@@ -7,8 +7,9 @@ use crate::{
     Error, PartitionTable,
 };
 
-use std::{io::Write, str::FromStr};
+use std::io::Write;
 
+use crate::error::{ChipDetectError, FlashDetectError};
 use crate::flash_target::{Esp32Target, Esp8266Target, FlashTarget, RamTarget};
 use crate::flasher::SpiAttachParams;
 pub use esp32::Esp32;
@@ -102,14 +103,14 @@ pub enum Chip {
 }
 
 impl Chip {
-    pub fn from_magic(magic: u32) -> Option<Self> {
+    pub fn from_magic(magic: u32) -> Result<Self, ChipDetectError> {
         match magic {
-            Esp32::CHIP_DETECT_MAGIC_VALUE => Some(Chip::Esp32),
+            Esp32::CHIP_DETECT_MAGIC_VALUE => Ok(Chip::Esp32),
             Esp32c3::CHIP_DETECT_MAGIC_VALUE | Esp32c3::CHIP_DETECT_MAGIC_VALUE2 => {
-                Some(Chip::Esp32c3)
+                Ok(Chip::Esp32c3)
             }
-            Esp8266::CHIP_DETECT_MAGIC_VALUE => Some(Chip::Esp8266),
-            _ => None,
+            Esp8266::CHIP_DETECT_MAGIC_VALUE => Ok(Chip::Esp8266),
+            _ => Err(ChipDetectError::from(magic)),
         }
     }
 
@@ -154,19 +155,6 @@ impl Chip {
     }
 }
 
-impl FromStr for Chip {
-    type Err = Error;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s {
-            "esp32" => Ok(Chip::Esp32),
-            "esp32c3" => Ok(Chip::Esp32c3),
-            "esp8266" => Ok(Chip::Esp8266),
-            _ => Err(Error::UnrecognizedChip),
-        }
-    }
-}
-
 #[derive(Copy, Clone, Zeroable, Pod, Debug)]
 #[repr(C)]
 struct EspCommonHeader {
@@ -186,16 +174,16 @@ struct SegmentHeader {
 
 // Note that this function ONLY applies to the ESP32 and variants; the ESP8266
 // has defined its own version rather than using this implementation.
-fn encode_flash_size(size: FlashSize) -> Result<u8, Error> {
+fn encode_flash_size(size: FlashSize) -> Result<u8, FlashDetectError> {
     match size {
-        FlashSize::Flash256Kb => Err(Error::UnsupportedFlash(size as u8)),
-        FlashSize::Flash512Kb => Err(Error::UnsupportedFlash(size as u8)),
+        FlashSize::Flash256Kb => Err(FlashDetectError::from(size as u8)),
+        FlashSize::Flash512Kb => Err(FlashDetectError::from(size as u8)),
         FlashSize::Flash1Mb => Ok(0x00),
         FlashSize::Flash2Mb => Ok(0x10),
         FlashSize::Flash4Mb => Ok(0x20),
         FlashSize::Flash8Mb => Ok(0x30),
         FlashSize::Flash16Mb => Ok(0x40),
-        FlashSize::FlashRetry => Err(Error::UnsupportedFlash(size as u8)),
+        FlashSize::FlashRetry => Err(FlashDetectError::from(size as u8)),
     }
 }
 

espflash/src/connection.rs

@@ -3,7 +3,7 @@ use std::thread::sleep;
 use std::time::Duration;
 
 use crate::encoder::SlipEncoder;
-use crate::error::{Error, RomError};
+use crate::error::{ConnectionError, Error, RomError};
 use binread::io::Cursor;
 use binread::{BinRead, BinReaderExt};
 use serial::{BaudRate, SerialPort, SerialPortSettings};
@@ -135,7 +135,7 @@ impl Connection {
                 }
             }
         }
-        Err(Error::ConnectionFailed)
+        Err(Error::Connection(ConnectionError::ConnectionFailed))
     }
 
     fn read(&mut self) -> Result<Vec<u8>, Error> {