Merge pull request #50 from autonomys/better-error-handling

Add --slack=false and --alert-limit=N options, and integration tests in CI

Author: teor2345

.cargo/config.toml

@@ -66,4 +66,5 @@ rustflags = [
     "-Wclippy::manual_assert",
     "-Wclippy::needless_pass_by_value",
     "-Wclippy::precedence_bits",
+    "-Wclippy::let_underscore_untyped",
 ]

.github/workflows/rust.yml

@@ -91,7 +91,7 @@ jobs:
         ]
     runs-on: ${{ matrix.os }}
     # Don't use the full 6 hours if a test hangs
-    timeout-minutes: 120
+    timeout-minutes: 30
 
     steps:
       - name: Checkout
@@ -106,6 +106,51 @@ jobs:
         run: |
           cargo nextest run --locked --all-features
 
+  # Runs the chain-alerter process until it sends a startup alert, but with Slack disabled.
+  # This is useful for testing the chain-alerter process setup and main loop.
+  cargo-run:
+    name: cargo-run (${{ matrix.os }})
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [
+            "ubuntu-24.04",
+            "ubuntu-24.04-arm",
+            "windows-2025",
+            "windows-11-arm",
+            "macos-15",
+        ]
+    runs-on: ${{ matrix.os }}
+    # Unfortunately, this doesn't work at the step level on Windows.
+    defaults:
+      run:
+        shell: bash
+    # Don't use the full 6 hours if this integration test hangs
+    timeout-minutes: 30
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+
+      - name: Install ripgrep from crates.io
+        uses: baptiste0928/cargo-install@b687c656bda5733207e629b50a22bf68974a0305 # v3.3.2
+        with:
+          crate: ripgrep
+
+      # Randomly choose between the foundation and labs nodes, then run the integration test,
+      # checking for the startup alert, but printing all the logs to the console.
+      - name: cargo run --locked -- --slack=false --alert-limit=1
+        run: |
+          if [[ $(($RANDOM % 2)) == 0 ]]; then
+            echo "Using foundation node"
+            NODE_URL=wss://rpc.mainnet.subspace.foundation/ws
+          else
+            echo "Using labs node"
+            NODE_URL=wss://rpc-0.mainnet.autonomys.xyz/ws
+          fi
+          cargo run --locked -- --slack=false --alert-limit=1 --node-rpc-url="$NODE_URL" \
+            | rg --fixed-strings --passthru "**Launched and connected to the node**"
+
   # This job checks for incorrectly added dependencies, or dependencies that were made unused by code changes.
   cargo-unused-deps:
     name: cargo-unused-deps (ubuntu-24.04)
@@ -138,6 +183,7 @@ jobs:
       - cargo-clippy
       - cargo-build
       - cargo-test
+      - cargo-run
       - cargo-unused-deps
     steps:
       - name: Check job statuses
@@ -148,4 +194,5 @@ jobs:
           [[ "${{ needs.cargo-clippy.result }}" == "success" ]] || exit 1
           [[ "${{ needs.cargo-build.result }}" == "success" ]] || exit 1
           [[ "${{ needs.cargo-test.result }}" == "success" ]] || exit 1
+          [[ "${{ needs.cargo-run.result }}" == "success" ]] || exit 1
           [[ "${{ needs.cargo-unused-deps.result }}" == "success" ]] || exit 1

Cargo.lock

@@ -16,6 +16,7 @@ hex-literal = "0.4.1"
 humantime = "2.2.0"
 hyper-rustls = { version = "0.27.7", default-features = false }
 hyper-util = { version = "0.1.16", default-features = false }
+rand = "0.9.2"
 reqwest = { version = "0.12.22", default-features = false }
 rustls = { version = "0.23.31", default-features = false }
 scale-value = { version = "0.18.0", default-features = false }

Cargo.toml

