improved error messages

uses miette for error reporting and extends the error messages to hopefully be more usefull

Author: icewind1991

cargo-espflash/Cargo.toml

@@ -24,7 +24,7 @@ categories = [
 ]
 
 [dependencies]
-anyhow = "1.0"
+miette = "2"
 cargo_metadata = "0.14"
 clap = "2.33"
 espflash = { version = "*", path = "../espflash" }

cargo-espflash/src/main.rs

@@ -1,7 +1,7 @@
-use anyhow::{anyhow, bail, Context};
 use cargo_metadata::Message;
 use clap::{App, Arg, SubCommand};
-use espflash::{Config, Flasher, PartitionTable};
+use espflash::{Config, Error, Flasher, PartitionTable};
+use miette::{bail, miette, IntoDiagnostic, Result, WrapErr};
 use serial::{BaudRate, SerialPort};
 
 use std::{
@@ -14,7 +14,7 @@ use std::{
 mod cargo_config;
 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 +84,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 +99,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 +120,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(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 +149,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,8 +159,8 @@ 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)?;
+        let path = fs::canonicalize(path).into_diagnostic()?;
+        let data = fs::read_to_string(path).into_diagnostic()?;
 
         match PartitionTable::try_from_str(data) {
             Ok(t) => Some(t),
@@ -167,7 +171,7 @@ fn main() -> anyhow::Result<()> {
     };
 
     // 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,7 +187,7 @@ 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.
@@ -219,8 +223,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,7 +235,7 @@ 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() {
@@ -266,7 +272,7 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> anyhow
         target_artifact
             .unwrap()
             .executable
-            .ok_or_else(|| anyhow!("artifact executable path is missing"))?,
+            .ok_or_else(|| miette!("artifact executable path is missing"))?,
     );
 
     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/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> {

espflash/src/error.rs

@@ -1,38 +1,120 @@
+use miette::Diagnostic;
 use slip_codec::Error as SlipError;
+use std::io;
 use thiserror::Error;
 
-#[derive(Error, Debug)]
+#[derive(Error, Debug, Diagnostic)]
 #[non_exhaustive]
 pub enum Error {
+    #[error("Error while connecting to device")]
+    #[diagnostic(transparent)]
+    Connection(#[source] ConnectionError),
+    #[error("Communication error while flashing device")]
+    #[diagnostic(transparent)]
+    Flashing(#[source] ConnectionError),
+    #[error("Supplied elf image is not valid")]
+    #[diagnostic(
+        code(espflash::invalid_elf),
+        help("Try running `cargo clean` and rebuilding the image")
+    )]
+    InvalidElf,
+    #[error("Supplied elf image can not be ran from ram")]
+    #[diagnostic(
+        code(espflash::not_ram_loadable),
+        help("Either build the binary to be all in ram or remove the `--ram` option to load the image to flash")
+    )]
+    ElfNotRamLoadable,
+    #[error("The bootloader returned an error")]
+    #[diagnostic(transparent)]
+    RomError(#[source] RomError),
+    #[error("Chip not recognized, supported chip types are esp8266, esp32 and esp32-c3")]
+    #[diagnostic(
+        code(espflash::unrecognized_chip),
+        help("If your chip is supported, try hard-resetting the device and try again")
+    )]
+    UnrecognizedChip,
+    #[error(
+        "Flash chip not supported, flash id: {0:#x}, flash sizes from 1 to 16MB are supported"
+    )]
+    #[diagnostic(code(espflash::unrecognized_flash))]
+    UnsupportedFlash(u8),
+    #[error("Failed to connect to on-device flash")]
+    #[diagnostic(code(espflash::flash_connect))]
+    FlashConnect,
+}
+
+#[derive(Error, Debug, Diagnostic)]
+#[non_exhaustive]
+pub enum ConnectionError {
     #[error("IO error while using serial port: {0}")]
-    Serial(#[from] serial::core::Error),
+    #[diagnostic(code(espflash::serial_error))]
+    Serial(#[source] serial::core::Error),
     #[error("Failed to connect to the device")]
+    #[diagnostic(
+        code(espflash::connection_failed),
+        help("Ensure that the device is connected and the reset and boot pins are not being held down")
+    )]
     ConnectionFailed,
+    #[error("Serial port not found")]
+    #[diagnostic(
+        code(espflash::connection_failed),
+        help("Ensure that the device is connected and your host recognizes the serial adapter")
+    )]
+    DeviceNotFound,
     #[error("Timeout while running command")]
+    #[diagnostic(code(espflash::timeout))]
     Timeout,
-    #[error("Invalid SLIP framing")]
+    #[error("Received packet has invalid SLIP framing")]
+    #[diagnostic(
+        code(espflash::slip_framing),
+        help("Try hard-resetting the device and try again, if the error persists your rom might be corrupted")
+    )]
     FramingError,
