Exit when worker detects expired client, remove event decoding trace (#…

…1023)

* Exit when worker detects expired client, remove event decoding trace

* Provide worker's name when worker aborts

* Exit client worker without error when client is expired or frozen

* Remove unused import

* Changelog

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Adi Seredinschi <adi@informal.systems>

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+Special thanks to Colin Axnér (@colin-axner) and Jongwhan Lee (@leejw51crypto)
+for raising multiple issues that helped us improve the reliability of Hermes.
+
 ### FEATURES
 
 - [ibc-relayer]
@@ -23,6 +26,7 @@
 
 - [ibc-relayer]
   - Fix for a client worker bug; Hermes `start` returns error if no chain is reachable ([#972])
+  - Client worker aborts gracefully if the client is expired or frozen ([#1022])
 
 - [gaia-manager]
   - Import hermes keys properly even if wallet HD derivation path is set ([#975])
@@ -42,6 +46,7 @@
 [#996]: https://github.com/informalsystems/ibc-rs/issues/996
 [#998]: https://github.com/informalsystems/ibc-rs/issues/998
 [#1003]: https://github.com/informalsystems/ibc-rs/issues/1003
+[#1022]: https://github.com/informalsystems/ibc-rs/issues/1022
 
 ## v0.3.2
 *May 21st, 2021*

relayer/src/event/rpc.rs

@@ -2,7 +2,6 @@ use std::{collections::HashMap, convert::TryFrom};
 
 use anomaly::BoxError;
 use tendermint_rpc::event::{Event as RpcEvent, EventData as RpcEventData};
-use tracing::trace;
 
 use ibc::ics02_client::events::NewBlock;
 use ibc::ics02_client::height::Height;
@@ -42,14 +41,13 @@ pub fn get_all_events(
 
             let actions_and_indices = extract_helper(&events)?;
             for action in actions_and_indices {
-                match build_event(RawObject::new(
+                if let Ok(event) = build_event(RawObject::new(
                     height,
                     action.0,
                     action.1 as usize,
                     events.clone(),
                 )) {
-                    Ok(event) => vals.push((height, event)),
-                    Err(e) => trace!("error while building event {}", e.to_string()),
+                    vals.push((height, event));
                 }
             }
         }

relayer/src/worker.rs

@@ -80,6 +80,7 @@ impl Worker {
     /// Run the worker event loop.
     fn run(self, msg_tx: Sender<WorkerMsg>) {
         let object = self.object();
+        let name = object.short_name();
 
         let result = match self {
             Self::Client(w) => w.run(),
@@ -88,14 +89,17 @@ impl Worker {
         };
 
         if let Err(e) = result {
-            error!("worker error: {}", e);
+            error!("[{}] worker aborted with error: {}", name, e);
         }
 
         if let Err(e) = msg_tx.send(WorkerMsg::Stopped(object)) {
-            error!("failed to notify supervisor that worker stopped: {}", e);
+            error!(
+                "[{}] failed to notify supervisor that worker stopped: {}",
+                name, e
+            );
         }
 
-        info!("worker stopped");
+        info!("[{}] worker stopped", name);
     }
 
     fn chains(&self) -> &ChainHandlePair {

relayer/src/worker/client.rs

@@ -2,7 +2,7 @@ use std::{thread, time::Duration};
 
 use anomaly::BoxError;
 use crossbeam_channel::Receiver;
-use tracing::{debug, error, info, trace};
+use tracing::{debug, info, trace, warn};
 
 use ibc::{events::IbcEvent, ics02_client::events::UpdateClient};
 
@@ -76,8 +76,11 @@ impl ClientWorker {
                     };
                 }
                 Err(e @ ForeignClientError::ExpiredOrFrozen(..)) => {
-                    error!("failed to refresh client '{}': {}", client, e);
-                    continue;
+                    warn!("failed to refresh client '{}': {}", client, e);
+
+                    // This worker has completed its job as the client cannot be refreshed any
+                    // further, and can therefore exit without an error.
+                    return Ok(());
                 }
                 _ => (),
             };