Commit 863f4593 authored by Nick Mathewson's avatar Nick Mathewson 🦀
Browse files

Add a "RetryError" to capture the idea of multiple failed attempts. 

When we try to do something a few times and it fails each time, it
can be a good idea to remember why the individual failures
happened.
parent 566b7a16

Cargo.toml

+1 −0
Original line number Original line Diff line number Diff line
@@ -14,6 +14,7 @@ members = [ 
    "tor-linkspec",

    "tor-linkspec",

    "tor-cell",

    "tor-cell",

    "tor-proto",

    "tor-proto",

    "tor-retry",

    "tor-netdoc",

    "tor-netdoc",

    "tor-netdir",

    "tor-netdir",

    "tor-chanmgr",

    "tor-chanmgr",

tor-dirmgr/Cargo.toml

+1 −0
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ tor-dirclient = { path="../tor-dirclient", version= "*" } 
tor-netdir = { path="../tor-netdir", version= "*" }

tor-netdir = { path="../tor-netdir", version= "*" }

tor-netdoc = { path="../tor-netdoc", version= "*" }

tor-netdoc = { path="../tor-netdoc", version= "*" }

tor-llcrypto = { path="../tor-llcrypto", version= "*" }

tor-llcrypto = { path="../tor-llcrypto", version= "*" }

tor-retry = { path="../tor-retry", version= "*" }

tor-rtcompat = { path="../tor-rtcompat", version= "*" }

tor-rtcompat = { path="../tor-rtcompat", version= "*" }





anyhow = "1.0.35"

anyhow = "1.0.35"

tor-dirmgr/src/lib.rs

+10 −16
Original line number Original line Diff line number Diff line
@@ -29,6 +29,7 @@ use tor_netdoc::doc::authcert::{AuthCert, AuthCertKeyIds}; 
use tor_netdoc::doc::microdesc::{MDDigest, Microdesc, MicrodescReader};

use tor_netdoc::doc::microdesc::{MDDigest, Microdesc, MicrodescReader};

use tor_netdoc::doc::netstatus::{MDConsensus, UnvalidatedMDConsensus};

use tor_netdoc::doc::netstatus::{MDConsensus, UnvalidatedMDConsensus};

use tor_netdoc::AllowAnnotations;

use tor_netdoc::AllowAnnotations;

use tor_retry::RetryError;





use anyhow::{anyhow, Result};

use anyhow::{anyhow, Result};

use async_rwlock::RwLock;

use async_rwlock::RwLock;
@@ -416,21 +417,20 @@ impl NoInformation { 
        let n_retries = 3_u32;

        let n_retries = 3_u32;

        let mut retry_delay = RetryDelay::default();

        let mut retry_delay = RetryDelay::default();





        let mut last_err: Option<anyhow::Error> = None;

        let mut errors = RetryError::while_doing("download a consensus");

        for _ in 0..n_retries {

        for _ in 0..n_retries {

            let cm = Arc::clone(&circmgr);

            let cm = Arc::clone(&circmgr);

            match self.fetch_consensus_once(config, store, info, cm).await {

            match self.fetch_consensus_once(config, store, info, cm).await {

                Ok(v) => return Ok(v),

                Ok(v) => return Ok(v),

                Err(e) => {

                Err(e) => {

                    last_err = Some(e);

                    errors.push(e);

                    let delay = retry_delay.next_delay(&mut rand::thread_rng());

                    let delay = retry_delay.next_delay(&mut rand::thread_rng());

                    tor_rtcompat::task::sleep(delay).await;

                    tor_rtcompat::task::sleep(delay).await;

                }

                }

            }

            }

        }

        }





        // TODO: XXXX-A1: don't forget all the other errors.

        Err(errors.into())

        Err(last_err.unwrap())

    }

    }





    /// Try to fetch a currently timely consensus directory document

    /// Try to fetch a currently timely consensus directory document
