Do not include error source() in display() format.

According to doc/Errors.md, and in keeping with current best
practices, we should not include display an error's `source()` as
part of that error's display method.  Instead, we should let the
caller decide to call source() and display that error in turn.

Part of #323.
This commit is contained in:
Nick Mathewson 2022-06-21 14:14:14 -04:00
parent d95f597323
commit 08d9bbf33b
9 changed files with 38 additions and 37 deletions

View File

@ -109,11 +109,11 @@ pub_if_error_detail! {
#[non_exhaustive] #[non_exhaustive]
enum ErrorDetail { enum ErrorDetail {
/// Error setting up the channel manager /// Error setting up the channel manager
#[error("Error setting up the channel manager {0}")] #[error("Error setting up the channel manager")]
ChanMgrSetup(#[source] tor_chanmgr::Error), // TODO should this be its own type? ChanMgrSetup(#[source] tor_chanmgr::Error), // TODO should this be its own type?
/// Error setting up the circuit manager /// Error setting up the circuit manager
#[error("Error setting up the circuit manager {0}")] #[error("Error setting up the circuit manager")]
CircMgrSetup(#[source] tor_circmgr::Error), // TODO should this be its own type? CircMgrSetup(#[source] tor_circmgr::Error), // TODO should this be its own type?
/// Failed to obtain exit circuit /// Failed to obtain exit circuit
@ -128,15 +128,15 @@ enum ErrorDetail {
}, },
/// Error while getting a circuit /// Error while getting a circuit
#[error("Directory state error {0}")] #[error("Directory state error")]
DirMgr(#[from] tor_dirmgr::Error), DirMgr(#[from] tor_dirmgr::Error),
/// A protocol error while launching a stream /// A protocol error while launching a stream
#[error("Protocol error while launching a stream: {0}")] #[error("Protocol error while launching a stream")]
Proto(#[from] tor_proto::Error), Proto(#[from] tor_proto::Error),
/// An error while interfacing with the persistent data layer. /// An error while interfacing with the persistent data layer.
#[error("Error from state manager: {0}")] #[error("Error from state manager")]
Persist(#[from] tor_persist::Error), Persist(#[from] tor_persist::Error),
/// We asked an exit to do something, and waited too long for an answer.. /// We asked an exit to do something, and waited too long for an answer..
@ -148,7 +148,7 @@ enum ErrorDetail {
OnionAddressNotSupported, OnionAddressNotSupported,
/// Unusable target address. /// Unusable target address.
#[error("Could not parse target address: {0}")] #[error("Could not parse target address")]
Address(#[from] crate::address::TorAddrError), Address(#[from] crate::address::TorAddrError),
/// Hostname not valid. /// Hostname not valid.
@ -160,11 +160,11 @@ enum ErrorDetail {
LocalAddress, LocalAddress,
/// Building configuration for the client failed. /// Building configuration for the client failed.
#[error("Configuration failed: {0}")] #[error("Configuration failed")]
Configuration(#[from] tor_config::ConfigBuildError), Configuration(#[from] tor_config::ConfigBuildError),
/// Unable to change configuration. /// Unable to change configuration.
#[error("Reconfiguration failed: {0}")] #[error("Reconfiguration failed")]
Reconfigure(#[from] tor_config::ReconfigureError), Reconfigure(#[from] tor_config::ReconfigureError),
/// Unable to spawn task /// Unable to spawn task

View File

@ -99,7 +99,7 @@ pub enum Error {
/// A field was missing when we tried to construct a /// A field was missing when we tried to construct a
/// [`Mistrust`](crate::Mistrust). /// [`Mistrust`](crate::Mistrust).
#[error("Missing field: {0}")] #[error("Missing field when constructing Mistrust")]
MissingField(#[from] derive_builder::UninitializedFieldError), MissingField(#[from] derive_builder::UninitializedFieldError),
/// A group that we were configured to trust could not be found. /// A group that we were configured to trust could not be found.

View File

@ -12,13 +12,13 @@ use tor_error::{ErrorKind, HasKind};
pub enum Error { pub enum Error {
/// An error that occurred in the tor_bytes crate while decoding an /// An error that occurred in the tor_bytes crate while decoding an
/// object. /// object.
#[error("parsing error: {0}")] #[error("parsing error")]
BytesErr(#[from] tor_bytes::Error), BytesErr(#[from] tor_bytes::Error),
/// There was a programming error somewhere in the code. /// There was a programming error somewhere in the code.
#[error("Internal programming error: {0}")] #[error("Internal programming error")]
Internal(tor_error::Bug), Internal(tor_error::Bug),
/// Protocol violation at the channel level /// Protocol violation at the channel level
#[error("channel protocol violation: {0}")] #[error("channel protocol violation")]
ChanProto(String), ChanProto(String),
/// Tried to make or use a stream to an invalid destination address. /// Tried to make or use a stream to an invalid destination address.
#[error("invalid stream target address")] #[error("invalid stream target address")]

View File

@ -14,7 +14,7 @@ use tor_proto::ClockSkew;
#[non_exhaustive] #[non_exhaustive]
pub enum Error { pub enum Error {
/// A ChanTarget was given for which no channel could be built. /// A ChanTarget was given for which no channel could be built.
#[error("Target was unusable: {0}")] #[error("Bug: Target was unusable")]
UnusableTarget(#[source] tor_error::Bug), UnusableTarget(#[source] tor_error::Bug),
/// We were waiting on a pending channel, but it didn't succeed. /// We were waiting on a pending channel, but it didn't succeed.
@ -68,7 +68,7 @@ pub enum Error {
cause: Arc<SpawnError>, cause: Arc<SpawnError>,
}, },
/// An internal error of some kind that should never occur. /// An internal error of some kind that should never occur.
#[error("Internal error: {0}")] #[error("Internal error")]
Internal(#[from] tor_error::Bug), Internal(#[from] tor_error::Bug),
} }

View File

@ -29,7 +29,7 @@ pub enum Error {
/// We were waiting on a pending circuits, but it failed. /// We were waiting on a pending circuits, but it failed.
#[error("Pending circuit failed.")] #[error("Pending circuit failed.")]
PendingFailed(Box<Error>), PendingFailed(#[source] Box<Error>),
/// We were told that we could use a given circuit, but before we got a /// We were told that we could use a given circuit, but before we got a
/// chance to try it, its usage changed so that we had no longer find /// chance to try it, its usage changed so that we had no longer find
@ -79,11 +79,11 @@ pub enum Error {
NoExit(String), NoExit(String),
/// Problem creating or updating a guard manager. /// Problem creating or updating a guard manager.
#[error("Problem creating or updating guards list: {0}")] #[error("Problem creating or updating guards list")]
GuardMgr(#[source] tor_guardmgr::GuardMgrError), GuardMgr(#[source] tor_guardmgr::GuardMgrError),
/// Problem selecting a guard relay. /// Problem selecting a guard relay.
#[error("Unable to select a guard relay: {0}")] #[error("Unable to select a guard relay")]
Guard(#[from] tor_guardmgr::PickGuardError), Guard(#[from] tor_guardmgr::PickGuardError),
/// Unable to get or build a circuit, despite retrying. /// Unable to get or build a circuit, despite retrying.
@ -128,12 +128,12 @@ pub enum Error {
}, },
/// Problem loading or storing persistent state. /// Problem loading or storing persistent state.
#[error("Problem loading or storing state: {0}")] #[error("Problem loading or storing state")]
State(#[from] tor_persist::Error), State(#[from] tor_persist::Error),
/// An error caused by a programming issue . or a failure in another /// An error caused by a programming issue . or a failure in another
/// library that we can't work around. /// library that we can't work around.
#[error("Programming issue: {0}")] #[error("Programming error")]
Bug(#[from] Bug), Bug(#[from] Bug),
} }

View File

@ -14,7 +14,7 @@ use crate::SourceInfo;
#[non_exhaustive] #[non_exhaustive]
pub enum Error { pub enum Error {
/// Error while getting a circuit /// Error while getting a circuit
#[error("Error while getting a circuit {0}")] #[error("Error while getting a circuit")]
CircMgr(#[from] tor_circmgr::Error), CircMgr(#[from] tor_circmgr::Error),
/// An error that has occurred after we have contacted a directory cache and made a circuit to it. /// An error that has occurred after we have contacted a directory cache and made a circuit to it.
@ -50,11 +50,11 @@ pub enum RequestError {
Utf8Encoding(#[from] std::string::FromUtf8Error), Utf8Encoding(#[from] std::string::FromUtf8Error),
/// Io error while reading on connection /// Io error while reading on connection
#[error("IO error: {0}")] #[error("IO error")]
IoError(#[source] Arc<std::io::Error>), IoError(#[source] Arc<std::io::Error>),
/// A protocol error while launching a stream /// A protocol error while launching a stream
#[error("Protocol error while launching a stream: {0}")] #[error("Protocol error while launching a stream")]
Proto(#[from] tor_proto::Error), Proto(#[from] tor_proto::Error),
/// Error when parsing http /// Error when parsing http

View File

@ -25,7 +25,7 @@ pub enum Error {
#[error("corrupt cache: {0}")] #[error("corrupt cache: {0}")]
CacheCorruption(&'static str), CacheCorruption(&'static str),
/// rusqlite gave us an error. /// rusqlite gave us an error.
#[error("sqlite error: {0}")] #[error("sqlite error")]
SqliteError(#[source] Arc<rusqlite::Error>), SqliteError(#[source] Arc<rusqlite::Error>),
/// A schema version that says we can't read it. /// A schema version that says we can't read it.
#[error("unrecognized data storage schema")] #[error("unrecognized data storage schema")]
@ -48,7 +48,7 @@ pub enum Error {
#[error("storage error: {0}")] #[error("storage error: {0}")]
StorageError(String), StorageError(String),
/// An error given by the consensus diff crate. /// An error given by the consensus diff crate.
#[error("consdiff error: {0}")] #[error("consdiff error")]
ConsensusDiffError(#[from] tor_consdiff::Error), ConsensusDiffError(#[from] tor_consdiff::Error),
/// Invalid UTF8 in directory response. /// Invalid UTF8 in directory response.
#[error("invalid utf-8 from directory server")] #[error("invalid utf-8 from directory server")]
@ -85,13 +85,13 @@ pub enum Error {
#[error("object expired or not yet valid.")] #[error("object expired or not yet valid.")]
UntimelyObject(#[from] tor_checkable::TimeValidityError), UntimelyObject(#[from] tor_checkable::TimeValidityError),
/// An error given by dirclient /// An error given by dirclient
#[error("dirclient error: {0}")] #[error("dirclient error")]
DirClientError(#[from] tor_dirclient::Error), DirClientError(#[from] tor_dirclient::Error),
/// An error given by the checkable crate. /// An error given by the checkable crate.
#[error("checkable error: {0}")] #[error("checkable error")]
SignatureError(#[source] Arc<signature::Error>), SignatureError(#[source] Arc<signature::Error>),
/// An IO error occurred while manipulating storage on disk. /// An IO error occurred while manipulating storage on disk.
#[error("IO error: {0}")] #[error("IO error")]
IOError(#[source] Arc<std::io::Error>), IOError(#[source] Arc<std::io::Error>),
/// An attempt was made to bootstrap a `DirMgr` created in offline mode. /// An attempt was made to bootstrap a `DirMgr` created in offline mode.
#[error("cannot bootstrap offline DirMgr")] #[error("cannot bootstrap offline DirMgr")]
@ -121,7 +121,7 @@ pub enum Error {
}, },
/// A programming problem, either in our code or the code calling it. /// A programming problem, either in our code or the code calling it.
#[error("programming problem: {0}")] #[error("programming problem")]
Bug(#[from] tor_error::Bug), Bug(#[from] tor_error::Bug),
} }

View File

@ -511,17 +511,18 @@ pub(crate) mod test {
#[test] #[test]
fn send_bad() { fn send_bad() {
tor_rtcompat::test_with_all_runtimes!(|_rt| async move { tor_rtcompat::test_with_all_runtimes!(|_rt| async move {
use std::error::Error;
let chan = fake_channel(fake_channel_details()); let chan = fake_channel(fake_channel_details());
let cell = ChanCell::new(7.into(), msg::Created2::new(&b"hihi"[..]).into()); let cell = ChanCell::new(7.into(), msg::Created2::new(&b"hihi"[..]).into());
let e = chan.check_cell(&cell); let e = chan.check_cell(&cell);
assert!(e.is_err()); assert!(e.is_err());
assert!(format!("{}", e.unwrap_err()) assert!(format!("{}", e.unwrap_err().source().unwrap())
.contains("Can't send CREATED2 cell on client channel")); .contains("Can't send CREATED2 cell on client channel"));
let cell = ChanCell::new(0.into(), msg::Certs::new_empty().into()); let cell = ChanCell::new(0.into(), msg::Certs::new_empty().into());
let e = chan.check_cell(&cell); let e = chan.check_cell(&cell);
assert!(e.is_err()); assert!(e.is_err());
assert!(format!("{}", e.unwrap_err()) assert!(format!("{}", e.unwrap_err().source().unwrap())
.contains("Can't send CERTS cell after handshake is done")); .contains("Can't send CERTS cell after handshake is done"));
let cell = ChanCell::new(5.into(), msg::Create2::new(2, &b"abc"[..]).into()); let cell = ChanCell::new(5.into(), msg::Create2::new(2, &b"abc"[..]).into());

View File

@ -14,17 +14,17 @@ use tor_error::{ErrorKind, HasKind};
pub enum Error { pub enum Error {
/// An error that occurred in the tor_bytes crate while decoding an /// An error that occurred in the tor_bytes crate while decoding an
/// object. /// object.
#[error("parsing error: {0}")] #[error("parsing error")]
BytesErr(#[from] tor_bytes::Error), BytesErr(#[from] tor_bytes::Error),
/// An error that occurred from the io system when using a /// An error that occurred from the io system when using a
/// channel. /// channel.
#[error("io error on channel: {0}")] #[error("io error on channel")]
ChanIoErr(#[source] Arc<std::io::Error>), ChanIoErr(#[source] Arc<std::io::Error>),
/// An error from the io system that occurred when trying to connect a channel. /// An error from the io system that occurred when trying to connect a channel.
#[error("io error in handshake: {0}")] #[error("io error in handshake")]
HandshakeIoErr(#[source] Arc<std::io::Error>), HandshakeIoErr(#[source] Arc<std::io::Error>),
/// An error occurred in the cell-handling layer. /// An error occurred in the cell-handling layer.
#[error("cell encoding error: {0}")] #[error("cell encoding error")]
CellErr(#[source] tor_cell::Error), CellErr(#[source] tor_cell::Error),
/// We tried to produce too much output for some function. /// We tried to produce too much output for some function.
#[error("couldn't produce that much output")] #[error("couldn't produce that much output")]
@ -88,11 +88,11 @@ pub enum Error {
#[error("channel mismatch: {0}")] #[error("channel mismatch: {0}")]
ChanMismatch(String), ChanMismatch(String),
/// There was a programming error somewhere in our code, or the calling code. /// There was a programming error somewhere in our code, or the calling code.
#[error("Programming error: {0}")] #[error("Programming error")]
Bug(#[from] tor_error::Bug), Bug(#[from] tor_error::Bug),
/// Remote DNS lookup failed. /// Remote DNS lookup failed.
#[error("remote resolve failed: {0}")] #[error("remote resolve failed")]
ResolveError(ResolveError), ResolveError(#[source] ResolveError),
} }
/// Details about an error received while resolving a domain /// Details about an error received while resolving a domain