@@ -119,8 +119,10 @@ Other alert kinds are not de-duplicated.
 4. Build and run
    - `cargo run -- --name "My Test Bot" --icon "warning" --node-rpc-url wss://rpc.mainnet.subspace.foundation/ws`
      - public node URLs are [listed in subspace.rs](https://github.com/autonomys/subspace-chain-alerts/blob/ac33ed7d200a1fdc3b92c1919f7b9cfacfba37c6/chain-alerter/src/subspace.rs#L43-L49)
+     - `--slack=false` will disable Slack message posting entirely, and just log alerts to the terminal.
+     - `--alert-limit=5` will exit after 5 alerts have been posted, including the startup alert.
    - On first observed block, you should see a Slack message in `#chain-alerts-test` summarizing connection and block info.
-   - All arguments are optional. The default node is localhost, and the default icon is the instance external IP address country flag.
+   - All arguments are optional. The default node is `localhost`, and the default icon is the instance external IP address country flag (looked up via an online GeoIP service, which can be wrong).
    - `RUST_LOG` can be used to filter logs, see:
      <https://docs.rs/tracing-subscriber/0.3.19/tracing_subscriber/filter/struct.EnvFilter.html#directives>
 

README.md

@@ -25,3 +25,4 @@ zeroize.workspace = true
 
 [dev-dependencies]
 hex-literal.workspace = true
+rand.workspace = true

chain-alerter/Cargo.toml

@@ -15,6 +15,7 @@ use chrono::Utc;
 use std::fmt::{self, Display};
 use std::time::Duration;
 use tokio::sync::{mpsc, watch};
+use tokio::task::JoinHandle;
 use tokio::time::sleep;
 use tracing::{trace, warn};
 
@@ -96,6 +97,16 @@ pub struct Alert {
     pub mode: BlockCheckMode,
 }
 
+impl Display for Alert {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "Mode: {:?}", self.mode)?;
+        write!(f, "\n{}", self.alert)?;
+        write!(f, "\nBlock: {}", self.block_info)?;
+
+        Ok(())
+    }
+}
+
 impl Alert {
     /// Create a new alert.
     pub fn new(alert: AlertKind, block_info: BlockInfo, mode: BlockCheckMode) -> Self {
@@ -710,7 +721,11 @@ pub async fn startup_alert(
     alert_tx: &mpsc::Sender<Alert>,
     block_info: &BlockInfo,
 ) -> anyhow::Result<()> {
-    assert!(mode.is_startup(), "should only be called at startup");
+    // We could check this is only called once, but that's a low priority.
+    assert!(
+        mode.is_current(),
+        "should only be called on the first current block",
+    );
 
     // TODO:
     // - always post this to the test channel, because it's not a real "alert"
@@ -770,27 +785,27 @@ pub async fn check_block(
 /// Spawn a task that waits for `MIN_BLOCK_GAP`, then alerts if there was no block received on
 /// `latest_block_rx` in that gap.
 ///
-/// Doesn't work on replayed blocks, because the chain has already resumed after a replayed block
-/// gap.
+/// Fatal errors will be returned from the spawned task's join handle.
 ///
-/// Fatal errors will panic in the spawned task.
+/// # Panics
+///
+/// On replayed blocks, because the chain has already resumed after a replayed block
+/// gap.
+#[must_use = "the spawned task must be joined"]
 pub async fn check_for_block_stall(
     mode: BlockCheckMode,
     alert_tx: mpsc::Sender<Alert>,
     block_info: BlockInfo,
     latest_block_rx: watch::Receiver<Option<BlockInfo>>,
-) {
+) -> JoinHandle<anyhow::Result<()>> {
     // It doesn't make sense to check the local clock for stalls on replayed blocks, because we
     // already know there's a new current block. It's expensive to spawn a task, so return
     // early.
-    if !mode.is_current() {
-        return;
-    }
+    assert!(mode.is_current(), "should only be called on current blocks");
 
     let old_block_info = block_info;
 
-    // There's no need to check the result of the spawned task, panics are impossible, and other
-    // errors are handled by replaying missed blocks on restart.
+    // We handle channel errors by restarting all tasks.
     tokio::spawn(async move {
         // Stall alerts without a resume are alarming, it looks like either the chain or alerter
         // has stopped. We've seen a spurious stall at exactly 60 seconds, but only once.
@@ -805,21 +820,22 @@ pub async fn check_for_block_stall(
             // There's a new block since we sent our block and spawned our task, so block
             // production hasn't stalled. But the latest block also spawned a task, so it
             // will alert if there is actually a stall.
-            return;
+            return Ok(());
         }
 
         let gap = gap_since_time(Utc::now(), old_block_info);
 
-        // If we have restarted, the new alerter will replay missed blocks, so we can ignore any
-        // send errors to dropped channels. This also avoids panics during process shutdown.
-        let _ = alert_tx
+        // If there is an error, we will restart, and the new alerter will replay missed blocks.
+        alert_tx
             .send(Alert::new(
                 AlertKind::BlockProductionStall { gap },
                 old_block_info,
                 mode,
             ))
-            .await;
-    });
+            .await?;
+
+        Ok(())
+    })
 }
 
 /// Check an extrinsic for alerts.

chain-alerter/src/alerts.rs

@@ -162,11 +162,11 @@ async fn test_startup_alert() -> anyhow::Result<()> {
 
     let (block_info, _, _) = fetch_block_info(&subspace_client, None, None).await?;
 
-    alerts::startup_alert(BlockCheckMode::Startup, &alert_tx, &block_info).await?;
+    alerts::startup_alert(BlockCheckMode::Current, &alert_tx, &block_info).await?;
     let alert = alert_rx.try_recv().expect("no alert received");
     assert_eq!(
         alert,
-        Alert::new(AlertKind::Startup, block_info, BlockCheckMode::Startup),
+        Alert::new(AlertKind::Startup, block_info, BlockCheckMode::Current),
     );
 
     // There should be no other alerts.
@@ -494,10 +494,10 @@ async fn no_expected_test_slot_time_alert() -> anyhow::Result<()> {
 
     naive_slot_time_monitor
         .process_block(BlockCheckMode::Replay, &first_block)
-        .await;
+        .await?;
     naive_slot_time_monitor
         .process_block(BlockCheckMode::Replay, &second_block)
-        .await;
+        .await?;
 
     alert_rx
         .try_recv()
@@ -525,10 +525,10 @@ async fn expected_test_slot_time_alert() -> anyhow::Result<()> {
 
     strict_slot_time_monitor
         .process_block(BlockCheckMode::Replay, &first_block)
-        .await;
+        .await?;
     strict_slot_time_monitor
         .process_block(BlockCheckMode::Replay, &second_block)
-        .await;
+        .await?;
 
     let alert = alert_rx
         .try_recv()
@@ -576,10 +576,10 @@ async fn expected_test_slot_time_alert_but_not_yet() -> anyhow::Result<()> {
 
     strict_slot_time_monitor
         .process_block(BlockCheckMode::Replay, &first_block)
-        .await;
+        .await?;
     strict_slot_time_monitor
         .process_block(BlockCheckMode::Replay, &second_block)
-        .await;
+        .await?;
 
     alert_rx
         .try_recv()

chain-alerter/src/alerts/tests.rs

@@ -721,12 +721,9 @@ pub async fn add_blocks_to_chain_fork_state(
                     // TODO: fix this at a lower level in the init, startup, or fork detection code
                     // instead
                     if event.needs_alert() && !mode.is_startup() {
-                        // If we have restarted, the new alerter will replay missed blocks, so we
-                        // can ignore any send errors to dropped channels.
-                        // This also avoids panics during process shutdown.
-                        let _ = alert_tx
+                        alert_tx
                             .send(Alert::from_chain_fork_event(event, block_info, mode))
-                            .await;
+                            .await?;
                     }
 
                     send_best_fork_block(