circmgr: Use event_report!() and similar macros

This simplifies our logging a little, and implements part of

crates/tor-circmgr/Cargo.toml

@@ -72,7 +72,7 @@ thiserror = "1"
 tor-basic-utils = { path = "../tor-basic-utils", version = "0.7.1" }
 tor-chanmgr = { path = "../tor-chanmgr", version = "0.9.1" }
 tor-config = { path = "../tor-config", version = "0.9.2" }
-tor-error = { path = "../tor-error", version = "0.5.2" }
+tor-error = { path = "../tor-error", version = "0.5.2", features = ["tracing"] }
 tor-guardmgr = { path = "../tor-guardmgr", version = "0.9.1" }
 tor-hscrypto = { path = "../tor-hscrypto", version = "0.3.0", optional = true }
 tor-linkspec = { path = "../tor-linkspec", version = "0.8.1" }

crates/tor-circmgr/src/build.rs

@@ -13,13 +13,12 @@ use std::sync::{
 };
 use std::time::{Duration, Instant};
 use tor_chanmgr::{ChanMgr, ChanProvenance, ChannelUsage};
-use tor_error::ErrorReport;
+use tor_error::warn_report;
 use tor_guardmgr::GuardStatus;
 use tor_linkspec::{ChanTarget, IntoOwnedChanTarget, OwnedChanTarget, OwnedCircTarget};
 use tor_netdir::params::NetParameters;
 use tor_proto::circuit::{CircParameters, ClientCirc, PendingClientCirc};
 use tor_rtcompat::{Runtime, SleepProviderExt};
-use tracing::warn;
 
 mod guardstatus;
 
