Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 75 additions & 2 deletions crates/core/src/node/op_state_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,27 @@ impl OpManager {
}
}

async fn notify_transaction_timeout(
event_loop_notifier: &EventLoopNotificationsSender,
tx: Transaction,
) -> bool {
match event_loop_notifier
.notifications_sender
.send(Either::Right(NodeEvent::TransactionTimedOut(tx)))
.await
{
Ok(()) => true,
Err(err) => {
tracing::warn!(
tx = %tx,
error = ?err,
"Failed to notify event loop about timed out transaction; receiver likely dropped"
);
false
}
}
}

async fn garbage_cleanup_task<ER: NetEventRegister>(
mut new_transactions: tokio::sync::mpsc::Receiver<Transaction>,
ops: Arc<Ops>,
Expand Down Expand Up @@ -376,7 +397,7 @@ async fn garbage_cleanup_task<ER: NetEventRegister>(
ops.under_progress.remove(&tx);
ops.completed.remove(&tx);
tracing::debug!("Transaction timed out: {tx}");
event_loop_notifier.notifications_sender.send(Either::Right(NodeEvent::TransactionTimedOut(tx))).await.unwrap();
notify_transaction_timeout(&event_loop_notifier, tx).await;
live_tx_tracker.remove_finished_transaction(tx);
}
}
Expand Down Expand Up @@ -405,11 +426,63 @@ async fn garbage_cleanup_task<ER: NetEventRegister>(
};
if removed {
tracing::debug!("Transaction timed out: {tx}");
event_loop_notifier.notifications_sender.send(Either::Right(NodeEvent::TransactionTimedOut(tx))).await.unwrap();
notify_transaction_timeout(&event_loop_notifier, tx).await;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude change this to warn indtead

Copy link

@claude claude bot Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude finished @iduartgomez's task —— View job


Change Complete

  • Read the file to identify line 429
  • Change log level to warn
  • Commit and push changes

Changed the log level from debug to warn in the notify_transaction_timeout function at line 345. This warning now properly alerts when the event loop receiver has been dropped during shutdown.

Changes:

  • crates/core/src/node/op_state_manager.rs:345 - Changed tracing::debug! to tracing::warn!

View commit |

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude check if is an error while sending and log a warn

Copy link

@claude claude bot Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude finished @iduartgomez's task —— View job


Analysis Complete

  • Read the current code to understand the send operation
  • Analyze error handling implementation

Current Implementation:

The notify_transaction_timeout function (crates/core/src/node/op_state_manager.rs:334-353) already implements error checking and warning logging:

match event_loop_notifier
    .notifications_sender
    .send(Either::Right(NodeEvent::TransactionTimedOut(tx)))
    .await
{
    Ok(()) => true,
    Err(err) => {
        tracing::warn!(
            tx = %tx,
            error = ?err,
            "Failed to notify event loop about timed out transaction; receiver likely dropped"
        );
        false
    }
}

The function returns a bool (true on success, false on error) and logs a warning when the send fails.

Call Sites:

The function is called at two locations (lines 400 and 429) without using the return value:

  • Line 400: notify_transaction_timeout(&event_loop_notifier, tx).await;
  • Line 429: notify_transaction_timeout(&event_loop_notifier, tx).await;

Question:

The error checking and warning are already implemented inside the function. Would you like me to:

  1. Keep it as-is (the function already logs warnings on send errors)
  2. Make the call sites explicitly check the return value and add additional logging
  3. Something else?

Please clarify what specific change you'd like made.

live_tx_tracker.remove_finished_transaction(tx);
}
}
}
}
}
}

#[cfg(test)]
mod tests {
use super::super::network_bridge::event_loop_notification_channel;
use super::*;
use crate::node::network_bridge::EventLoopNotificationsReceiver;
use either::Either;
use tokio::time::{timeout, Duration};

#[tokio::test]
async fn notify_timeout_succeeds_when_receiver_alive() {
let (receiver, notifier) = event_loop_notification_channel();
let EventLoopNotificationsReceiver {
mut notifications_receiver,
..
} = receiver;

let tx = Transaction::ttl_transaction();

let delivered = notify_transaction_timeout(&notifier, tx).await;
assert!(
delivered,
"notification should be delivered while receiver is alive"
);

let received = timeout(Duration::from_millis(100), notifications_receiver.recv())
.await
.expect("timed out waiting for notification")
.expect("notification channel closed");

match received {
Either::Right(NodeEvent::TransactionTimedOut(observed)) => {
assert_eq!(observed, tx, "unexpected transaction in notification");
}
other => panic!("unexpected notification: {other:?}"),
}
}

#[tokio::test]
async fn notify_timeout_handles_dropped_receiver() {
let (receiver, notifier) = event_loop_notification_channel();
drop(receiver);

let tx = Transaction::ttl_transaction();

let delivered = notify_transaction_timeout(&notifier, tx).await;
assert!(
!delivered,
"notification delivery should fail once receiver is dropped"
);
}
}
Loading