Refactor CurrentTimings

src/config.rs

@@ -1,14 +1,12 @@
 use std::fmt;
-use std::sync::Arc;
 use std::time::Duration;
 
 use hoot::client::flow::RedirectAuthHeaders;
 use http::Uri;
 
 use crate::middleware::MiddlewareChain;
 use crate::resolver::IpFamily;
-use crate::transport::time::{Instant, NextTimeout};
-use crate::{Proxy, TimeoutReason};
+use crate::Proxy;
 
 #[cfg(feature = "_tls")]
 use crate::tls::TlsConfig;
@@ -325,159 +323,3 @@ impl fmt::Debug for Timeouts {
             .finish()
     }
 }
-
-#[derive(Debug, Default)]
-pub(crate) struct CallTimings {
-    pub timeouts: Timeouts,
-    pub current_time: CurrentTime,
-    pub time_global_start: Option<Instant>,
-    pub time_call_start: Option<Instant>,
-    pub time_resolve: Option<Instant>,
-    pub time_connect: Option<Instant>,
-    pub time_send_request: Option<Instant>,
-    pub time_send_body: Option<Instant>,
-    pub time_await_100: Option<Instant>,
-    pub time_recv_response: Option<Instant>,
-    pub time_recv_body: Option<Instant>,
-}
-
-#[derive(Clone)]
-pub(crate) struct CurrentTime(Arc<dyn Fn() -> Instant + Send + Sync + 'static>);
-
-impl CurrentTime {
-    pub(crate) fn now(&self) -> Instant {
-        self.0()
-    }
-}
-
-impl CallTimings {
-    pub(crate) fn now(&self) -> Instant {
-        self.current_time.now()
-    }
-
-    pub(crate) fn record_timeout(&mut self, reason: TimeoutReason) {
-        match reason {
-            TimeoutReason::Global => {
-                let now = self.now();
-                if self.time_global_start.is_none() {
-                    self.time_global_start = Some(now);
-                }
-                self.time_call_start = Some(now);
-            }
-            TimeoutReason::Resolver => {
-                self.time_resolve = Some(self.now());
-            }
-            TimeoutReason::OpenConnection => {
-                self.time_connect = Some(self.now());
-            }
-            TimeoutReason::SendRequest => {
-                self.time_send_request = Some(self.now());
-            }
-            TimeoutReason::SendBody => {
-                self.time_send_body = Some(self.now());
-            }
-            TimeoutReason::Await100 => {
-                self.time_await_100 = Some(self.now());
-            }
-            TimeoutReason::RecvResponse => {
-                self.time_recv_response = Some(self.now());
-            }
-            TimeoutReason::RecvBody => {
-                self.time_recv_body = Some(self.now());
-            }
-        }
-    }
-
-    pub(crate) fn next_timeout(&self, reason: TimeoutReason) -> NextTimeout {
-        // self.time_xxx unwraps() below are OK. If the unwrap fails, we have a state
-        // bug where we progressed to a certain state without setting the corresponding time.
-        let timeouts = &self.timeouts;
-
-        let expire_at = match reason {
-            TimeoutReason::Global => timeouts
-                .global
-                .map(|t| self.time_global_start.unwrap() + t.into()),
-            TimeoutReason::Resolver => timeouts
-                .resolve
-                .map(|t| self.time_call_start.unwrap() + t.into()),
-            TimeoutReason::OpenConnection => timeouts
-                .connect
-                .map(|t| self.time_resolve.unwrap() + t.into()),
-            TimeoutReason::SendRequest => timeouts
-                .send_request
-                .map(|t| self.time_connect.unwrap() + t.into()),
-            TimeoutReason::SendBody => timeouts
-                .send_body
-                .map(|t| self.time_send_request.unwrap() + t.into()),
-            TimeoutReason::Await100 => timeouts
-                .await_100
-                .map(|t| self.time_send_request.unwrap() + t.into()),
-            TimeoutReason::RecvResponse => timeouts.recv_response.map(|t| {
-                // The fallback order is important. See state diagram in hoot.
-                self.time_send_body
-                    .or(self.time_await_100)
-                    .or(self.time_send_request)
-                    .unwrap()
-                    + t.into()
-            }),
-            TimeoutReason::RecvBody => timeouts
-                .recv_body
-                .map(|t| self.time_recv_response.unwrap() + t.into()),
-        }
-        .unwrap_or(Instant::NotHappening);
-
-        let global_at = self.global_timeout();
-
-        let (at, reason) = if global_at < expire_at {
-            (global_at, TimeoutReason::Global)
-        } else {
-            (expire_at, reason)
-        };
-
-        let after = at.duration_since(self.now());
-
-        NextTimeout { after, reason }
-    }
-
-    fn global_timeout(&self) -> Instant {
-        let global_start = self.time_global_start.unwrap();
-        let call_start = self.time_call_start.unwrap();
-
-        let global_at = global_start
-            + self
-                .timeouts
-                .global
-                .map(|t| t.into())
-                .unwrap_or(crate::transport::time::Duration::NotHappening);
-
-        let call_at = call_start
-            + self
-                .timeouts
-                .per_call
-                .map(|t| t.into())
-                .unwrap_or(crate::transport::time::Duration::NotHappening);
-
-        global_at.min(call_at)
-    }
-
-    pub(crate) fn new_call(self) -> CallTimings {
-        CallTimings {
-            timeouts: self.timeouts,
-            time_global_start: self.time_global_start,
-            current_time: self.current_time,
-            ..Default::default()
-        }
-    }
-}
-
-impl fmt::Debug for CurrentTime {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_tuple("CurrentTime").finish()
-    }
-}
-
-impl Default for CurrentTime {
-    fn default() -> Self {
-        Self(Arc::new(Instant::now))
-    }
-}

