Simplify errors with thiserror (#1533)

Thiserror is far better for most of these, and reduces accidental bugs /
mismatches with descriptions.

pgrx-pg-config/Cargo.toml

@@ -26,6 +26,7 @@ edition = "2021"
 cargo_toml.workspace = true
 eyre.workspace = true
 owo-colors.workspace = true
+thiserror.workspace = true
 url.workspace = true
 
 home = "0.5.9"

pgrx-pg-config/src/lib.rs

@@ -12,13 +12,13 @@ use eyre::{eyre, WrapErr};
 use owo_colors::OwoColorize;
 use serde_derive::{Deserialize, Serialize};
 use std::collections::{BTreeMap, HashMap};
-use std::error::Error;
 use std::ffi::OsString;
 use std::fmt::{self, Debug, Display, Formatter};
 use std::io::ErrorKind;
 use std::path::PathBuf;
 use std::process::{Command, Stdio};
 use std::str::FromStr;
+use thiserror::Error;
 use url::Url;
 
 pub mod cargo;
@@ -491,17 +491,15 @@ impl<'a> PgConfigSelector<'a> {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
 pub enum PgrxHomeError {
+    #[error("You don't seem to have a home directory")]
     NoHomeDirectory,
+    // allow caller to decide whether it is safe to enumerate paths
+    #[error("$PGRX_HOME does not exist")]
     MissingPgrxHome(PathBuf),
-    IoError(std::io::Error),
-}
-
-impl From<std::io::Error> for PgrxHomeError {
-    fn from(value: std::io::Error) -> Self {
-        PgrxHomeError::IoError(value)
-    }
+    #[error(transparent)]
+    IoError(#[from] std::io::Error),
 }
 
 impl From<PgrxHomeError> for std::io::Error {
@@ -518,18 +516,6 @@ impl From<PgrxHomeError> for std::io::Error {
     }
 }
 
-impl Display for PgrxHomeError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            PgrxHomeError::NoHomeDirectory => write!(f, "You don't seem to have a home directory"),
-            PgrxHomeError::MissingPgrxHome(_) => write!(f, "$PGRX_HOME does not exist"),
-            PgrxHomeError::IoError(e) => write!(f, "{e}"),
-        }
-    }
-}
-
-impl Error for PgrxHomeError {}
-
 impl Pgrx {
     pub fn new(base_port: u16, base_testing_port: u16) -> Self {
         Pgrx { pg_configs: vec![], base_port, base_testing_port }

pgrx-pg-sys/src/submodules/errcodes.rs

@@ -391,8 +391,6 @@ impl Display for PgSqlErrorCode {
     }
 }
 
-impl std::error::Error for PgSqlErrorCode {}
-
 impl From<i32> for PgSqlErrorCode {
     fn from(error_code: i32) -> Self {
         (error_code as isize).into()

pgrx-sql-entity-graph/Cargo.toml

@@ -27,6 +27,7 @@ no-schema-generation = []
 [dependencies]
 eyre.workspace = true
 unescape.workspace = true
+thiserror.workspace = true
 
 convert_case = "0.6.0"
 petgraph = "0.6.4"

pgrx-sql-entity-graph/src/control_file.rs

@@ -18,6 +18,7 @@ to the `pgrx` framework and very subject to change between versions. While you m
 use super::{SqlGraphEntity, SqlGraphIdentifier, ToSql};
 use core::convert::TryFrom;
 use std::collections::HashMap;
+use thiserror::Error;
 
 /// The parsed contents of a `.control` file.
 ///
@@ -107,28 +108,14 @@ impl From<ControlFile> for SqlGraphEntity {
 }
 
 /// An error met while parsing a `.control` file.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Error)]
 pub enum ControlFileError {
+    #[error("Missing field in control file! Please add `{field}`.")]
     MissingField { field: &'static str },
+    #[error("Redundant field in control file! Please remove `{field}`.")]
     RedundantField { field: &'static str },
 }
 
-impl std::fmt::Display for ControlFileError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            ControlFileError::MissingField { field } => {
-                write!(f, "Missing field in control file! Please add `{field}`.")?;
-            }
-            ControlFileError::RedundantField { field } => {
-                write!(f, "Redundant field in control file! Please remove `{field}`.")?;
-            }
-        };
-        Ok(())
-    }
-}
-
-impl std::error::Error for ControlFileError {}
-
 impl TryFrom<&str> for ControlFile {
     type Error = ControlFileError;
 

pgrx-sql-entity-graph/src/metadata/return_variant.rs

@@ -15,7 +15,7 @@ Return value specific metadata for Rust to SQL translation
 to the `pgrx` framework and very subject to change between versions. While you may use this, please do it with caution.
 
 */
-use std::error::Error;
+use thiserror::Error;
 
 use super::sql_translatable::SqlMapping;
 
@@ -29,54 +29,24 @@ pub enum Returns {
     Table(Vec<SqlMapping>),
 }
 
-#[derive(Clone, Copy, Debug, Hash, Ord, PartialOrd, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, Hash, Ord, PartialOrd, PartialEq, Eq, Error)]
 pub enum ReturnsError {
+    #[error("Nested SetOfIterator in return type")]
     NestedSetOf,
+    #[error("Nested TableIterator in return type")]
     NestedTable,
+    #[error("SetOfIterator containing TableIterator in return type")]
     SetOfContainingTable,
+    #[error("TableIterator containing SetOfIterator in return type")]
     TableContainingSetOf,
+    #[error("SetofIterator inside Array is not valid")]
     SetOfInArray,
+    #[error("TableIterator inside Array is not valid")]
     TableInArray,
+    #[error("Cannot use bare u8")]
     BareU8,
+    #[error("SqlMapping::Skip inside Array is not valid")]
     SkipInArray,
+    #[error("A Datum as a return means that `sql = \"...\"` must be set in the declaration")]
     Datum,
 }
-
-impl std::fmt::Display for ReturnsError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            ReturnsError::NestedSetOf => {
-                write!(f, "Nested SetOfIterator in return type")
-            }
-            ReturnsError::NestedTable => {
-                write!(f, "Nested TableIterator in return type")
-            }
-            ReturnsError::SetOfContainingTable => {
-                write!(f, "SetOfIterator containing TableIterator in return type")
-            }
-            ReturnsError::TableContainingSetOf => {
-                write!(f, "TableIterator containing SetOfIterator in return type")
-            }
-            ReturnsError::SetOfInArray => {
-                write!(f, "SetofIterator inside Array is not valid")
-            }
-            ReturnsError::TableInArray => {
-                write!(f, "TableIterator inside Array is not valid")
-            }
-            ReturnsError::SkipInArray => {
-                write!(f, "SqlMapping::Skip inside Array is not valid")
-            }
-            ReturnsError::BareU8 => {
-                write!(f, "Cannot use bare u8")
-            }
-            ReturnsError::Datum => {
-                write!(
-                    f,
-                    "A Datum as a return means that `sql = \"...\"` must be set in the declaration"
-                )
-            }
-        }
-    }
-}
-
-impl Error for ReturnsError {}

