From cb39cb00a1e239affc6a1fb4075d8e414ac453a9 Mon Sep 17 00:00:00 2001
From: haixuanTao <tao.xavier@outlook.com>
Date: Mon, 24 Apr 2023 14:04:42 +0800
Subject: [PATCH] Improve error message for logs

---
 Cargo.lock                      | 25 +++++++++++++++++--------
 binaries/cli/src/logs.rs        | 23 +++++++++++++----------
 binaries/coordinator/src/lib.rs | 11 ++++++-----
 3 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index dbe82c3e..fa128169 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1770,7 +1770,7 @@ checksum = "8a3de6e8d11b22ff9edc6d916f890800597d60f8b2da1caf2955c274638d6412"
 dependencies = [
  "cfg-if",
  "libc",
- "redox_syscall",
+ "redox_syscall 0.2.12",
  "windows-sys 0.45.0",
 ]
 
@@ -3303,7 +3303,7 @@ checksum = "28141e0cc4143da2443301914478dc976a61ffdb3f043058310c70df2fed8954"
 dependencies = [
  "cfg-if",
  "libc",
- "redox_syscall",
+ "redox_syscall 0.2.12",
  "smallvec",
  "windows-sys 0.32.0",
 ]
@@ -3896,6 +3896,15 @@ dependencies = [
  "bitflags 1.3.2",
 ]
 
+[[package]]
+name = "redox_syscall"
+version = "0.3.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "567664f262709473930a4bf9e51bf2ebf3348f2e748ccc50dea20646858f8f29"
+dependencies = [
+ "bitflags 1.3.2",
+]
+
 [[package]]
 name = "redox_users"
 version = "0.4.2"
@@ -3903,7 +3912,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7776223e2696f1aa4c6b0170e83212f47296a00424305117d013dfe86fb0fe55"
 dependencies = [
  "getrandom",
- "redox_syscall",
+ "redox_syscall 0.2.12",
  "thiserror",
 ]
 
@@ -4597,15 +4606,15 @@ checksum = "c02424087780c9b71cc96799eaeddff35af2bc513278cda5c99fc1f5d026d3c1"
 
 [[package]]
 name = "tempfile"
-version = "3.4.0"
+version = "3.5.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "af18f7ae1acd354b992402e9ec5864359d693cd8a79dcbef59f76891701c1e95"
+checksum = "b9fbec84f381d5795b08656e4912bec604d162bff9291d6189a78f4c8ab87998"
 dependencies = [
  "cfg-if",
  "fastrand",
- "redox_syscall",
- "rustix 0.36.9",
- "windows-sys 0.42.0",
+ "redox_syscall 0.3.5",
+ "rustix 0.37.3",
+ "windows-sys 0.45.0",
 ]
 
 [[package]]
diff --git a/binaries/cli/src/logs.rs b/binaries/cli/src/logs.rs
index f96d06a2..45a990c2 100644
--- a/binaries/cli/src/logs.rs
+++ b/binaries/cli/src/logs.rs
@@ -10,17 +10,20 @@ pub fn logs(uuid: Option<Uuid>, name: Option<String>, node: String) -> Result<()
     let connection = control_connection(&mut control_session)?;
     let logs = {
         let reply_raw = connection
-            .request(&serde_json::to_vec(&ControlRequest::Logs {
-                uuid,
-                name,
-                node: node.clone(),
-            })?)
-            .wrap_err("failed to send DaemonConnected message")?;
+            .request(
+                &serde_json::to_vec(&ControlRequest::Logs {
+                    uuid,
+                    name,
+                    node: node.clone(),
+                })
+                .wrap_err("")?,
+            )
+            .wrap_err("failed to send Logs request message")?;
 
         let reply = serde_json::from_slice(&reply_raw).wrap_err("failed to parse reply")?;
         match reply {
             ControlRequestReply::Logs { logs } => logs,
-            other => bail!("unexpected reply to daemon connection check: {other:?}"),
+            other => bail!("unexpected reply to daemon logs: {other:?}"),
         }
     };
 
@@ -28,12 +31,12 @@ pub fn logs(uuid: Option<Uuid>, name: Option<String>, node: String) -> Result<()
         .header(true)
         .grid(true)
         .line_numbers(true)
-        .paging_mode(bat::PagingMode::Always)
+        .paging_mode(bat::PagingMode::QuitIfOneScreen)
         .inputs(vec![Input::from_bytes(&logs)
-            .name("Logs") // TODO: Make a better name
+            .name("Logs")
             .title(format!("Logs from {node}.").as_str())])
         .print()
-        .unwrap();
+        .wrap_err("Something went wrong with viewing log file")?;
 
     Ok(())
 }
diff --git a/binaries/coordinator/src/lib.rs b/binaries/coordinator/src/lib.rs
index 654c3f6b..fd2b45f9 100644
--- a/binaries/coordinator/src/lib.rs
+++ b/binaries/coordinator/src/lib.rs
@@ -621,29 +621,30 @@ async fn retrieve_logs(
     };
     let message = serde_json::to_vec(&DaemonCoordinatorEvent::Logs {
         dataflow_id,
-        node_id,
+        node_id: node_id.clone(),
     })?;
     let mut reply_logs = Vec::new();
+
     for machine_id in &dataflow.machines {
         let daemon_connection = daemon_connections
             .get_mut(machine_id)
             .wrap_err("no daemon connection")?; // TODO: take from dataflow spec
         tcp_send(daemon_connection, &message)
             .await
-            .wrap_err("failed to send reload message to daemon")?;
+            .wrap_err("failed to send logs message to daemon")?;
 
         // wait for reply
         let reply_raw = tcp_receive(daemon_connection)
             .await
-            .wrap_err("failed to receive reload reply from daemon")?;
+            .wrap_err("failed to retrieve logs reply from daemon")?;
         match serde_json::from_slice(&reply_raw)
-            .wrap_err("failed to deserialize reload reply from daemon")?
+            .wrap_err("failed to deserialize logs reply from daemon")?
         {
             DaemonCoordinatorReply::Logs { logs } => reply_logs = logs,
             other => bail!("unexpected reply after sending reload: {other:?}"),
         }
     }
-    tracing::info!("successfully reloaded dataflow `{dataflow_id}`");
+    tracing::info!("successfully retrieved logs for `{dataflow_id}/{node_id}`");
 
     Ok(reply_logs)
 }