From 4b5be3685f43f44ef3e200fb9302071b96e126ee Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Mon, 28 Nov 2022 14:11:06 -0500
Subject: [PATCH] ptmgr: Fill in some error-related code; resolve TODOs.

---
 crates/tor-error/src/lib.rs |  5 ++++
 crates/tor-ptmgr/src/err.rs | 46 ++++++++++++++++++++++++++++++++-----
 crates/tor-ptmgr/src/lib.rs | 12 ++++++++--
 3 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/crates/tor-error/src/lib.rs b/crates/tor-error/src/lib.rs
index bb661a3df..00f645284 100644
--- a/crates/tor-error/src/lib.rs
+++ b/crates/tor-error/src/lib.rs
@@ -341,6 +341,11 @@ pub enum ErrorKind {
     #[display(fmt = "local authentication refused.")]
     LocalProtocolFailed,
 
+    /// A problem occurred when launching or communicating with an external
+    /// process running on this computer.
+    #[display(fmt = "local plug-in tool failed")]
+    LocalPluginFailed,
+
     /// A relay had an identity other than the one we expected.
     ///
     /// This could indicate a MITM attack, but more likely indicates that the
diff --git a/crates/tor-ptmgr/src/err.rs b/crates/tor-ptmgr/src/err.rs
index 544f33f0f..cb05559b6 100644
--- a/crates/tor-ptmgr/src/err.rs
+++ b/crates/tor-ptmgr/src/err.rs
@@ -78,21 +78,55 @@ pub enum PtError {
         error: CfgPathError,
     },
     /// The pluggable transport reactor failed.
-    #[error("PT reactor failed")]
-    // TODO pt-client: This should just be a bug.
-    ReactorFailed,
+    #[error("Internal error")]
+    Internal(#[from] tor_error::Bug),
 }
 
-// TODO pt-client: implement.
 impl HasKind for PtError {
     fn kind(&self) -> ErrorKind {
-        todo!()
+        use ErrorKind as EK;
+        use PtError as E;
+        match self {
+            E::ClientTransportsUnsupported(_) => EK::InvalidConfig,
+            E::ChildProtocolViolation(_)
+            | E::ProtocolViolation(_)
+            | E::UnsupportedVersion
+            | E::IpcParseFailed { .. } => EK::LocalProtocolViolation,
+            E::Timeout
+            | E::ClientTransportFailed { .. }
+            | E::ChildGone
+            | E::ChildReadFailed(_)
+            | E::ChildSpawnFailed { .. }
+            | E::StdioUnavailable
+            | E::ProxyError(_) => EK::LocalPluginFailed,
+            E::TempdirCreateFailed(_) => EK::FsPermissions,
+            E::PathExpansionFailed { .. } => EK::InvalidConfig,
+            E::Internal(e) => e.kind(),
+        }
     }
 }
 
 impl HasRetryTime for PtError {
     fn retry_time(&self) -> RetryTime {
-        todo!()
+        use PtError as E;
+        use RetryTime as RT;
+        match self {
+            E::ClientTransportsUnsupported(_)
+            | E::ClientTransportFailed { .. }
+            | E::ChildProtocolViolation(_)
+            | E::ProtocolViolation(_)
+            | E::ChildSpawnFailed { .. }
+            | E::IpcParseFailed { .. }
+            | E::UnsupportedVersion
+            | E::Internal(_)
+            | E::PathExpansionFailed { .. } => RT::Never,
+            E::TempdirCreateFailed(_)
+            | E::StdioUnavailable
+            | E::Timeout
+            | E::ProxyError(_)
+            | E::ChildGone
+            | E::ChildReadFailed(_) => RT::AfterWaiting,
+        }
     }
 }
 
diff --git a/crates/tor-ptmgr/src/lib.rs b/crates/tor-ptmgr/src/lib.rs
index d83e38b84..52cbd0480 100644
--- a/crates/tor-ptmgr/src/lib.rs
+++ b/crates/tor-ptmgr/src/lib.rs
@@ -250,11 +250,19 @@ impl<R: Runtime> tor_chanmgr::factory::AbstractPtMgr for PtMgr<R> {
                         pt: transport.clone(),
                         result: tx,
                     })
-                    .map_err(|_| Arc::new(PtError::ReactorFailed) as Arc<dyn AbstractPtError>)?;
+                    .map_err(|_| {
+                        Arc::new(PtError::Internal(tor_error::internal!(
+                            "PT reactor closed unexpectedly"
+                        ))) as Arc<dyn AbstractPtError>
+                    })?;
                 cmethod = Some(
                     // NOTE(eta): Could be improved with result flattening.
                     rx.await
-                        .map_err(|_| Arc::new(PtError::ReactorFailed) as Arc<dyn AbstractPtError>)?
+                        .map_err(|_| {
+                            Arc::new(PtError::Internal(tor_error::internal!(
+                                "PT reactor closed unexpectedly"
+                            ))) as Arc<dyn AbstractPtError>
+                        })?
                         .map_err(|x| Arc::new(x) as Arc<dyn AbstractPtError>)?,
                 );
             } else {