From 69a4f49a88ef5001ff6468e08550b6a4baed8eab Mon Sep 17 00:00:00 2001
From: Ian Clarke <sanity@users.noreply.github.com>
Date: Wed, 11 Oct 2023 01:52:50 -0500
Subject: [PATCH] Propagate router estimation errors (#867)

* propagate estimation errors

* fix build

* format
---
 crates/core/src/router.rs                    | 27 +++++++++++++++++---
 crates/core/src/router/isotonic_estimator.rs | 10 +++++++-
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/crates/core/src/router.rs b/crates/core/src/router.rs
index 9d6ca0689..46f229aaa 100644
--- a/crates/core/src/router.rs
+++ b/crates/core/src/router.rs
@@ -5,7 +5,7 @@ mod util;
 use crate::ring::{Location, PeerKeyLocation};
 use isotonic_estimator::{EstimatorType, IsotonicEstimator, IsotonicEvent};
 use serde::Serialize;
-use std::time::Duration;
+use std::{fmt, time::Duration};
 use util::{Mean, TransferSpeed};
 
 #[derive(Debug, Clone, Serialize)]
@@ -196,15 +196,24 @@ impl Router {
         let time_to_response_start_estimate = self
             .response_start_time_estimator
             .estimate_retrieval_time(peer, contract_location)
-            .unwrap();
+            .map_err(|e| {
+                RoutingError::EstimationError(format!(
+                    "Response Start Time Estimation failed: {}",
+                    e
+                ))
+            })?;
         let failure_estimate = self
             .failure_estimator
             .estimate_retrieval_time(peer, contract_location)
-            .unwrap();
+            .map_err(|e| {
+                RoutingError::EstimationError(format!("Failure Estimation failed: {}", e))
+            })?;
         let transfer_rate_estimate = self
             .transfer_rate_estimator
             .estimate_retrieval_time(peer, contract_location)
-            .unwrap();
+            .map_err(|e| {
+                RoutingError::EstimationError(format!("Transfer Rate Estimation failed: {}", e))
+            })?;
 
         // This is a fairly naive approach, assuming that the cost of a failure is a multiple
         // of the cost of success.
@@ -233,6 +242,16 @@ impl Router {
 #[derive(Debug)]
 pub(crate) enum RoutingError {
     InsufficientDataError,
+    EstimationError(String),
+}
+
+impl fmt::Display for RoutingError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            RoutingError::InsufficientDataError => write!(f, "Insufficient data provided"),
+            RoutingError::EstimationError(err_msg) => write!(f, "Estimation error: {}", err_msg),
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy, Serialize)]
diff --git a/crates/core/src/router/isotonic_estimator.rs b/crates/core/src/router/isotonic_estimator.rs
index 300854e11..e3670607e 100644
--- a/crates/core/src/router/isotonic_estimator.rs
+++ b/crates/core/src/router/isotonic_estimator.rs
@@ -1,7 +1,7 @@
 use crate::ring::{Distance, Location, PeerKeyLocation};
 use pav_regression::pav::{IsotonicRegression, Point};
 use serde::Serialize;
-use std::collections::HashMap;
+use std::{collections::HashMap, fmt};
 
 const MIN_POINTS_FOR_REGRESSION: usize = 5;
 
@@ -160,6 +160,14 @@ pub(crate) enum EstimationError {
     InsufficientData, // Error indicating that there is not enough data for estimation
 }
 
+impl fmt::Display for EstimationError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            EstimationError::InsufficientData => write!(f, "Insufficient data for estimation"),
+        }
+    }
+}
+
 /// A routing event is a single request to a peer for a contract, and some value indicating
 /// the result of the request, such as the time it took to retrieve the contract.
 #[derive(Debug, Clone)]