@@ -470,7 +469,7 @@ impl<R: Runtime> CircuitBuilder<R> {
 pub fn circparameters_from_netparameters(inp: &NetParameters) -> CircParameters {
     let mut p = CircParameters::default();
     if let Err(e) = p.set_initial_send_window(inp.circuit_window.get() as u16) {
-        warn!("Invalid parameter in directory: {}", e.report());
+        warn_report!(e, "Invalid parameter in directory");
     }
     p.set_extend_by_ed25519_id(inp.extend_by_ed25519_id.into());
     p

crates/tor-circmgr/src/hspool.rs

@@ -11,7 +11,8 @@ use std::{
 use crate::{timeouts, CircMgr, Error, Result};
 use futures::{task::SpawnExt, StreamExt, TryFutureExt};
 use once_cell::sync::OnceCell;
-use tor_error::{bad_api_usage, internal, ErrorReport};
+use tor_error::debug_report;
+use tor_error::{bad_api_usage, internal};
 use tor_linkspec::{CircTarget, OwnedCircTarget};
 use tor_netdir::{NetDir, NetDirProvider, Relay, SubnetConfig};
 use tor_proto::circuit::{self, ClientCirc};
@@ -19,7 +20,7 @@ use tor_rtcompat::{
     scheduler::{TaskHandle, TaskSchedule},
     Runtime, SleepProviderExt,
 };
-use tracing::{debug, warn};
+use tracing::warn;
 
 /// The (onion-service-related) purpose for which a given circuit is going to be
 /// used.
@@ -435,10 +436,7 @@ async fn launch_hs_circuits_as_needed<R: Runtime>(
                         n_to_launch -= 1;
                     }
                     Err(err) => {
-                        debug!(
-                            "Unable to build preemptive circuit for onion services: {}",
-                            err.report()
-                        );
+                        debug_report!(err, "Unable to build preemptive circuit for onion services");
                     }
                 }
             } else {

crates/tor-circmgr/src/lib.rs

@@ -42,7 +42,7 @@
 
 use tor_basic_utils::retry::RetryDelay;
 use tor_chanmgr::ChanMgr;
-use tor_error::ErrorReport;
+use tor_error::{error_report, warn_report};
 use tor_linkspec::ChanTarget;
 use tor_netdir::{DirEvent, NetDir, NetDirProvider, Timeliness};
 use tor_proto::circuit::{CircParameters, ClientCirc, UniqId};
@@ -55,7 +55,7 @@ use futures::task::SpawnExt;
 use futures::StreamExt;
 use std::sync::{Arc, Mutex, Weak};
 use std::time::{Duration, Instant};
-use tracing::{debug, error, info, trace, warn};
+use tracing::{debug, info, trace, warn};
 
 #[cfg(feature = "testing")]
 pub use config::test_config::TestConfig;
@@ -492,11 +492,7 @@ impl<R: Runtime> CircMgr<R> {
                     trace!("Circuit already existed created for {:?}", circs[i]);
                 }
                 Err(e) => {
-                    warn!(
-                        "Failed to build preemptive circuit {:?}: {}",
-                        sv(&circs[i]),
-                        e.report(),
-                    );
+                    warn_report!(e, "Failed to build preemptive circuit {:?}", sv(&circs[i]));
                     n_errors += 1;
                 }
             }
@@ -665,10 +661,7 @@ impl<R: Runtime> CircMgr<R> {
             if let (Some(cm), Some(dm)) = (Weak::upgrade(&circmgr), Weak::upgrade(&dirmgr)) {
                 if let Ok(netdir) = dm.netdir(Timeliness::Unchecked) {
                     if let Err(e) = cm.launch_timeout_testing_circuit_if_appropriate(&netdir) {
-                        warn!(
-                            "Problem launching a timeout testing circuit: {}",
-                            e.report()
-                        );
+                        warn_report!(e, "Problem launching a timeout testing circuit");
                     }
                     let delay = netdir
                         .params()
@@ -707,25 +700,25 @@ impl<R: Runtime> CircMgr<R> {
 
                 match statemgr.try_lock() {
                     Err(e) => {
-                        error!("Problem with state lock file: {}", e.report());
+                        error_report!(e, "Problem with state lock file");
                         break;
                     }
                     Ok(NewlyAcquired) => {
                         info!("We now own the lock on our state files.");
                         if let Err(e) = circmgr.upgrade_to_owned_persistent_state() {
-                            error!("Unable to upgrade to owned state files: {}", e.report());
+                            error_report!(e, "Unable to upgrade to owned state files");
                             break;
                         }
                     }
                     Ok(AlreadyHeld) => {
                         if let Err(e) = circmgr.store_persistent_state() {
-                            error!("Unable to flush circmgr state: {}", e.report());
+                            error_report!(e, "Unable to flush circmgr state");
                             break;
                         }
                     }
                     Ok(NoLock) => {
                         if let Err(e) = circmgr.reload_persistent_state() {
-                            error!("Unable to reload circmgr state: {}", e.report());
+                            error_report!(e, "Unable to reload circmgr state");
                             break;
                         }
                     }
@@ -839,10 +832,7 @@ impl<R: Runtime> Drop for CircMgr<R> {
         match self.store_persistent_state() {
             Ok(true) => info!("Flushed persistent state at exit."),
             Ok(false) => debug!("Lock not held; no state to flush."),
-            Err(e) => error!(
-                "Unable to flush state on circuit manager drop: {}",
-                e.report()
-            ),
+            Err(e) => error_report!(e, "Unable to flush state on circuit manager drop"),
         }
     }
 }

crates/tor-circmgr/src/mgr.rs

@@ -29,7 +29,7 @@ use retry_error::RetryError;
 use tor_basic_utils::retry::RetryDelay;
 use tor_chanmgr::ChannelUsage;
 use tor_config::MutCfg;
-use tor_error::{internal, AbsRetryTime, ErrorReport, HasRetryTime};
+use tor_error::{debug_report, info_report, internal, warn_report, AbsRetryTime, HasRetryTime};
 use tor_rtcompat::{Runtime, SleepProviderExt};
 
 use async_trait::async_trait;
@@ -43,7 +43,7 @@ use std::hash::Hash;
 use std::panic::AssertUnwindSafe;
 use std::sync::{self, Arc, Weak};
 use std::time::{Duration, Instant};
-use tracing::{debug, info, warn};
+use tracing::{debug, warn};
 use weak_table::PtrWeakHashSet;
 
 mod streams;
@@ -876,10 +876,10 @@ impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
                 }
                 Err(e) => {
                     // We couldn't pick the action!
-                    debug!(
-                        "Couldn't pick action for circuit attempt {}: {}",
+                    debug_report!(
+                        &e,
+                        "Couldn't pick action for circuit attempt {}",
                         attempt_num,
-                        e.report(),
                     );
                     e
                 }
@@ -1146,18 +1146,18 @@ impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
                                     x => x,
                                 };
                                 if src == streams::Source::Left {
-                                    info!(
-                                        "{} suggested we use {:?}, but restrictions failed: {}",
+                                    info_report!(
+                                        &e,
+                                        "{} suggested we use {:?}, but restrictions failed",
                                         describe_source(building, src),
                                         id,
-                                        e.report(),
                                     );
                                 } else {
-                                    debug!(
-                                        "{} suggested we use {:?}, but restrictions failed: {}",
+                                    debug_report!(
+                                        &e,
+                                        "{} suggested we use {:?}, but restrictions failed",
                                         describe_source(building, src),
                                         id,
-                                        e.report(),
                                     );
                                 }
                                 record_error(&mut retry_error, src, building, e);
@@ -1523,7 +1523,7 @@ fn spawn_expiration_task<B, R>(
             };
             cm.expire_circ(&circ_id, exp_inst);
         }) {
-            warn!("Unable to launch expiration task: {}", e.report());
+            warn_report!(e, "Unable to launch expiration task");
         }
     }
 }

crates/tor-circmgr/src/timeouts/estimator.rs

@@ -8,7 +8,7 @@ use crate::timeouts::{
 use crate::TimeoutStateHandle;
 use std::sync::Mutex;
 use std::time::Duration;
-use tor_error::ErrorReport;
+use tor_error::warn_report;
 use tor_netdir::params::NetParameters;
 use tracing::{debug, warn};
 
@@ -138,7 +138,7 @@ fn estimator_from_storage(
         Ok(Some(v)) => v,
         Ok(None) => ParetoTimeoutState::default(),
         Err(e) => {
-            warn!("Unable to load timeout state: {}", e.report());
+            warn_report!(e, "Unable to load timeout state");
             return (true, Box::new(ReadonlyTimeoutEstimator::new()));
         }
     };