From c47477dda9343c923ea2cd8fc15eab1b2a92bf7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= <drdr.xp@gmail.com>
Date: Tue, 9 Apr 2024 12:04:36 +0800
Subject: [PATCH] Refactor: Remove inappropraite error log on quit

`RaftCore` should not output ERROR level log on a normal quit.
Reduce ERROR level to INFO.

- Fix: #1096
---
 openraft/src/core/raft_core.rs  | 30 ++++++++++++++++++++----------
 openraft/src/raft/core_state.rs |  5 +++--
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/openraft/src/core/raft_core.rs b/openraft/src/core/raft_core.rs
index 599e7529d..ceaf12ce8 100644
--- a/openraft/src/core/raft_core.rs
+++ b/openraft/src/core/raft_core.rs
@@ -47,6 +47,7 @@ use crate::entry::RaftEntry;
 use crate::error::ClientWriteError;
 use crate::error::Fatal;
 use crate::error::ForwardToLeader;
+use crate::error::Infallible;
 use crate::error::InitializeError;
 use crate::error::QuorumNotEnough;
 use crate::error::RPCError;
@@ -220,34 +221,41 @@ where
     pub(crate) async fn main(
         mut self,
         rx_shutdown: <C::AsyncRuntime as AsyncRuntime>::OneshotReceiver<()>,
-    ) -> Result<(), Fatal<C::NodeId>> {
+    ) -> Result<Infallible, Fatal<C::NodeId>> {
         let span = tracing::span!(parent: &self.span, Level::DEBUG, "main");
         let res = self.do_main(rx_shutdown).instrument(span).await;
 
         // Flush buffered metrics
         self.report_metrics(None);
 
-        tracing::info!("update the metrics for shutdown");
+        // Safe unwrap: res is Result<Infallible, _>
+        let err = res.unwrap_err();
+        match err {
+            Fatal::Stopped => { /* Normal quit */ }
+            _ => {
+                tracing::error!(error = display(&err), "quit RaftCore::main on error");
+            }
+        }
+
+        tracing::debug!("update the metrics for shutdown");
         {
             let mut curr = self.tx_metrics.borrow().clone();
             curr.state = ServerState::Shutdown;
-
-            if let Err(err) = &res {
-                tracing::error!(?err, "quit RaftCore::main on error");
-                curr.running_state = Err(err.clone());
-            }
+            curr.running_state = Err(err.clone());
 
             let _ = self.tx_metrics.send(curr);
         }
 
-        res
+        tracing::info!("RaftCore shutdown complete");
+
+        Err(err)
     }
 
     #[tracing::instrument(level="trace", skip_all, fields(id=display(self.id), cluster=%self.config.cluster_name))]
     async fn do_main(
         &mut self,
         rx_shutdown: <C::AsyncRuntime as AsyncRuntime>::OneshotReceiver<()>,
-    ) -> Result<(), Fatal<C::NodeId>> {
+    ) -> Result<Infallible, Fatal<C::NodeId>> {
         tracing::debug!("raft node is initializing");
 
         self.engine.startup();
@@ -900,11 +908,13 @@ where
     }
 
     /// Run an event handling loop
+    ///
+    /// It always returns a [`Fatal`] error upon returning.
     #[tracing::instrument(level="debug", skip_all, fields(id=display(self.id)))]
     async fn runtime_loop(
         &mut self,
         mut rx_shutdown: <C::AsyncRuntime as AsyncRuntime>::OneshotReceiver<()>,
-    ) -> Result<(), Fatal<C::NodeId>> {
+    ) -> Result<Infallible, Fatal<C::NodeId>> {
         // Ratio control the ratio of number of RaftMsg to process to number of Notify to process.
         let mut balancer = Balancer::new(10_000);
 
diff --git a/openraft/src/raft/core_state.rs b/openraft/src/raft/core_state.rs
index c924ca310..b944beaf8 100644
--- a/openraft/src/raft/core_state.rs
+++ b/openraft/src/raft/core_state.rs
@@ -1,4 +1,5 @@
 use crate::error::Fatal;
+use crate::error::Infallible;
 use crate::AsyncRuntime;
 use crate::NodeId;
 
@@ -9,8 +10,8 @@ where
     A: AsyncRuntime,
 {
     /// The RaftCore task is still running.
-    Running(A::JoinHandle<Result<(), Fatal<NID>>>),
+    Running(A::JoinHandle<Result<Infallible, Fatal<NID>>>),
 
     /// The RaftCore task has finished. The return value of the task is stored.
-    Done(Result<(), Fatal<NID>>),
+    Done(Result<Infallible, Fatal<NID>>),
 }