improve more cargo-espflash error messages

Author: icewind1991

cargo-espflash/Cargo.toml

@@ -32,3 +32,4 @@ 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 cargo_metadata::Message;
 use clap::{App, Arg, SubCommand};
-use espflash::{Config, Error, Flasher, PartitionTable};
-use miette::{bail, miette, IntoDiagnostic, Result, WrapErr};
+use error::Error;
+use espflash::{Config, Flasher, PartitionTable};
+use miette::{IntoDiagnostic, Result, WrapErr};
 use serial::{BaudRate, SerialPort};
 
 use std::{
@@ -12,6 +13,8 @@ use std::{
 };
 
 mod cargo_config;
+mod error;
+
 use cargo_config::has_build_std;
 
 fn main() -> Result<()> {
@@ -121,7 +124,7 @@ fn main() -> Result<()> {
     println!("Serial port: {}", port);
     println!("Connecting...\n");
     let mut serial = serial::open(&port)
-        .map_err(Error::from)
+        .map_err(espflash::Error::from)
         .wrap_err_with(|| format!("Failed to open serial port {}", port))?;
     serial
         .reconfigure(&|settings| {
@@ -161,11 +164,8 @@ fn main() -> Result<()> {
     let partition_table = if let Some(path) = matches.value_of("partition_table") {
         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),
-            Err(e) => bail!("{}", e),
-        }
+        let table = PartitionTable::try_from_str(data)?;
+        Some(table)
     } else {
         None
     };
@@ -192,11 +192,7 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> Result
     // 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'.
@@ -239,8 +235,7 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> Result
             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);
                     }
@@ -264,16 +259,9 @@ fn build(release: bool, example: Option<&str>, features: Option<&str>) -> Result
     }
 
     // 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(|| miette!("artifact executable path is missing"))?,
-    );
+    let artifact_path = target_artifact.executable.unwrap().into();
 
     Ok(artifact_path)
 }

espflash/src/error.rs

@@ -1,4 +1,5 @@
-use miette::Diagnostic;
+use csv::Position;
+use miette::{Diagnostic, SourceOffset, SourceSpan};
 use slip_codec::Error as SlipError;
 use std::io;
 use thiserror::Error;
@@ -41,6 +42,9 @@ pub enum Error {
     #[error("Failed to connect to on-device flash")]
     #[diagnostic(code(espflash::flash_connect))]
     FlashConnect,
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    MalformedPartitionTable(PartitionTableError),
 }
 
 #[derive(Error, Debug, Diagnostic)]
@@ -204,3 +208,60 @@ impl<T> ResultExt for Result<T, Error> {
         }
     }
 }
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("Malformed partition table")]
+#[diagnostic(
+    code(espflash::mallformed_partition_table),
+    help("See the espressif documentation for information on the partition table format:
+
+          https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/partition-tables.html#creating-custom-tables")
+)]
+pub struct PartitionTableError {
+    source: String,
+    #[snippet(source)]
+    snip: SourceSpan,
+    #[highlight(snip, label("{}", self.hint))]
+    err_span: SourceSpan,
+    hint: String,
+    #[source]
+    error: csv::Error,
+}
+
+impl PartitionTableError {
+    pub fn new(error: csv::Error, source: String) -> Self {
+        let err_pos = match error.kind() {
+            csv::ErrorKind::Deserialize { pos: Some(pos), .. } => pos.clone(),
+            csv::ErrorKind::UnequalLengths { pos: Some(pos), .. } => pos.clone(),
+            _ => Position::new(),
+        };
+        let hint = match error.kind() {
+            csv::ErrorKind::Deserialize { err, .. } => err.to_string(),
+            csv::ErrorKind::UnequalLengths {
+                expected_len, len, ..
+            } => format!(
+                "record has {} fields, but the previous record has {} fields",
+                len, expected_len
+            ),
+            _ => String::new(),
+        };
+        let snip_start =
+            SourceOffset::from_location(&source, err_pos.line().saturating_sub(2) as usize, 0);
+        let snip_end =
+            SourceOffset::from_location(&source, err_pos.line().saturating_add(2) as usize, 0);
+        let snip = SourceSpan::new(snip_start, (snip_end.offset() - snip_start.offset()).into());
+        let err_span = SourceSpan::new(pos_to_offset(err_pos), 0.into());
+
+        PartitionTableError {
+            source,
+            err_span,
+            snip,
+            hint,
+            error,
+        }
+    }
+}
+
+fn pos_to_offset(pos: Position) -> SourceOffset {
+    (pos.byte() as usize + 1).into()
+}

espflash/src/partition_table.rs

@@ -2,7 +2,8 @@ use md5::{Context, Digest};
 use regex::Regex;
 use serde::{Deserialize, Deserializer};
 
-use std::{error::Error, io::Write};
+use crate::error::PartitionTableError;
+use std::io::Write;
 
 const MAX_PARTITION_LENGTH: usize = 0xC00;
 const PARTITION_TABLE_SIZE: usize = 0x1000;
@@ -147,9 +148,9 @@ impl PartitionTable {
     }
 
     /// Attempt to parse a partition table from the given string. For more
-    /// information on the paritition table CSV format see:
+    /// information on the partition table CSV format see:
     /// https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/partition-tables.html
-    pub fn try_from_str<S: Into<String>>(data: S) -> Result<Self, Box<dyn Error>> {
+    pub fn try_from_str<S: Into<String>>(data: S) -> Result<Self, PartitionTableError> {
         let data = data.into();
         let mut reader = csv::ReaderBuilder::new()
             .comment(Some(b'#'))
@@ -159,7 +160,8 @@ impl PartitionTable {
 
         let mut partitions = Vec::with_capacity(MAX_PARTITION_TABLE_ENTRIES);
         for partition in reader.deserialize() {
-            let partition: Partition = partition?;
+            let partition: Partition =
+                partition.map_err(|e| PartitionTableError::new(e, data.clone()))?;
             partitions.push(partition);
         }