pgrx-sql-entity-graph/src/metadata/sql_translatable.rs

@@ -16,47 +16,28 @@ to the `pgrx` framework and very subject to change between versions. While you m
 
 */
 use std::any::Any;
-use std::error::Error;
 use std::fmt::Display;
+use thiserror::Error;
 
 use super::return_variant::ReturnsError;
 use super::{FunctionMetadataTypeEntity, Returns};
 
-#[derive(Clone, Copy, Debug, Hash, Ord, PartialOrd, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, Hash, Ord, PartialOrd, PartialEq, Eq, Error)]
 pub enum ArgumentError {
+    #[error("Cannot use SetOfIterator as an argument")]
     SetOf,
+    #[error("Cannot use TableIterator as an argument")]
     Table,
+    #[error("Cannot use bare u8")]
     BareU8,
+    #[error("SqlMapping::Skip inside Array is not valid")]
     SkipInArray,
+    #[error("A Datum as an argument means that `sql = \"...\"` must be set in the declaration")]
     Datum,
+    #[error("`{0}` is not able to be used as a function argument")]
     NotValidAsArgument(&'static str),
 }
 
-impl std::fmt::Display for ArgumentError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            ArgumentError::SetOf => {
-                write!(f, "Cannot use SetOfIterator as an argument")
-            }
-            ArgumentError::Table => {
-                write!(f, "Cannot use TableIterator as an argument")
-            }
-            ArgumentError::BareU8 => {
-                write!(f, "Cannot use bare u8")
-            }
-            ArgumentError::SkipInArray => {
-                write!(f, "SqlMapping::Skip inside Array is not valid")
-            }
-            ArgumentError::Datum => {
-                write!(f, "A Datum as an argument means that `sql = \"...\"` must be set in the declaration")
-            }
-            ArgumentError::NotValidAsArgument(type_name) => {
-                write!(f, "`{}` is not able to be used as a function argument", type_name)
-            }
-        }
-    }
-}
-
 /// Describes ways that Rust types are mapped into SQL
 #[derive(Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
 pub enum SqlMapping {
@@ -75,8 +56,6 @@ impl SqlMapping {
     }
 }
 
-impl Error for ArgumentError {}
-
 /**
 A value which can be represented in SQL
 

pgrx/src/datum/numeric_support/error.rs

@@ -7,32 +7,22 @@
 //LICENSE All rights reserved.
 //LICENSE
 //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
-use std::fmt;
-use std::fmt::{Display, Formatter};
+use thiserror::Error;
 
 /// Represents some kind of conversion error when working with Postgres numerics
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Debug, Eq, PartialEq, Error)]
 #[non_exhaustive]
 pub enum Error {
     /// A conversion to Numeric would produce a value outside the precision and scale constraints
     /// of the target Numeric
+    #[error("{0}")]
     OutOfRange(String),
 
     /// A provided value is not also a valid Numeric
+    #[error("{0}")]
     Invalid(String),
 
     /// Postgres versions less than 14 do not support `Infinity` and `-Infinity` values
+    #[error("{0}")]
     ConversionNotSupported(String),
 }
-
-impl Display for Error {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            Error::OutOfRange(s) => write!(f, "{}", s),
-            Error::Invalid(s) => write!(f, "{}", s),
-            Error::ConversionNotSupported(s) => write!(f, "{}", s),
-        }
-    }
-}
-
-impl std::error::Error for Error {}