@@ -540,11 +540,11 @@ impl UnvalidatedDir { 
        let n_retries = 3_u32;

        let n_retries = 3_u32;

        let mut retry_delay = RetryDelay::default();

        let mut retry_delay = RetryDelay::default();





        let mut last_err: Option<anyhow::Error> = None;

        let mut errors = RetryError::while_doing("downloading authority certificates");

        for _ in 0..n_retries {

        for _ in 0..n_retries {

            let cm = Arc::clone(&circmgr);

            let cm = Arc::clone(&circmgr);

            if let Err(e) = self.fetch_certs_once(config, store, info, cm).await {

            if let Err(e) = self.fetch_certs_once(config, store, info, cm).await {

                last_err = Some(e);

                errors.push(e);

            }

            }





            if self.missing_certs(config).is_empty() {

            if self.missing_certs(config).is_empty() {
@@ -555,10 +555,7 @@ impl UnvalidatedDir { 
            tor_rtcompat::task::sleep(delay).await;

            tor_rtcompat::task::sleep(delay).await;

        }

        }





        match last_err {

        Err(errors.into())

            Some(e) => Err(e),

            None => Err(anyhow!("Couldn't get certs after retries.")),

        }

    }

    }





    /// Try to fetch authority certificates from the network.

    /// Try to fetch authority certificates from the network.
@@ -662,11 +659,11 @@ impl PartialDir { 
        let n_retries = 3_u32;

        let n_retries = 3_u32;

        let mut retry_delay = RetryDelay::default();

        let mut retry_delay = RetryDelay::default();





        let mut last_err: Option<anyhow::Error> = None;

        let mut errors = RetryError::while_doing("download microdescriptors");

        for _ in 0..n_retries {

        for _ in 0..n_retries {

            let cm = Arc::clone(&circmgr);

            let cm = Arc::clone(&circmgr);

            if let Err(e) = self.fetch_mds_once(store, info, cm).await {

            if let Err(e) = self.fetch_mds_once(store, info, cm).await {

                last_err = Some(e);

                errors.push(e);

            }

            }





            if self.dir.have_enough_paths() {

            if self.dir.have_enough_paths() {
@@ -677,10 +674,7 @@ impl PartialDir { 
            tor_rtcompat::task::sleep(delay).await;

            tor_rtcompat::task::sleep(delay).await;

        }

        }





        match last_err {

        Err(errors.into())

            Some(e) => Err(e),

            None => Err(anyhow!("Couldn't get microdescs after retries.")),

        }

    }

    }

    /// Try to fetch microdescriptors from the network.

    /// Try to fetch microdescriptors from the network.

    async fn fetch_mds_once(

    async fn fetch_mds_once(

tor-retry/Cargo.toml

0 → 100644
+11 −0
Original line number Original line Diff line number Diff line 
[package]

name = "tor-retry"

version = "0.0.0"

authors = ["Nick Mathewson <nickm@torproject.org>"]

edition = "2018"

license = "MIT OR Apache-2.0"

publish = false



[dependencies]



anyhow = "1.0.35"

tor-retry/src/lib.rs

0 → 100644
+134 −0
Original line number Original line Diff line number Diff line 
//! Helpers to implement retry-related functionality.

//!

//! Right now, this crate only has an error type that we use when we

//! retry something a few times, and they all fail.



#![deny(missing_docs)]

#![deny(clippy::missing_docs_in_private_items)]



use std::fmt::{Display, Error as FmtError, Formatter};



/// An error type for use when we're going to do something a few times,

/// and they might all fail.

///

/// To use this error type, initialize a new RetryError before you

/// start trying to do whatever it is.  Then, every time the operation

/// fails, use [`RetryError::push()`] to add a new error to the list

/// of errors.  If the operation fails too many times, you can use

/// RetryError as an [`Error`] itself.

#[derive(Debug, Default)]

pub struct RetryError {

    /// The operation we were trying to do.

    doing: Option<&'static str>,

    /// The errors that we encountered when doing the operation.

    // TODO: It might be nice to have this crate not depend on anyhow.

    // When I first tried to do that, though, I ran into a big pile of

    // type errors and gave up.

    errors: Vec<anyhow::Error>,

}



// TODO: Should we declare that some error is the 'source' of this one?

impl std::error::Error for RetryError {}



impl RetryError {

    /// Create a new RetryError, with no failed attempts.

    ///

    /// This RetryError should not be used as-is, since when no

    /// [`Error`]s have been pushed into it, it doesn't represent an

    /// actual failure.

    pub fn new() -> Self {

        RetryError {

            doing: None,

            errors: Vec::new(),

        }

    }

    /// Crate a new RetryError, with no failed attempts,

    ///

    /// The provided `doing` argument is a short string that describes

    /// what we were trying to do when we failed too many times.  It

    /// will be used to format the final error message; it should be a

    /// phrase that can go after "while trying to".

    ///

    /// As with [`RetryError::new()`], the result of this method

    /// shouln't be used before any errors are pushed into it.

    pub fn while_doing(doing: &'static str) -> Self {

        RetryError {

            doing: Some(doing),

            errors: Vec::new(),

        }

    }

    /// Add an error to this RetryError.

    ///

    /// You should call this method when an attempt at the underlying operation

    /// has failed.

    pub fn push<E>(&mut self, err: E)

    where

        E: Into<anyhow::Error>,

    {

        let e: anyhow::Error = err.into();

        self.errors.push(e);

    }

    /// Return an iterator over all of the reasons that the attempt

    /// behind this RetryError has failed.

    pub fn sources(&self) -> impl Iterator<Item = &anyhow::Error> {

        self.errors.iter()

    }

}



impl Display for RetryError {

    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> {

        match self.errors.len() {

            0 => writeln!(

                f,

                "Programming error: Somebody made a RetryError without any errors!"

            ),

            1 => self.errors[0].fmt(f),

            n => {

                match self.doing {

                    Some(doing) => writeln!(

                        f,

                        "Tried to {} {} times, but all attempts failed.",

                        doing, n

                    ),

                    None => writeln!(f, "Tried {} times, but all attempts failed.", n),

                }?;

                for (idx, e) in self.sources().enumerate() {

                    write!(f, "Attempt {}:\n{}\n", idx + 1, e)?;

                }

                Ok(())

            }

        }

    }

}



#[cfg(test)]

mod test {

    use super::*;



    #[test]

    fn bad_bools() {

        let mut err = RetryError::while_doing("convert some things");

        if let Err(e) = "maybe".parse::<bool>() {

            err.push(e);

        }

        if let Err(e) = "a few".parse::<u32>() {

            err.push(e);

        }

        if let Err(e) = "teh_g1b50n".parse::<std::net::IpAddr>() {

            err.push(e);

        }

        let disp = format!("{}", err);

        assert_eq!(

            disp,

            "\

Tried to convert some things 3 times, but all attempts failed.

Attempt 1:

provided string was not `true` or `false`

Attempt 2:

invalid digit found in string

Attempt 3:

invalid IP address syntax

"

        );

    }

}