From dcb062e95e8ce677ee398e8b17b5f371c661bd64 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 ea026e45f..ca6e9b616 100644
--- a/openraft/src/core/raft_core.rs
+++ b/openraft/src/core/raft_core.rs
@@ -48,6 +48,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;
@@ -216,31 +217,38 @@ where
     SM: RaftStateMachine<C>,
 {
     /// The main loop of the Raft protocol.
-    pub(crate) async fn main(mut self, rx_shutdown: OneshotReceiverOf<C, ()>) -> Result<(), Fatal<C>> {
+    pub(crate) async fn main(mut self, rx_shutdown: OneshotReceiverOf<C, ()>) -> Result<Infallible, Fatal<C>> {
         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: OneshotReceiverOf<C, ()>) -> Result<(), Fatal<C>> {
+    async fn do_main(&mut self, rx_shutdown: OneshotReceiverOf<C, ()>) -> Result<Infallible, Fatal<C>> {
         tracing::debug!("raft node is initializing");
 
         self.engine.startup();
@@ -890,8 +898,10 @@ 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: OneshotReceiverOf<C, ()>) -> Result<(), Fatal<C>> {
+    async fn runtime_loop(&mut self, mut rx_shutdown: OneshotReceiverOf<C, ()>) -> Result<Infallible, Fatal<C>> {
         // 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 b586e6e65..acefa3c42 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::type_config::alias::JoinHandleOf;
 use crate::RaftTypeConfig;
 
@@ -7,8 +8,8 @@ pub(in crate::raft) enum CoreState<C>
 where C: RaftTypeConfig
 {
     /// The RaftCore task is still running.
-    Running(JoinHandleOf<C, Result<(), Fatal<C>>>),
+    Running(JoinHandleOf<C, Result<Infallible, Fatal<C>>>),
 
     /// The RaftCore task has finished. The return value of the task is stored.
-    Done(Result<(), Fatal<C>>),
+    Done(Result<Infallible, Fatal<C>>),
 }