Skip to content

Commit ea38b13

Browse files
apollo_integration_tests: fix non-interpolating .expect() messages and log typos (#14069)
The original motivating bug lives in apollo_integration_tests: // crates/apollo_integration_tests/src/integration_test_manager.rs .expect("Service {service:?} does not exist in the running executables map."); .expect("Node {node_idx} does not exist in idle or running nodes."); `.expect()` is an ordinary method on Option/Result that takes a `&str` and prints it verbatim on panic — it does not invoke `format_args!`. So named arguments inside the braces never get substituted: Before: thread '...' panicked at 'Service {service:?} does not exist in the running executables map.' After: thread '...' panicked at 'Service Batcher does not exist in the running executables map.' The panic still fires at the right moment for the right reason; only the diagnostic value is lost, which defeats the whole point of embedding the variable. The fix is to wrap the call as `.unwrap_or_else(|| panic!(...))` (or `|err| panic!("...: {err}")` on a Result), since `panic!` IS a format macro and the placeholders interpolate normally. A workspace grep revealed the same anti-pattern in nine more places, plus a small batch of unrelated typos in user-facing log/panic/assert strings. Bundling them all here because the common audit "are all log strings in the sequencer correct?" is what surfaced them. .expect() placeholder fixes (panic message would have been useless before): - apollo_storage/src/header.rs ({block_number}) - apollo_consensus_manager/src/consensus_manager.rs ({height}) - apollo_consensus/src/manager.rs (was {self.height} -- field access also invalid in fmt; corrected to local {height}) - apollo_storage/src/serialization/serializers_test.rs (7 sites; one referenced an out-of-scope {casm_file}, corrected to {bin_file_name}) - starknet_os/.../state_diff_encryption/utils.rs ({public_key}, {sn_public_key}) - apollo_integration_tests/src/integration_test_manager.rs ({service:?}, {node_idx}) - apollo_batcher/src/batcher.rs ({new_latest_height}) Plain typo fixes in log/panic/assert messages: - apollo_integration_tests/src/integration_test_manager.rs "succesfully" -> "successfully" (info!) - blockifier/src/execution/entry_point.rs "to to usize" -> "to usize" (log::warn!, duplicate word) - apollo_monitoring_endpoint/src/monitoring_endpoint.rs "MointoringEndpoint" -> "MonitoringEndpoint" (panic!) - starknet_patricia/.../filled_tree/tree.rs "a unmodified" -> "an unmodified" (panic!) - apollo_integration_tests/src/flow_test_setup.rs "a init message" -> "an init message" (panic!) - apollo_infra_utils/src/path_test.rs "an non-existent" -> "a non-existent" (assert!) No behavior change beyond what panic messages display; cargo check --tests passes on all touched crates. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0e3a471 commit ea38b13

12 files changed

Lines changed: 45 additions & 37 deletions

File tree

crates/apollo_batcher/src/batcher.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,9 @@ impl Batcher {
14281428
.storage_reader
14291429
.global_root(new_latest_height)
14301430
.expect("Failed to get global root from storage.")
1431-
.expect("Global root is not set for height {new_latest_height}.");
1431+
.unwrap_or_else(|| {
1432+
panic!("Global root is not set for height {new_latest_height}.")
1433+
});
14321434
assert_eq!(
14331435
global_root, stored_global_root,
14341436
"The given global root does not match the stored global root for height \

crates/apollo_consensus/src/manager.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1048,7 +1048,9 @@ impl<ContextT: ConsensusContext> MultiHeightManager<ContextT> {
10481048
.lock()
10491049
.await
10501050
.set_prev_voted_height(height)
1051-
.expect("Failed to write voted height {self.height} to storage");
1051+
.unwrap_or_else(|err| {
1052+
panic!("Failed to write voted height {height} to storage: {err:?}")
1053+
});
10521054
context.broadcast(vote.clone()).await?;
10531055
// Schedule a rebroadcast after the appropriate timeout.
10541056
let duration = match vote.vote_type {

crates/apollo_consensus_manager/src/consensus_manager.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,9 @@ impl ConsensusManager {
350350

351351
// This function will panic if the revert fails.
352352
let revert_blocks_fn = move |height| async move {
353-
self.batcher_client
354-
.revert_block(RevertBlockInput { height })
355-
.await
356-
.expect("Failed to revert block at height {height} in the batcher");
353+
self.batcher_client.revert_block(RevertBlockInput { height }).await.unwrap_or_else(
354+
|err| panic!("Failed to revert block at height {height} in the batcher: {err:?}"),
355+
);
357356
};
358357

359358
const BATCHER_REVERT_COMPONENT_DATA: RevertComponentData = RevertComponentData {

crates/apollo_infra_utils/src/path_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ fn resolve_project_relative_path_on_non_existent_path() {
77
assert!(!expected_path.exists());
88
let result = resolve_project_relative_path(relative_path);
99

10-
assert!(result.is_err(), "Expected an non-existent path error, got {result:?}");
10+
assert!(result.is_err(), "Expected a non-existent path error, got {result:?}");
1111
}
1212

1313
#[test]

crates/apollo_integration_tests/src/flow_test_setup.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ impl TxCollector {
482482
"Expected the first message in the stream to have id 0, got {incoming_message_id}"
483483
);
484484
let StreamMessageBody::Content(ProposalPart::Init(incoming_init)) = init_message else {
485-
panic!("Expected a init message. Got: {init_message:?}")
485+
panic!("Expected an init message. Got: {init_message:?}")
486486
};
487487

488488
self.accumulated_txs.lock().await.start_round(incoming_init.height, incoming_init.round);

crates/apollo_integration_tests/src/integration_test_manager.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,10 +363,9 @@ impl RunningNode {
363363
}
364364

365365
pub fn shutdown_service(&mut self, service: NodeService) {
366-
let handle = self
367-
.executable_handles
368-
.remove(&service)
369-
.expect("Service {service:?} does not exist in the running executables map.");
366+
let handle = self.executable_handles.remove(&service).unwrap_or_else(|| {
367+
panic!("Service {service:?} does not exist in the running executables map.")
368+
});
370369
assert!(!handle.is_finished(), "Handle should still be running.");
371370
handle.abort();
372371
}
@@ -448,7 +447,7 @@ impl IntegrationTestManager {
448447
.idle_nodes
449448
.get(&node_idx)
450449
.or_else(|| self.running_nodes.get(&node_idx).map(|node| &node.node_setup))
451-
.expect("Node {node_idx} does not exist in idle or running nodes.");
450+
.unwrap_or_else(|| panic!("Node {node_idx} does not exist in idle or running nodes."));
452451
node_setup.node_type
453452
}
454453

@@ -631,7 +630,7 @@ impl IntegrationTestManager {
631630
.expect("Running Nodes should be reverted.");
632631

633632
info!(
634-
"All running nodes have been reverted succesfully to block number \
633+
"All running nodes have been reverted successfully to block number \
635634
{expected_block_number}."
636635
);
637636
}

crates/apollo_monitoring_endpoint/src/monitoring_endpoint.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ pub fn create_monitoring_endpoint(
143143
impl ComponentStarter for MonitoringEndpoint {
144144
async fn start(&mut self) {
145145
info!("Starting component {}.", short_type_name::<Self>());
146-
self.run().await.unwrap_or_else(|e| panic!("Failed to start MointoringEndpoint: {e:?}"));
146+
self.run().await.unwrap_or_else(|e| panic!("Failed to start MonitoringEndpoint: {e:?}"));
147147
}
148148
}
149149

crates/apollo_storage/src/header.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ impl HeaderStorageWriter for StorageTxn<'_, RW> {
390390

391391
let reverted_header = headers_table
392392
.get(&self.txn, &block_number)?
393-
.expect("Missing header for block {block_number}.");
393+
.unwrap_or_else(|| panic!("Missing header for block {block_number}."));
394394
markers_table.upsert(&self.txn, &MarkerKind::Header, &block_number)?;
395395
headers_table.delete(&self.txn, &block_number)?;
396396
block_hash_to_number_table.delete(&self.txn, &reverted_header.block_hash)?;

crates/apollo_storage/src/serialization/serializers_test.rs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,14 +155,15 @@ fn casm_serialization_regression() {
155155
let mut serialized: Vec<u8> = Vec::new();
156156
json_casm
157157
.serialize_into(&mut serialized)
158-
.expect("Failed to serialize casm file: {json_file_name}");
158+
.unwrap_or_else(|_| panic!("Failed to serialize casm file: {json_file_name}"));
159159
let bin_path = path_in_resources(Path::new("casm").join(bin_file_name));
160-
let mut bin_file = File::open(bin_path)
161-
.expect("Failed to open bin file: {bin_file_name}\n{FIX_SUGGESTION}");
160+
let mut bin_file = File::open(bin_path).unwrap_or_else(|_| {
161+
panic!("Failed to open bin file: {bin_file_name}\n{FIX_SUGGESTION}")
162+
});
162163
let mut regression_casm_bytes = Vec::new();
163-
bin_file
164-
.read_to_end(&mut regression_casm_bytes)
165-
.expect("Failed to read bin file: {bin_file_name}\n{FIX_SUGGESTION}");
164+
bin_file.read_to_end(&mut regression_casm_bytes).unwrap_or_else(|_| {
165+
panic!("Failed to read bin file: {bin_file_name}\n{FIX_SUGGESTION}")
166+
});
166167
assert_eq!(
167168
regression_casm_bytes, serialized,
168169
"Serializing the casm gave a result different from the hardcoded \
@@ -179,16 +180,17 @@ fn casm_deserialization_regression() {
179180
}
180181

181182
for (json_file_name, bin_file_name) in CASM_SERIALIZATION_REGRESSION_FILES {
182-
let mut regression_casm_file =
183-
File::open(path_in_resources(Path::new("casm").join(bin_file_name)))
184-
.expect("Failed to open bin file: {bin_file_name}\n{FIX_SUGGESTION}");
183+
let mut regression_casm_file = File::open(path_in_resources(
184+
Path::new("casm").join(bin_file_name),
185+
))
186+
.unwrap_or_else(|_| panic!("Failed to open bin file: {bin_file_name}\n{FIX_SUGGESTION}"));
185187
let mut regression_casm_bytes = Vec::new();
186-
regression_casm_file
187-
.read_to_end(&mut regression_casm_bytes)
188-
.expect("Failed to read bin file: {bin_file_name}\n{FIX_SUGGESTION}");
188+
regression_casm_file.read_to_end(&mut regression_casm_bytes).unwrap_or_else(|_| {
189+
panic!("Failed to read bin file: {bin_file_name}\n{FIX_SUGGESTION}")
190+
});
189191
let regression_casm =
190192
CasmContractClass::deserialize_from(&mut regression_casm_bytes.as_slice())
191-
.expect("Failed to deserialize casm file: {casm_file}.");
193+
.unwrap_or_else(|| panic!("Failed to deserialize casm file: {bin_file_name}."));
192194
let json_path = format!("casm/{json_file_name}");
193195
let json_casm: CasmContractClass = read_json_file(&json_path);
194196
assert_eq!(
@@ -208,7 +210,7 @@ fn fix_casm_regression_files() {
208210
let casm_bytes = serialized.into_boxed_slice();
209211
let mut hardcoded_file =
210212
File::create(path_in_resources(Path::new("casm").join(bin_file_name)))
211-
.expect("Failed to create bin file {bin_file_name}\n");
213+
.unwrap_or_else(|_| panic!("Failed to create bin file {bin_file_name}\n"));
212214
hardcoded_file.write_all(&casm_bytes).unwrap();
213215
}
214216
}

crates/blockifier/src/execution/entry_point.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ impl EntryPointExecutionContext {
394394
}
395395
.try_into()
396396
.unwrap_or_else(|error| {
397-
log::warn!("Failed to convert global step limit to to usize: {error}.");
397+
log::warn!("Failed to convert global step limit to usize: {error}.");
398398
usize::MAX
399399
});
400400

0 commit comments

Comments
 (0)