refactor: return some after sync error as execution error

Signed-off-by: bsbds <69835502+bsbds@users.noreply.github.com>
xline-kv · Jul 26, 2023 · 5744b56 · 5744b56
1 parent 4dd9d2a
commit 5744b56
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 8 deletions.
diff --git a/curp/src/client.rs b/curp/src/client.rs
@@ -241,8 +241,16 @@ where
                 SyncResult::Error(SyncError::Timeout) => {
                     return Err(ProposeError::SyncedError("wait sync timeout".to_owned()));
                 }
-                SyncResult::Error(e) => {
-                    return Err(ProposeError::SyncedError(format!("{e:?}")));
+                SyncResult::Error(SyncError::NoSuchCmd(id)) => {
+                    return Err(ProposeError::SyncedError(format!(
+                        "No such cmd to be waited, propose id: {id}"
+                    )));
+                }
+                SyncResult::Error(SyncError::AfterSyncError(e)) => {
+                    return Err(ProposeError::SyncedError(e));
+                }
+                SyncResult::Error(SyncError::ExecuteError(e)) => {
+                    return Err(ProposeError::ExecutionError(e));
                 }
             }
         }

diff --git a/curp/src/rpc/mod.rs b/curp/src/rpc/mod.rs
@@ -176,23 +176,23 @@ impl WaitSyncedResponse {
                 unreachable!("should not call after sync if execution fails")
             }
             (None, None) => WaitSyncedResponse::new_error(&SyncError::<C>::AfterSyncError(
-                "can't get er result".to_owned().into(),
+                "can't get er result".to_owned(),
             )), // this is highly unlikely to happen,
             (Some(Err(_)), Some(_)) => {
                 unreachable!("should not call after_sync when exe failed")
             }
             (Some(Err(err)), None) => {
                 WaitSyncedResponse::new_error(&SyncError::<C>::ExecuteError(err))
             }
+            // Because after sync result actually wraps the execution error, we return it as `ExecuteError`.
+            // There's no need to return the execution result as the execution failed anyway.
             (Some(Ok(_er)), Some(Err(err))) => {
-                // FIXME: should er be returned?
-                WaitSyncedResponse::new_error(&SyncError::<C>::AfterSyncError(err))
+                WaitSyncedResponse::new_error(&SyncError::<C>::ExecuteError(err))
             }
             (Some(Ok(er)), Some(Ok(asr))) => WaitSyncedResponse::new_success::<C>(&asr, &er),
             (Some(Ok(_er)), None) => {
-                // FIXME: should er be returned?
                 WaitSyncedResponse::new_error(&SyncError::<C>::AfterSyncError(
-                    "can't get after sync result".to_owned().into(),
+                    "can't get after sync result".to_owned(),
                 )) // this is highly unlikely to happen,
             }
         }
@@ -233,7 +233,7 @@ pub(crate) enum SyncError<C: Command> {
     /// If the execution of the cmd went wrong
     ExecuteError(C::Error),
     /// If after sync of the cmd went wrong
-    AfterSyncError(C::Error),
+    AfterSyncError(String),
     /// If there is no such cmd to be waited
     NoSuchCmd(ProposeId),
     /// Wait timeout