Skip to content

Commit 535016b

Browse files
committed
fix(consensus): exit non-zero on transient remote-signer startup failure
When the remote signer is unreachable at startup, the node's eager connect() fails fast (unlike the runtime signing path, which retries via sign_message_with_retry). run() caught this in its startup error arm and called wait_for_termination(), parking the process alive-but-idle until SIGTERM. A supervisor's restart policy never fires and a process-only liveness check doesn't notice, so the node stays parked indefinitely even after the signer recovers seconds later. Return an error for transient remote-signer failures (Transport, ServiceUnavailable, Timeout) so main exits non-zero and the supervisor restarts the node — the same behavior already used for the EL IPC watchdog. Permanent errors (configuration, authentication, malformed responses) still park for manual intervention. gRPC Status errors are intentionally excluded: the failure mode this guards against is the eager connect() at startup, which surfaces as Transport. Add unit tests covering the helper through the production eyre wrap_err chain.
1 parent a85368c commit 535016b

1 file changed

Lines changed: 79 additions & 0 deletions

File tree

‎crates/malachite-app/src/node.rs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ use arc_eth_engine::engine::Engine;
5959
use arc_eth_engine::json_structures::ExecutionBlock;
6060
use arc_node_consensus_cli::metrics;
6161
use arc_signer::local::{LocalSigningProvider, PrivateKey};
62+
use arc_signer::remote::RemoteSigningError;
6263
use arc_signer::ArcSigningProvider;
6364

6465
use crate::env_config::EnvConfig;
@@ -771,6 +772,24 @@ impl App {
771772
Err(e) => {
772773
let startup_error = e.wrap_err("Node failed to start");
773774
error!("{startup_error:?}");
775+
776+
// Transient remote-signer / network failures at startup should not park
777+
// the process. Parking (waiting for SIGTERM below) leaves the process alive
778+
// but idle, so a supervisor's restart policy never fires and liveness checks
779+
// that only test the process don't notice. Instead, return an error so the
780+
// caller (main) exits non-zero and the supervisor restarts us — the same
781+
// behavior used for the EL IPC watchdog below. The actual restart absorbs
782+
// the blip; the signer's runtime path already retries (sign_message_with_retry),
783+
// only the eager startup connect() fails fast.
784+
if is_retryable_startup_error(&startup_error) {
785+
error!(
786+
"Could not reach the remote signer at startup (connection failed, \
787+
unavailable, or timed out); exiting non-zero so the supervisor can \
788+
restart the node once the signer is reachable again..."
789+
);
790+
return Err(startup_error);
791+
}
792+
774793
error!("Manual intervention required! Waiting for termination signal (SIGTERM)...");
775794

776795
// Wait for SIGTERM to allow graceful shutdown
@@ -869,6 +888,29 @@ fn install_sigterm_handler(handle: &Handle) {
869888
#[cfg(not(unix))]
870889
fn install_sigterm_handler(_handle: &Handle) {}
871890

891+
/// Returns `true` if the startup error chain contains a *transient* remote-signer
892+
/// error — one that a restart is likely to recover from (the remote signer or the
893+
/// network to it being temporarily unavailable). Permanent errors (configuration,
894+
/// authentication, malformed responses) return `false` so they still park for
895+
/// manual intervention rather than restart-looping.
896+
///
897+
/// Note: gRPC `Status` errors (`RemoteSigningError::Status`) are intentionally out
898+
/// of scope here — the failure this guards against is the eager `connect()` at
899+
/// startup, which surfaces as `Transport`. Keeping the set narrow avoids
900+
/// restart-looping on errors whose transience is ambiguous.
901+
fn is_retryable_startup_error(error: &eyre::Report) -> bool {
902+
error.chain().any(|cause| {
903+
matches!(
904+
cause.downcast_ref::<RemoteSigningError>(),
905+
Some(
906+
RemoteSigningError::Transport(_)
907+
| RemoteSigningError::ServiceUnavailable(_)
908+
| RemoteSigningError::Timeout(_)
909+
)
910+
)
911+
})
912+
}
913+
872914
/// Wait for a termination signal (SIGTERM on Unix)
873915
async fn wait_for_termination() {
874916
#[cfg(unix)]
@@ -996,6 +1038,43 @@ mod tests {
9961038
assert_eq!(identity.consensus.address(), file_address);
9971039
}
9981040

1041+
// Mirror the production call site: RemoteSigningProvider::new returns a
1042+
// RemoteSigningError, which is wrapped twice with eyre context
1043+
// ("Failed to create remote signing provider" then "Node failed to start")
1044+
// before reaching is_retryable_startup_error. The test must exercise the
1045+
// chain-walk + downcast, not the bare enum.
1046+
fn wrapped(err: RemoteSigningError) -> eyre::Report {
1047+
eyre::Report::new(err)
1048+
.wrap_err("Failed to create remote signing provider")
1049+
.wrap_err("Node failed to start")
1050+
}
1051+
1052+
#[test]
1053+
fn transient_remote_signer_errors_are_retryable() {
1054+
assert!(is_retryable_startup_error(&wrapped(
1055+
RemoteSigningError::ServiceUnavailable("down".into())
1056+
)));
1057+
assert!(is_retryable_startup_error(&wrapped(
1058+
RemoteSigningError::Timeout("slow".into())
1059+
)));
1060+
}
1061+
1062+
#[test]
1063+
fn permanent_remote_signer_errors_are_not_retryable() {
1064+
assert!(!is_retryable_startup_error(&wrapped(
1065+
RemoteSigningError::Configuration("bad config".into())
1066+
)));
1067+
assert!(!is_retryable_startup_error(&wrapped(
1068+
RemoteSigningError::Authentication("denied".into())
1069+
)));
1070+
}
1071+
1072+
#[test]
1073+
fn non_signer_startup_errors_are_not_retryable() {
1074+
let report = eyre::eyre!("disk full").wrap_err("Node failed to start");
1075+
assert!(!is_retryable_startup_error(&report));
1076+
}
1077+
9991078
#[test]
10001079
fn ephemeral_consensus_identity_generates_valid_identity() {
10011080
let dir = tempdir().unwrap();

0 commit comments

Comments
 (0)