From 2101dd5e39eae16d2763b4e54da96d7c8e273217 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Fri, 4 Feb 2022 13:20:54 +0000
Subject: [PATCH] errors: Introduce error_detail feature

Right now we must always expose the `Error` type since we haven't
converted everything.
---
 crates/arti-client/Cargo.toml |  1 +
 crates/arti-client/README.md  |  3 +++
 crates/arti-client/src/err.rs | 20 ++++++++++++++++++--
 crates/arti-client/src/lib.rs | 18 +++++++++++++++++-
 4 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/crates/arti-client/Cargo.toml b/crates/arti-client/Cargo.toml
index 13dc52e0c..d8f46a54d 100644
--- a/crates/arti-client/Cargo.toml
+++ b/crates/arti-client/Cargo.toml
@@ -17,6 +17,7 @@ tokio = [ "tor-rtcompat/tokio", "tor-proto/tokio" ]
 native-tls = [ "tor-rtcompat/native-tls" ]
 rustls = [ "tor-rtcompat/rustls" ]
 static = [ "tor-rtcompat/static", "tor-dirmgr/static" ]
+error_detail = [ ]
 
 # Enable experimental APIs that are not yet officially supported.
 #
diff --git a/crates/arti-client/README.md b/crates/arti-client/README.md
index 8ff5309dd..04674b414 100644
--- a/crates/arti-client/README.md
+++ b/crates/arti-client/README.md
@@ -138,4 +138,7 @@ including sqlite and/or openssl.
 Note that these APIs are NOT covered by semantic versioning guarantees:
 we might break them or remove them between patch versions.
 
+`error_detail` -- Make the `TorError` type transparent, and expose the `Error` within.
+Note that the resulting APIs are not stable.
+
 License: MIT OR Apache-2.0
diff --git a/crates/arti-client/src/err.rs b/crates/arti-client/src/err.rs
index 87fed2d41..cb617480d 100644
--- a/crates/arti-client/src/err.rs
+++ b/crates/arti-client/src/err.rs
@@ -9,6 +9,10 @@ use thiserror::Error;
 use tor_error::{ErrorKind, HasKind};
 use tor_rtcompat::TimeoutError;
 
+/// Wrapper for definitions which need to vary according to `error_details`
+macro_rules! define_according_to_cfg_error_details { { $vis:vis } => {
+// We cheat with the indentation, a bit.  Happily rustfmt doesn't seem to mind.
+
 /// Main high-level error type for the Arti Tor client
 ///
 /// If you need to handle different errors differently,
@@ -16,18 +20,21 @@ use tor_rtcompat::TimeoutError;
 /// to check what kind of error it is,
 #[derive(Error, Debug)]
 // TODO #[derive(Clone)] // we need to make everything inside Clone first
+#[allow(clippy::exhaustive_structs)]
 pub struct TorError {
     /// The actual error
     #[from]
-    detail: Error,
+    $vis detail: Error,
 }
 
 /// Alias for the [`Result`] type used within the `arti_client` crate.
-pub type Result<T> = std::result::Result<T, Error>;
+$vis type Result<T> = std::result::Result<T, Error>;
 
 /// Represents errors that can occur while doing Tor operations.
 #[derive(Error, Debug)]
 #[non_exhaustive]
+// should be $vis
+// but right now we need to re-export it unconditionally
 pub enum Error {
     /// Error while getting a circuit
     #[error("Error while getting a circuit {0}")]
@@ -82,6 +89,9 @@ pub enum Error {
     Spawn(#[from] Arc<SpawnError>),
 }
 
+// End of the use of $vis to refer to visibility according to `error_detail`
+} }
+
 impl Display for TorError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "tor: {}: {}", self.detail.kind(), &self.detail)
@@ -111,3 +121,9 @@ impl tor_error::HasKind for Error {
         ErrorKind::TODO
     }
 }
+
+#[cfg(feature = "error_detail")]
+define_according_to_cfg_error_details! { pub }
+
+#[cfg(not(feature = "error_detail"))]
+define_according_to_cfg_error_details! { pub(crate) }
diff --git a/crates/arti-client/src/lib.rs b/crates/arti-client/src/lib.rs
index 9d93a9ccb..7106fa51a 100644
--- a/crates/arti-client/src/lib.rs
+++ b/crates/arti-client/src/lib.rs
@@ -188,7 +188,23 @@ pub use tor_circmgr::IsolationToken;
 pub use tor_proto::stream::{DataReader, DataStream, DataWriter};
 
 mod err;
-pub use err::{Error, Result, TorError};
+pub use err::TorError;
+
+// Ideally these two `use`s would be pub(crate) or pub, depending on the error_detail
+// feature.  But varying visibility according to a cargo feature is awkward (see
+// how it's done in err.rs), and we want to avoid doing it unless we have to.
+//
+// So, we just conditionally `use`.  This means that other bits of this crate can't
+// name them by crate::Error and crate::Result, which is slightly annoying.
+
+//#[cfg(feature = "error_detail")] TODO
+// Right now if we hide this, we get
+// error[E0446]: crate-private type `err::Error` in public interface
+// and also failures in `arti`
+pub use err::Error;
+
+#[cfg(feature = "error_detail")]
+pub use err::Result;
 
 /// Alias for the [`Result`] type corredponding to the high-level `TorError`
 pub type TorResult<T> = std::result::Result<T, TorError>;