Skip to content

Commit 034f7dd

Browse files
fix(node-rewards-canister): Fix possible Timer Task Termination via Uncaught Panic (#8152)
Currently, the HourlySyncTask uses a recursive scheduling pattern: after executing, it reschedules itself with new_task.schedule_with_delay. However, if the task panics (e.g., due to .expect() or .unwrap() in downstream code), the panic rolls back the state and prevents the rescheduling line from executing. This results in the timer stopping until a canister upgrade occurs. **This PR fixes this bug by removing all possible panic occurrences**
1 parent 364ef1d commit 034f7dd

File tree

4 files changed

+21
-19
lines changed

4 files changed

+21
-19
lines changed

rs/node_rewards/canister/src/canister/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ impl NodeRewardsCanister {
121121
});
122122
let registry_querier = RegistryQuerier::new(registry_client.clone());
123123
let version = registry_client.get_latest_version();
124-
let subnets_list = registry_querier.subnets_list(version);
124+
let subnets_list = registry_querier.subnets_list(version)?;
125125
let last_day_synced: NaiveDate =
126126
metrics_manager.update_subnets_metrics(subnets_list).await?;
127127
canister.with_borrow(|canister| {

rs/node_rewards/canister/src/metrics.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,9 @@ where
109109
for (subnet_id, call_result) in subnets_metrics {
110110
match call_result {
111111
Ok(subnet_update) => {
112-
if subnet_update.is_empty() {
113-
ic_cdk::println!("No updates for subnet {}", subnet_id);
114-
} else {
115-
// Update the last timestamp for this subnet.
116-
let last_timestamp = subnet_update
117-
.last()
118-
.map(|metrics| metrics.timestamp_nanos)
119-
.expect("Not empty");
112+
if let Some(last_timestamp) =
113+
subnet_update.last().map(|metrics| metrics.timestamp_nanos)
114+
{
120115
self.last_timestamp_per_subnet
121116
.borrow_mut()
122117
.insert(subnet_id.into(), last_timestamp);
@@ -147,6 +142,8 @@ where
147142
subnet_id,
148143
date
149144
);
145+
} else {
146+
ic_cdk::println!("No updates for subnet {}", subnet_id);
150147
}
151148
}
152149
Err(e) => {

rs/node_rewards/canister/src/registry_querier.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,27 @@ impl RegistryQuerier {
4444
}
4545

4646
/// Returns a list of all subnets present in the registry at the specified version.
47-
pub fn subnets_list(&self, version: RegistryVersion) -> Vec<SubnetId> {
47+
pub fn subnets_list(&self, version: RegistryVersion) -> Result<Vec<SubnetId>, String> {
4848
let key = make_subnet_list_record_key();
49-
let record = self
49+
let record_bytes = self
5050
.registry_client
5151
.get_value(key.as_str(), version)
52-
.expect("Failed to get SubnetListRecord")
53-
.map(|v| {
54-
SubnetListRecord::decode(v.as_slice()).expect("Failed to decode SubnetListRecord")
55-
})
56-
.unwrap_or_default();
52+
.map_err(|e| format!("Failed to get SubnetListRecord: {:?}", e))?;
53+
54+
let record = if let Some(bytes) = record_bytes {
55+
SubnetListRecord::decode(bytes.as_slice())
56+
.map_err(|e| format!("Failed to decode SubnetListRecord: {:?}", e))?
57+
} else {
58+
SubnetListRecord::default()
59+
};
5760

5861
record
5962
.subnets
6063
.into_iter()
6164
.map(|s| {
62-
SubnetId::from(PrincipalId::try_from(s.as_slice()).expect("Invalid subnet ID"))
65+
let principal = PrincipalId::try_from(s.as_slice())
66+
.map_err(|e| format!("Invalid subnet ID: {:?}", e))?;
67+
Ok(SubnetId::from(principal))
6368
})
6469
.collect()
6570
}

rs/node_rewards/canister/src/registry_querier/tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,13 @@ fn test_subnets_list_returns_expected_subnets() {
173173
"2025-07-13",
174174
);
175175

176-
let got = client.subnets_list(version.into());
176+
let got = client.subnets_list(version.into()).unwrap();
177177

178178
let expected: Vec<SubnetId> = vec![subnet_1, subnet_2];
179179

180180
assert_eq!(got, expected);
181181

182-
let got = client.subnets_list(deleted_version.into());
182+
let got = client.subnets_list(deleted_version.into()).unwrap();
183183

184184
let expected: Vec<SubnetId> = vec![];
185185

0 commit comments

Comments
 (0)