-    #[error("Packet to large for buffer")]
+    #[error("Received packet to large for buffer")]
+    #[diagnostic(
+        code(espflash::oversized_packet),
+        help("Try hard-resetting the device and try again, if the error persists your rom might be corrupted")
+    )]
     OverSizedPacket,
-    #[error("elf image is not valid")]
-    InvalidElf,
-    #[error("elf image can not be ran from ram")]
-    ElfNotRamLoadable,
-    #[error("bootloader returned an error: {0:?}")]
-    RomError(RomError),
-    #[error("chip not recognized, supported chip types are esp8266 and esp32")]
-    UnrecognizedChip,
-    #[error("flash chip not supported, flash id: {0:#x}")]
-    UnsupportedFlash(u8),
 }
 
-impl From<std::io::Error> for Error {
-    fn from(err: std::io::Error) -> Self {
-        Self::Serial(serial::core::Error::from(err))
+impl From<serial::Error> for ConnectionError {
+    fn from(err: serial::Error) -> Self {
+        match err.kind() {
+            serial::ErrorKind::Io(kind) => from_error_kind(kind, err),
+            serial::ErrorKind::NoDevice => ConnectionError::DeviceNotFound,
+            _ => ConnectionError::Serial(err),
+        }
     }
 }
 
-impl From<SlipError> for Error {
+impl From<serial::Error> for Error {
+    fn from(err: serial::Error) -> Self {
+        Self::Connection(err.into())
+    }
+}
+
+impl From<io::Error> for ConnectionError {
+    fn from(err: io::Error) -> Self {
+        from_error_kind(err.kind(), err)
+    }
+}
+
+impl From<io::Error> for Error {
+    fn from(err: io::Error) -> Self {
+        Self::Connection(err.into())
+    }
+}
+
+fn from_error_kind<E: Into<serial::Error>>(kind: io::ErrorKind, err: E) -> ConnectionError {
+    match kind {
+        io::ErrorKind::TimedOut => ConnectionError::Timeout,
+        io::ErrorKind::NotFound => ConnectionError::DeviceNotFound,
+        _ => ConnectionError::Serial(err.into()),
+    }
+}
+
+impl From<SlipError> for ConnectionError {
     fn from(err: SlipError) -> Self {
         match err {
             SlipError::FramingError => Self::FramingError,
@@ -43,26 +125,54 @@ impl From<SlipError> for Error {
     }
 }
 
-impl From<binread::Error> for Error {
+impl From<SlipError> for Error {
+    fn from(err: SlipError) -> Self {
+        Self::Connection(err.into())
+    }
+}
+
+impl From<binread::Error> for ConnectionError {
     fn from(err: binread::Error) -> Self {
         match err {
-            binread::Error::Io(e) => Error::from(e),
+            binread::Error::Io(e) => ConnectionError::from(e),
             _ => unreachable!(),
         }
     }
 }
 
-#[derive(Copy, Clone, Debug)]
+impl From<binread::Error> for Error {
+    fn from(err: binread::Error) -> Self {
+        Self::Connection(err.into())
+    }
+}
+
+#[derive(Copy, Clone, Debug, Error, Diagnostic)]
 #[allow(dead_code)]
 #[repr(u8)]
 pub enum RomError {
+    #[error("Invalid message received")]
+    #[diagnostic(code(espflash::rom::invalid_message))]
     InvalidMessage = 0x05,
+    #[error("Bootloader failed to execute command")]
+    #[diagnostic(code(espflash::rom::failed))]
     FailedToAct = 0x06,
+    #[error("Received message has invalid crc")]
+    #[diagnostic(code(espflash::rom::crc))]
     InvalidCrc = 0x07,
+    #[error("Bootloader failed to write to flash")]
+    #[diagnostic(code(espflash::rom::flash_write))]
     FlashWriteError = 0x08,
+    #[error("Bootloader failed to read from flash")]
+    #[diagnostic(code(espflash::rom::flash_read))]
     FlashReadError = 0x09,
+    #[error("Invalid length for flash read")]
+    #[diagnostic(code(espflash::rom::flash_read_length))]
     FlashReadLengthError = 0x0a,
+    #[error("Malformed compressed data received")]
+    #[diagnostic(code(espflash::rom::deflate))]
     DeflateError = 0x0b,
+    #[error("Other")]
+    #[diagnostic(code(espflash::rom::other))]
     Other = 0xff,
 }
 
@@ -80,3 +190,17 @@ impl From<u8> for RomError {
         }
     }
 }
+
+pub(crate) trait ResultExt {
+    /// mark an error as having occurred during the flashing stage
+    fn flashing(self) -> Self;
+}
+
+impl<T> ResultExt for Result<T, Error> {
+    fn flashing(self) -> Self {
+        match self {
+            Err(Error::Connection(err)) => Err(Error::Flashing(err)),
+            res => res,
+        }
+    }
+}