From 99737c551a903bb9a41385bc62bed2c6c0088df0 Mon Sep 17 00:00:00 2001
From: Pawan Dhananjay <pawandhananjay@gmail.com>
Date: Mon, 30 Aug 2021 00:51:26 +0000
Subject: [PATCH] Improve eth1 fallback logging (#2490)

## Issue Addressed

Resolves #2487

## Proposed Changes

Logs a message once in every invocation of `Eth1Service::update` method if the primary endpoint is unavailable for some reason.

e.g.
```log
Aug 03 00:09:53.517 WARN Error connecting to eth1 node endpoint  action: trying fallbacks, endpoint: http://localhost:8545/, service: eth1_rpc
Aug 03 00:09:56.959 INFO Fetched data from fallback              fallback_number: 1, service: eth1_rpc
```

The main aim of this PR is to have an accompanying message to the "action: trying fallbacks" error message that is returned when checking the endpoint for liveness. This is mainly to indicate to the user that the fallback was live and reachable.

## Additional info
This PR is not meant to be a catch all for all cases where the primary endpoint failed. For instance, this won't log anything if the primary node was working fine during endpoint liveness checking and failed during deposit/block fetching. This is done intentionally to reduce number of logs while initial deposit/block sync and to avoid more complicated logic.
---
 beacon_node/eth1/src/service.rs | 38 ++++++++++++++++++++++-----------
 common/fallback/src/lib.rs      | 10 ++++++---
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/beacon_node/eth1/src/service.rs b/beacon_node/eth1/src/service.rs
index ab00ba0709..460f53e732 100644
--- a/beacon_node/eth1/src/service.rs
+++ b/beacon_node/eth1/src/service.rs
@@ -121,10 +121,12 @@ impl EndpointsCache {
         state
     }
 
+    /// Return the first successful result along with number of previous errors encountered
+    /// or all the errors encountered if every none of the fallback endpoints return required output.
     pub async fn first_success<'a, F, O, R>(
         &'a self,
         func: F,
-    ) -> Result<O, FallbackError<SingleEndpointError>>
+    ) -> Result<(O, usize), FallbackError<SingleEndpointError>>
     where
         F: Fn(&'a SensitiveUrl) -> R,
         R: Future<Output = Result<O, SingleEndpointError>>,
@@ -713,18 +715,24 @@ impl Service {
             e => format!("{:?}", e),
         };
 
-        let (remote_head_block, new_block_numbers_deposit, new_block_numbers_block_cache) =
-            endpoints
-                .first_success(|e| async move {
-                    get_remote_head_and_new_block_ranges(e, self, node_far_behind_seconds).await
-                })
-                .await
-                .map_err(|e| {
-                    format!(
-                        "Failed to update Eth1 service: {:?}",
-                        process_single_err(&e)
-                    )
-                })?;
+        let (
+            (remote_head_block, new_block_numbers_deposit, new_block_numbers_block_cache),
+            num_errors,
+        ) = endpoints
+            .first_success(|e| async move {
+                get_remote_head_and_new_block_ranges(e, self, node_far_behind_seconds).await
+            })
+            .await
+            .map_err(|e| {
+                format!(
+                    "Failed to update Eth1 service: {:?}",
+                    process_single_err(&e)
+                )
+            })?;
+
+        if num_errors > 0 {
+            info!(self.log, "Fetched data from fallback"; "fallback_number" => num_errors);
+        }
 
         *self.inner.remote_head_block.write() = Some(remote_head_block);
 
@@ -884,6 +892,7 @@ impl Service {
                         relevant_new_block_numbers_from_endpoint(e, self, HeadType::Deposit).await
                     })
                     .await
+                    .map(|(res, _)| res)
                     .map_err(Error::FallbackError)?,
             }
         };
@@ -930,6 +939,7 @@ impl Service {
                     .map_err(SingleEndpointError::GetDepositLogsFailed)
                 })
                 .await
+                .map(|(res, _)| res)
                 .map_err(Error::FallbackError)?;
 
             /*
@@ -1038,6 +1048,7 @@ impl Service {
                             .await
                     })
                     .await
+                    .map(|(res, _)| res)
                     .map_err(Error::FallbackError)?,
             }
         };
@@ -1103,6 +1114,7 @@ impl Service {
                     download_eth1_block(e, self.inner.clone(), Some(block_number)).await
                 })
                 .await
+                .map(|(res, _)| res)
                 .map_err(Error::FallbackError)?;
 
             self.inner
diff --git a/common/fallback/src/lib.rs b/common/fallback/src/lib.rs
index 94b177de35..d91de09be0 100644
--- a/common/fallback/src/lib.rs
+++ b/common/fallback/src/lib.rs
@@ -17,8 +17,12 @@ impl<T> Fallback<T> {
         Self { servers }
     }
 
-    /// Return the first successful result or all errors encountered.
-    pub async fn first_success<'a, F, O, E, R>(&'a self, func: F) -> Result<O, FallbackError<E>>
+    /// Return the first successful result along with number of previous errors encountered
+    /// or all the errors encountered if every server fails.
+    pub async fn first_success<'a, F, O, E, R>(
+        &'a self,
+        func: F,
+    ) -> Result<(O, usize), FallbackError<E>>
     where
         F: Fn(&'a T) -> R,
         R: Future<Output = Result<O, E>>,
@@ -26,7 +30,7 @@ impl<T> Fallback<T> {
         let mut errors = vec![];
         for server in &self.servers {
             match func(server).await {
-                Ok(val) => return Ok(val),
+                Ok(val) => return Ok((val, errors.len())),
                 Err(e) => errors.push(e),
             }
         }