src/error.rs

@@ -1,7 +1,9 @@
-use std::{fmt, io};
+use std::io;
 
 use thiserror::Error;
 
+use crate::Timeout;
+
 /// Errors from ureq.
 #[derive(Debug, Error)]
 #[non_exhaustive]
@@ -38,7 +40,7 @@ pub enum Error {
     ///
     /// By default no timeouts are set, which means this error can't happen.
     #[error("timeout: {0}")]
-    Timeout(TimeoutReason),
+    Timeout(Timeout),
 
     /// Error when resolving a hostname fails.
     #[error("host not found")]
@@ -183,53 +185,6 @@ impl Error {
     }
 }
 
-/// Motivation for an [`Error::Timeout`].
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-#[non_exhaustive]
-pub enum TimeoutReason {
-    /// Timeout for entire call.
-    Global,
-
-    /// Timeout in the resolver.
-    Resolver,
-
-    /// Timeout while opening the connection.
-    OpenConnection,
-
-    /// Timeout while sending the request headers.
-    SendRequest,
-
-    /// Timeout when sending then request body.
-    SendBody,
-
-    /// Internal value never seen outside ureq (since awaiting 100 is expected
-    /// to timeout).
-    #[doc(hidden)]
-    Await100,
-
-    /// Timeout while receiving the response headers.
-    RecvResponse,
-
-    /// Timeout while receiving the response body.
-    RecvBody,
-}
-
-impl fmt::Display for TimeoutReason {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let r = match self {
-            TimeoutReason::Global => "global",
-            TimeoutReason::Resolver => "resolver",
-            TimeoutReason::OpenConnection => "open connection",
-            TimeoutReason::SendRequest => "send request",
-            TimeoutReason::SendBody => "send body",
-            TimeoutReason::Await100 => "await 100",
-            TimeoutReason::RecvResponse => "receive response",
-            TimeoutReason::RecvBody => "receive body",
-        };
-        write!(f, "{}", r)
-    }
-}
-
 impl From<io::Error> for Error {
     fn from(e: io::Error) -> Self {
         let is_wrapped_ureq_error = e.get_ref().map(|x| x.is::<Error>()).unwrap_or(false);

src/lib.rs

@@ -340,6 +340,7 @@ mod proxy;
 mod request;
 mod run;
 mod send_body;
+mod timings;
 mod util;
 
 pub mod middleware;
@@ -355,8 +356,9 @@ mod cookies;
 pub use cookies::{Cookie, CookieJar};
 
 pub use agent::Agent;
-pub use error::{Error, TimeoutReason};
+pub use error::Error;
 pub use send_body::SendBody;
+pub use timings::Timeout;
 
 /// Run a [`http::Request<impl AsSendBody>`].
 pub fn run(request: Request<impl AsSendBody>) -> Result<Response<Body>, Error> {

src/pool.rs

@@ -6,8 +6,8 @@ use http::uri::{Authority, Scheme};
 use http::Uri;
 
 use crate::proxy::Proxy;
-use crate::transport::time::{Duration, Instant, NextTimeout};
-use crate::transport::{Buffers, ConnectionDetails, Connector, Transport};
+use crate::transport::time::{Duration, Instant};
+use crate::transport::{Buffers, ConnectionDetails, Connector, NextTimeout, Transport};
 use crate::util::DebugAuthority;
 use crate::{AgentConfig, Error};
 

src/resolver.rs

@@ -18,7 +18,7 @@ use http::uri::{Authority, Scheme};
 use http::Uri;
 use smallvec::{smallvec, SmallVec};
 
-use crate::transport::time::NextTimeout;
+use crate::transport::NextTimeout;
 use crate::util::{SchemeExt, UriExt};
 use crate::{AgentConfig, Error};
 
@@ -187,7 +187,7 @@ mod test {
                 &config,
                 NextTimeout {
                     after: Duration::NotHappening,
-                    reason: crate::TimeoutReason::Global,
+                    reason: crate::Timeout::Global,
                 },
             )
             .unwrap_err();