From c1f1e1f0268703982180fa164a73b4327d2b5ec9 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Thu, 23 Oct 2025 18:03:37 +0200 Subject: [PATCH 01/10] feat: Collect and return all bulk dataset processing errors in Vec --- pre-compute/src/compute/app_runner.rs | 32 +- pre-compute/src/compute/errors.rs | 4 +- pre-compute/src/compute/pre_compute_app.rs | 66 ++- pre-compute/src/compute/pre_compute_args.rs | 472 ++++++++++++++++---- 4 files changed, 454 insertions(+), 120 deletions(-) diff --git a/pre-compute/src/compute/app_runner.rs b/pre-compute/src/compute/app_runner.rs index 093a47b..186d1d6 100644 --- a/pre-compute/src/compute/app_runner.rs +++ b/pre-compute/src/compute/app_runner.rs @@ -42,27 +42,25 @@ pub fn start_with_app( pre_compute_app: &mut A, chain_task_id: &str, ) -> ExitMode { - let exit_cause = match pre_compute_app.run() { + let exit_causes = match pre_compute_app.run() { Ok(_) => { info!("TEE pre-compute completed"); return ExitMode::Success; } - Err(exit_cause) => { - error!("TEE pre-compute failed with known exit cause [{exit_cause:?}]"); - exit_cause + Err(exit_causes) => { + error!("TEE pre-compute failed with known exit cause [{exit_causes:?}]"); + exit_causes } }; let authorization = match get_challenge(chain_task_id) { Ok(auth) => auth, Err(_) => { - error!("Failed to sign exitCause message [{exit_cause:?}]"); + error!("Failed to sign exitCause message [{exit_causes:?}]"); return ExitMode::UnreportedFailure; } }; - let exit_causes = vec![exit_cause.clone()]; - match WorkerApiClient::from_env().send_exit_causes_for_pre_compute_stage( &authorization, chain_task_id, @@ -70,7 +68,7 @@ pub fn start_with_app( ) { Ok(_) => ExitMode::ReportedFailure, Err(_) => { - error!("Failed to report exitCause [{exit_cause:?}]"); + error!("Failed to report exitCause [{exit_causes:?}]"); ExitMode::UnreportedFailure } } @@ -150,7 +148,7 @@ mod pre_compute_start_with_app_tests { let mut mock = MockPreComputeAppTrait::new(); mock.expect_run() - .returning(|| Err(ReplicateStatusCause::PreComputeWorkerAddressMissing)); + .returning(|| Err(vec![ReplicateStatusCause::PreComputeWorkerAddressMissing])); temp_env::with_vars(env_vars_to_set, || { temp_env::with_vars_unset(env_vars_to_unset, || { @@ -172,8 +170,11 @@ mod pre_compute_start_with_app_tests { let env_vars_to_unset = vec![ENV_SIGN_TEE_CHALLENGE_PRIVATE_KEY]; let mut mock = MockPreComputeAppTrait::new(); - mock.expect_run() - .returning(|| Err(ReplicateStatusCause::PreComputeTeeChallengePrivateKeyMissing)); + mock.expect_run().returning(|| { + Err(vec![ + ReplicateStatusCause::PreComputeTeeChallengePrivateKeyMissing, + ]) + }); temp_env::with_vars(env_vars_to_set, || { temp_env::with_vars_unset(env_vars_to_unset, || { @@ -199,8 +200,11 @@ mod pre_compute_start_with_app_tests { let mock_server_addr_string = mock_server.address().to_string(); let mut mock = MockPreComputeAppTrait::new(); - mock.expect_run() - .returning(|| Err(ReplicateStatusCause::PreComputeTeeChallengePrivateKeyMissing)); + mock.expect_run().returning(|| { + Err(vec![ + ReplicateStatusCause::PreComputeTeeChallengePrivateKeyMissing, + ]) + }); let result_code = tokio::task::spawn_blocking(move || { let env_vars = vec![ @@ -248,7 +252,7 @@ mod pre_compute_start_with_app_tests { let mut mock = MockPreComputeAppTrait::new(); mock.expect_run() .times(1) - .returning(|| Err(ReplicateStatusCause::PreComputeOutputFolderNotFound)); + .returning(|| Err(vec![ReplicateStatusCause::PreComputeOutputFolderNotFound])); // Move the blocking operations into spawn_blocking let result_code = tokio::task::spawn_blocking(move || { diff --git a/pre-compute/src/compute/errors.rs b/pre-compute/src/compute/errors.rs index f9981de..751f0eb 100644 --- a/pre-compute/src/compute/errors.rs +++ b/pre-compute/src/compute/errors.rs @@ -27,8 +27,8 @@ pub enum ReplicateStatusCause { PreComputeInvalidTeeSignature, #[error("IS_DATASET_REQUIRED environment variable is missing")] PreComputeIsDatasetRequiredMissing, - #[error("Input files download failed")] - PreComputeInputFileDownloadFailed, + #[error("Input file download failed for input {0}")] + PreComputeInputFileDownloadFailed(usize), #[error("Input files number related environment variable is missing")] PreComputeInputFilesNumberMissing, #[error("Invalid dataset checksum for dataset {0}")] diff --git a/pre-compute/src/compute/pre_compute_app.rs b/pre-compute/src/compute/pre_compute_app.rs index 825f3ef..adb679f 100644 --- a/pre-compute/src/compute/pre_compute_app.rs +++ b/pre-compute/src/compute/pre_compute_app.rs @@ -9,9 +9,9 @@ use std::path::{Path, PathBuf}; #[cfg_attr(test, automock)] pub trait PreComputeAppTrait { - fn run(&mut self) -> Result<(), ReplicateStatusCause>; + fn run(&mut self) -> Result<(), Vec>; fn check_output_folder(&self) -> Result<(), ReplicateStatusCause>; - fn download_input_files(&self) -> Result<(), ReplicateStatusCause>; + fn download_input_files(&self) -> Result<(), Vec>; fn save_plain_dataset_file( &self, plain_content: &[u8], @@ -55,17 +55,33 @@ impl PreComputeAppTrait for PreComputeApp { /// let mut app = PreComputeApp::new("task_id".to_string()); /// app.run(); /// ``` - fn run(&mut self) -> Result<(), ReplicateStatusCause> { - // TODO: Collect all errors instead of propagating immediately, and return the list of errors - self.pre_compute_args = PreComputeArgs::read_args()?; - self.check_output_folder()?; + fn run(&mut self) -> Result<(), Vec> { + let (args, mut exit_causes) = PreComputeArgs::read_args(); + self.pre_compute_args = args; + + if let Err(exit_cause) = self.check_output_folder() { + return Err(vec![exit_cause]); + } + for dataset in self.pre_compute_args.datasets.iter() { - let encrypted_content = dataset.download_encrypted_dataset(&self.chain_task_id)?; - let plain_content = dataset.decrypt_dataset(&encrypted_content)?; - self.save_plain_dataset_file(&plain_content, &dataset.filename)?; + if let Err(exit_cause) = dataset + .download_encrypted_dataset(&self.chain_task_id) + .and_then(|encrypted_content| dataset.decrypt_dataset(&encrypted_content)) + .and_then(|plain_content| { + self.save_plain_dataset_file(&plain_content, &dataset.filename) + }) + { + exit_causes.push(exit_cause); + }; + } + if let Err(exit_cause) = self.download_input_files() { + exit_causes.extend(exit_cause); + }; + if !exit_causes.is_empty() { + Err(exit_causes) + } else { + Ok(()) } - self.download_input_files()?; - Ok(()) } /// Checks whether the output folder specified in `pre_compute_args` exists. @@ -105,19 +121,27 @@ impl PreComputeAppTrait for PreComputeApp { /// This function panics if: /// - `pre_compute_args` is `None`. /// - `chain_task_id` is `None`. - fn download_input_files(&self) -> Result<(), ReplicateStatusCause> { + fn download_input_files(&self) -> Result<(), Vec> { + let mut exit_causes: Vec = vec![]; let args = &self.pre_compute_args; let chain_task_id: &str = &self.chain_task_id; - for url in &args.input_files { + for (index, url) in args.input_files.iter().enumerate() { info!("Downloading input file [chainTaskId:{chain_task_id}, url:{url}]"); let filename = sha256(url.to_string()); if download_file(url, &args.output_dir, &filename).is_none() { - return Err(ReplicateStatusCause::PreComputeInputFileDownloadFailed); + exit_causes.push(ReplicateStatusCause::PreComputeInputFileDownloadFailed( + index, + )); } } - Ok(()) + + if !exit_causes.is_empty() { + Err(exit_causes) + } else { + Ok(()) + } } /// Saves the decrypted (plain) dataset to disk in the configured output directory. @@ -293,12 +317,12 @@ mod tests { let result = app.download_input_files(); assert_eq!( result.unwrap_err(), - ReplicateStatusCause::PreComputeInputFileDownloadFailed + vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed(0)] ); } #[test] - fn test_partial_failure_stops_on_first_error() { + fn test_partial_failure_dont_stops_on_first_error() { let (_container, json_url, xml_url) = start_container(); let temp_dir = TempDir::new().unwrap(); @@ -307,7 +331,7 @@ mod tests { vec![ &json_url, // This should succeed "https://invalid-url-that-should-fail.com/file.txt", // This should fail - &xml_url, // This shouldn't be reached + &xml_url, // This should succeed ], temp_dir.path().to_str().unwrap(), ); @@ -315,16 +339,16 @@ mod tests { let result = app.download_input_files(); assert_eq!( result.unwrap_err(), - ReplicateStatusCause::PreComputeInputFileDownloadFailed + vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed(1)] ); // First file should be downloaded with SHA256 filename let json_hash = sha256(json_url); assert!(temp_dir.path().join(json_hash).exists()); - // Third file should NOT be downloaded (stopped on second failure) + // Third file should be downloaded (not stopped on second failure) let xml_hash = sha256(xml_url); - assert!(!temp_dir.path().join(xml_hash).exists()); + assert!(temp_dir.path().join(xml_hash).exists()); } // endregion diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index e230afb..c008002 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -58,78 +58,125 @@ impl PreComputeArgs { /// // Typically called with task ID from execution context /// let args = PreComputeArgs::read_args(); /// ``` - pub fn read_args() -> Result { - let output_dir = get_env_var_or_error( + pub fn read_args() -> (PreComputeArgs, Vec) { + let mut exit_causes: Vec = vec![]; + + let output_dir = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecPreComputeOut, ReplicateStatusCause::PreComputeOutputPathMissing, - )?; + ) { + Ok(output_dir) => output_dir, + Err(exit_cause) => return (PreComputeArgs::default(), vec![exit_cause]), + }; - let is_dataset_required_str = get_env_var_or_error( + let is_dataset_required = get_env_var_or_error( TeeSessionEnvironmentVariable::IsDatasetRequired, ReplicateStatusCause::PreComputeIsDatasetRequiredMissing, - )?; - let is_dataset_required = is_dataset_required_str - .to_lowercase() - .parse::() - .map_err(|_| ReplicateStatusCause::PreComputeIsDatasetRequiredMissing)?; - - // Read iexec bulk slice size (defaults to 0 if not present for backward compatibility) - let iexec_bulk_slice_size_str = + ) + .and_then(|s| { + s.to_lowercase() + .parse::() + .map_err(|_| ReplicateStatusCause::PreComputeIsDatasetRequiredMissing) + }) + .unwrap_or_else(|e| { + exit_causes.push(e); + false + }); + + let iexec_bulk_slice_size = std::env::var(TeeSessionEnvironmentVariable::IexecBulkSliceSize.name()) - .unwrap_or("0".to_string()); - let iexec_bulk_slice_size = iexec_bulk_slice_size_str - .parse::() - .map_err(|_| ReplicateStatusCause::PreComputeFailedUnknownIssue)?; // TODO: replace with a more specific error + .ok() + .and_then(|s| s.parse::().ok()) + .unwrap_or_else(|| { + exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); + 0 + }); // TODO: replace with a more specific error let mut datasets = Vec::with_capacity(iexec_bulk_slice_size + 1); // Read datasets let start_index = if is_dataset_required { 0 } else { 1 }; for i in start_index..=iexec_bulk_slice_size { - let filename = get_env_var_or_error( + let filename = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetFilename(i), ReplicateStatusCause::PreComputeDatasetFilenameMissing(format!("dataset_{i}")), - )?; - let url = get_env_var_or_error( + ) { + Ok(filename) => filename, + Err(exit_cause) => { + exit_causes.push(exit_cause); + continue; + } + }; + + let url = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetUrl(i), ReplicateStatusCause::PreComputeDatasetUrlMissing(filename.clone()), - )?; - let checksum = get_env_var_or_error( + ) { + Ok(url) => url, + Err(exit_cause) => { + exit_causes.push(exit_cause); + continue; + } + }; + + let checksum = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetChecksum(i), ReplicateStatusCause::PreComputeDatasetChecksumMissing(filename.clone()), - )?; - let key = get_env_var_or_error( + ) { + Ok(checksum) => checksum, + Err(exit_cause) => { + exit_causes.push(exit_cause); + continue; + } + }; + + let key = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetKey(i), ReplicateStatusCause::PreComputeDatasetKeyMissing(filename.clone()), - )?; + ) { + Ok(key) => key, + Err(exit_cause) => { + exit_causes.push(exit_cause); + continue; + } + }; datasets.push(Dataset::new(url, checksum, filename, key)); } - let input_files_nb_str = get_env_var_or_error( + let input_files_nb = get_env_var_or_error( TeeSessionEnvironmentVariable::IexecInputFilesNumber, ReplicateStatusCause::PreComputeInputFilesNumberMissing, - )?; - let input_files_nb = input_files_nb_str - .parse::() - .map_err(|_| ReplicateStatusCause::PreComputeInputFilesNumberMissing)?; - - let mut input_files = Vec::with_capacity(input_files_nb); - for i in 1..=input_files_nb { - let url = get_env_var_or_error( - TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), - ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), - )?; - input_files.push(url); - } - - Ok(PreComputeArgs { - output_dir, - is_dataset_required, - input_files, - iexec_bulk_slice_size, - datasets, + ) + .and_then(|s| { + s.parse::() + .map_err(|_| ReplicateStatusCause::PreComputeInputFilesNumberMissing) }) + .unwrap_or_else(|e| { + exit_causes.push(e); + 0 + }); + + let input_files: Vec = (1..=input_files_nb) + .filter_map(|i| { + get_env_var_or_error( + TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), + ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), + ) + .map_err(|e| exit_causes.push(e)) + .ok() + }) + .collect(); + ( + PreComputeArgs { + output_dir, + is_dataset_required, + input_files, + iexec_bulk_slice_size, + datasets, + }, + exit_causes, + ) } } @@ -210,8 +257,8 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_ok()); - let args = result.unwrap(); + assert!(result.1.is_empty()); + let args = result.0; assert_eq!(args.output_dir, OUTPUT_DIR); assert!(!args.is_dataset_required); @@ -232,8 +279,8 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_ok()); - let args = result.unwrap(); + assert!(result.1.is_empty()); + let args = result.0; assert_eq!(args.output_dir, OUTPUT_DIR); assert!(args.is_dataset_required); @@ -258,8 +305,8 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_ok()); - let args = result.unwrap(); + assert!(result.1.is_empty()); + let args = result.0; assert_eq!(args.output_dir, OUTPUT_DIR); assert!(!args.is_dataset_required); @@ -283,8 +330,8 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_ok()); - let args = result.unwrap(); + assert!(result.1.is_empty()); + let args = result.0; assert!(!args.is_dataset_required); }); } @@ -297,10 +344,10 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeIsDatasetRequiredMissing + result.1, + vec![ReplicateStatusCause::PreComputeIsDatasetRequiredMissing] ); }); } @@ -316,10 +363,10 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeInputFilesNumberMissing + result.1, + vec![ReplicateStatusCause::PreComputeInputFilesNumberMissing] ); }); } @@ -336,8 +383,8 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_ok()); - let args = result.unwrap(); + assert!(result.1.is_empty()); + let args = result.0; assert_eq!(args.output_dir, OUTPUT_DIR); assert!(!args.is_dataset_required); @@ -375,8 +422,8 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_ok()); - let args = result.unwrap(); + assert!(result.1.is_empty()); + let args = result.0; assert_eq!(args.output_dir, OUTPUT_DIR); assert!(args.is_dataset_required); @@ -405,10 +452,10 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeFailedUnknownIssue + result.1, + vec![ReplicateStatusCause::PreComputeFailedUnknownIssue] ); }); } @@ -424,10 +471,12 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeDatasetUrlMissing("bulk-dataset-1.txt".to_string()) + result.1, + vec![ReplicateStatusCause::PreComputeDatasetUrlMissing( + "bulk-dataset-1.txt".to_string() + )] ); }); } @@ -443,12 +492,12 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeDatasetChecksumMissing( + result.1, + vec![ReplicateStatusCause::PreComputeDatasetChecksumMissing( "bulk-dataset-2.txt".to_string() - ) + )] ); }); } @@ -464,10 +513,12 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeDatasetFilenameMissing("dataset_2".to_string()) + result.1, + vec![ReplicateStatusCause::PreComputeDatasetFilenameMissing( + "dataset_2".to_string() + )] ); }); } @@ -483,10 +534,12 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); + assert!(!result.1.is_empty()); assert_eq!( - result.unwrap_err(), - ReplicateStatusCause::PreComputeDatasetKeyMissing("bulk-dataset-1.txt".to_string()) + result.1, + vec![ReplicateStatusCause::PreComputeDatasetKeyMissing( + "bulk-dataset-1.txt".to_string() + )] ); }); } @@ -532,13 +585,13 @@ mod tests { ), ]; for (env_var, error) in missing_env_var_causes { - test_read_args_fails_with_missing_env_var(env_var, error); + test_read_args_fails_with_missing_env_var(env_var, vec![error]); } } fn test_read_args_fails_with_missing_env_var( env_var: TeeSessionEnvironmentVariable, - error: ReplicateStatusCause, + errors: Vec, ) { //Set up environment variables let mut env_vars = setup_basic_env_vars(); @@ -548,8 +601,261 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - assert!(result.is_err()); - assert_eq!(result.unwrap_err(), error); + assert!(!result.1.is_empty()); + assert_eq!(result.1, errors); + }); + } + // endregion + + // region error collection tests + #[test] + fn read_args_collects_multiple_errors_when_multiple_env_vars_missing() { + let mut env_vars = setup_basic_env_vars(); + env_vars.extend(setup_dataset_env_vars()); + env_vars.extend(setup_input_files_env_vars(2)); + + // Remove dataset URL and an input file URL + env_vars.remove(&IexecDatasetUrl(0).name()); + env_vars.remove(&IexecInputFileUrlPrefix(1).name()); + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect both errors (dataset stops at URL, input file error also collected) + assert_eq!(result.1.len(), 2); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetUrlMissing( + DATASET_FILENAME.to_string() + ))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(1))); + }); + } + + #[test] + fn read_args_collects_errors_for_partial_bulk_dataset_failures() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.extend(setup_input_files_env_vars(0)); + env_vars.extend(setup_bulk_dataset_env_vars(3)); + + // Remove various fields from different bulk datasets + env_vars.remove(&IexecDatasetUrl(1).name()); + env_vars.remove(&IexecDatasetChecksum(2).name()); + env_vars.remove(&IexecDatasetKey(3).name()); + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect all 3 errors + assert_eq!(result.1.len(), 3); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetUrlMissing( + "bulk-dataset-1.txt".to_string() + ))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetChecksumMissing( + "bulk-dataset-2.txt".to_string() + ))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetKeyMissing( + "bulk-dataset-3.txt".to_string() + ))); + + // No datasets should be added since they all had errors + assert_eq!(result.0.datasets.len(), 0); + }); + } + + #[test] + fn read_args_continues_processing_after_dataset_errors() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.extend(setup_input_files_env_vars(0)); + env_vars.extend(setup_bulk_dataset_env_vars(3)); + + // Remove only the second dataset's URL + env_vars.remove(&IexecDatasetUrl(2).name()); + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should have one error for the missing URL + assert_eq!(result.1.len(), 1); + assert_eq!( + result.1[0], + ReplicateStatusCause::PreComputeDatasetUrlMissing("bulk-dataset-2.txt".to_string()) + ); + + // Should successfully load the other two datasets + assert_eq!(result.0.datasets.len(), 2); + assert_eq!(result.0.datasets[0].url, "https://bulk-dataset-1.bin"); + assert_eq!(result.0.datasets[1].url, "https://bulk-dataset-3.bin"); + }); + } + + #[test] + fn read_args_collects_all_missing_input_file_urls() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.extend(setup_input_files_env_vars(5)); + + // Remove multiple input file URLs + env_vars.remove(&IexecInputFileUrlPrefix(2).name()); + env_vars.remove(&IexecInputFileUrlPrefix(4).name()); + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect errors for missing URLs + assert_eq!(result.1.len(), 2); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(4))); + + // Should successfully load the other three input files + assert_eq!(result.0.input_files.len(), 3); + assert_eq!(result.0.input_files[0], "https://input-1.txt"); + assert_eq!(result.0.input_files[1], "https://input-3.txt"); + assert_eq!(result.0.input_files[2], "https://input-5.txt"); + }); + } + + #[test] + fn read_args_handles_mixed_errors_across_all_categories() { + let mut env_vars = setup_basic_env_vars(); + env_vars.extend(setup_dataset_env_vars()); + env_vars.extend(setup_input_files_env_vars(3)); + env_vars.extend(setup_bulk_dataset_env_vars(2)); + + // Create errors across different categories + env_vars.insert(IsDatasetRequired.name(), "invalid-bool".to_string()); + // Since invalid bool defaults to false, dataset at index 0 won't be read + // So we need to remove fields from bulk datasets (indices 1 and 2) + env_vars.remove(&IexecDatasetChecksum(1).name()); // Bulk dataset 1 error + env_vars.remove(&IexecInputFileUrlPrefix(2).name()); // Input file error + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect: bool parse error, bulk dataset checksum error, input file error + assert_eq!(result.1.len(), 3); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeIsDatasetRequiredMissing)); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetChecksumMissing( + "bulk-dataset-1.txt".to_string() + ))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2))); + }); + } + + #[test] + fn read_args_processes_valid_datasets_despite_some_failures() { + let mut env_vars = setup_basic_env_vars(); + env_vars.extend(setup_dataset_env_vars()); + env_vars.extend(setup_input_files_env_vars(0)); + env_vars.extend(setup_bulk_dataset_env_vars(4)); + + // Break datasets at indices 1 and 3 + env_vars.remove(&IexecDatasetUrl(1).name()); + env_vars.remove(&IexecDatasetKey(3).name()); + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should have 2 errors + assert_eq!(result.1.len(), 2); + + // Should successfully load datasets at indices 0, 2, and 4 + assert_eq!(result.0.datasets.len(), 3); + assert_eq!(result.0.datasets[0].url, DATASET_URL); + assert_eq!(result.0.datasets[1].url, "https://bulk-dataset-2.bin"); + assert_eq!(result.0.datasets[2].url, "https://bulk-dataset-4.bin"); + }); + } + + #[test] + fn read_args_continues_after_bulk_slice_size_parse_error() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.extend(setup_input_files_env_vars(2)); + env_vars.insert(IexecBulkSliceSize.name(), "invalid-number".to_string()); + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect the parse error + assert_eq!(result.1.len(), 1); + assert_eq!(result.1[0], ReplicateStatusCause::PreComputeFailedUnknownIssue); + + // Should still process input files successfully + assert_eq!(result.0.input_files.len(), 2); + assert_eq!(result.0.iexec_bulk_slice_size, 0); + }); + } + + #[test] + fn read_args_collects_all_dataset_field_errors_for_single_dataset() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.extend(setup_input_files_env_vars(0)); + + // Set up only one bulk dataset but with missing filename (first field checked) + env_vars.insert(IexecBulkSliceSize.name(), "1".to_string()); + // Intentionally not setting filename - this will cause early exit from loop + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect error for missing filename (loop exits early, doesn't check other fields) + assert_eq!(result.1.len(), 1); + assert_eq!( + result.1[0], + ReplicateStatusCause::PreComputeDatasetFilenameMissing("dataset_1".to_string()) + ); + + // No dataset should be added + assert_eq!(result.0.datasets.len(), 0); + }); + } + + #[test] + fn read_args_stops_at_first_dataset_field_error() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.extend(setup_input_files_env_vars(0)); + + // Set up bulk dataset with filename but missing URL (second field checked) + env_vars.insert(IexecBulkSliceSize.name(), "1".to_string()); + env_vars.insert(IexecDatasetFilename(1).name(), "incomplete-dataset.txt".to_string()); + // Missing URL, checksum, and key - but should only report URL error + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should only collect error for the first missing field (URL) + assert_eq!(result.1.len(), 1); + assert_eq!( + result.1[0], + ReplicateStatusCause::PreComputeDatasetUrlMissing("incomplete-dataset.txt".to_string()) + ); + + // No dataset should be added + assert_eq!(result.0.datasets.len(), 0); + }); + } + + #[test] + fn read_args_handles_empty_input_files_list_with_errors() { + let mut env_vars = setup_basic_env_vars(); + env_vars.insert(IsDatasetRequired.name(), "false".to_string()); + env_vars.insert(IexecInputFilesNumber.name(), "3".to_string()); + // Intentionally not setting any input file URLs + + temp_env::with_vars(to_temp_env_vars(env_vars), || { + let result = PreComputeArgs::read_args(); + + // Should collect errors for all missing input files + assert_eq!(result.1.len(), 3); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(1))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2))); + assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(3))); + + // Input files should be empty + assert_eq!(result.0.input_files.len(), 0); }); } // endregion From 931746bac4f0490d7bbe5c889a169c3da4e5ff1c Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Thu, 23 Oct 2025 18:10:52 +0200 Subject: [PATCH 02/10] style: format the code --- pre-compute/src/compute/pre_compute_args.rs | 164 +++++++++++++------- 1 file changed, 112 insertions(+), 52 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index c008002..b1d15a1 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -613,20 +613,28 @@ mod tests { let mut env_vars = setup_basic_env_vars(); env_vars.extend(setup_dataset_env_vars()); env_vars.extend(setup_input_files_env_vars(2)); - + // Remove dataset URL and an input file URL env_vars.remove(&IexecDatasetUrl(0).name()); env_vars.remove(&IexecInputFileUrlPrefix(1).name()); temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect both errors (dataset stops at URL, input file error also collected) assert_eq!(result.1.len(), 2); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetUrlMissing( - DATASET_FILENAME.to_string() - ))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(1))); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeDatasetUrlMissing( + DATASET_FILENAME.to_string() + )) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(1)) + ); }); } @@ -636,7 +644,7 @@ mod tests { env_vars.insert(IsDatasetRequired.name(), "false".to_string()); env_vars.extend(setup_input_files_env_vars(0)); env_vars.extend(setup_bulk_dataset_env_vars(3)); - + // Remove various fields from different bulk datasets env_vars.remove(&IexecDatasetUrl(1).name()); env_vars.remove(&IexecDatasetChecksum(2).name()); @@ -644,19 +652,31 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect all 3 errors assert_eq!(result.1.len(), 3); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetUrlMissing( - "bulk-dataset-1.txt".to_string() - ))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetChecksumMissing( - "bulk-dataset-2.txt".to_string() - ))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetKeyMissing( - "bulk-dataset-3.txt".to_string() - ))); - + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeDatasetUrlMissing( + "bulk-dataset-1.txt".to_string() + )) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeDatasetChecksumMissing( + "bulk-dataset-2.txt".to_string() + )) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeDatasetKeyMissing( + "bulk-dataset-3.txt".to_string() + )) + ); + // No datasets should be added since they all had errors assert_eq!(result.0.datasets.len(), 0); }); @@ -668,20 +688,20 @@ mod tests { env_vars.insert(IsDatasetRequired.name(), "false".to_string()); env_vars.extend(setup_input_files_env_vars(0)); env_vars.extend(setup_bulk_dataset_env_vars(3)); - + // Remove only the second dataset's URL env_vars.remove(&IexecDatasetUrl(2).name()); temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should have one error for the missing URL assert_eq!(result.1.len(), 1); assert_eq!( result.1[0], ReplicateStatusCause::PreComputeDatasetUrlMissing("bulk-dataset-2.txt".to_string()) ); - + // Should successfully load the other two datasets assert_eq!(result.0.datasets.len(), 2); assert_eq!(result.0.datasets[0].url, "https://bulk-dataset-1.bin"); @@ -694,19 +714,27 @@ mod tests { let mut env_vars = setup_basic_env_vars(); env_vars.insert(IsDatasetRequired.name(), "false".to_string()); env_vars.extend(setup_input_files_env_vars(5)); - + // Remove multiple input file URLs env_vars.remove(&IexecInputFileUrlPrefix(2).name()); env_vars.remove(&IexecInputFileUrlPrefix(4).name()); temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect errors for missing URLs assert_eq!(result.1.len(), 2); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(4))); - + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2)) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(4)) + ); + // Should successfully load the other three input files assert_eq!(result.0.input_files.len(), 3); assert_eq!(result.0.input_files[0], "https://input-1.txt"); @@ -721,7 +749,7 @@ mod tests { env_vars.extend(setup_dataset_env_vars()); env_vars.extend(setup_input_files_env_vars(3)); env_vars.extend(setup_bulk_dataset_env_vars(2)); - + // Create errors across different categories env_vars.insert(IsDatasetRequired.name(), "invalid-bool".to_string()); // Since invalid bool defaults to false, dataset at index 0 won't be read @@ -731,14 +759,26 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect: bool parse error, bulk dataset checksum error, input file error assert_eq!(result.1.len(), 3); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeIsDatasetRequiredMissing)); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeDatasetChecksumMissing( - "bulk-dataset-1.txt".to_string() - ))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2))); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeIsDatasetRequiredMissing) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeDatasetChecksumMissing( + "bulk-dataset-1.txt".to_string() + )) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2)) + ); }); } @@ -748,17 +788,17 @@ mod tests { env_vars.extend(setup_dataset_env_vars()); env_vars.extend(setup_input_files_env_vars(0)); env_vars.extend(setup_bulk_dataset_env_vars(4)); - + // Break datasets at indices 1 and 3 env_vars.remove(&IexecDatasetUrl(1).name()); env_vars.remove(&IexecDatasetKey(3).name()); temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should have 2 errors assert_eq!(result.1.len(), 2); - + // Should successfully load datasets at indices 0, 2, and 4 assert_eq!(result.0.datasets.len(), 3); assert_eq!(result.0.datasets[0].url, DATASET_URL); @@ -776,11 +816,14 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect the parse error assert_eq!(result.1.len(), 1); - assert_eq!(result.1[0], ReplicateStatusCause::PreComputeFailedUnknownIssue); - + assert_eq!( + result.1[0], + ReplicateStatusCause::PreComputeFailedUnknownIssue + ); + // Should still process input files successfully assert_eq!(result.0.input_files.len(), 2); assert_eq!(result.0.iexec_bulk_slice_size, 0); @@ -792,21 +835,21 @@ mod tests { let mut env_vars = setup_basic_env_vars(); env_vars.insert(IsDatasetRequired.name(), "false".to_string()); env_vars.extend(setup_input_files_env_vars(0)); - + // Set up only one bulk dataset but with missing filename (first field checked) env_vars.insert(IexecBulkSliceSize.name(), "1".to_string()); // Intentionally not setting filename - this will cause early exit from loop temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect error for missing filename (loop exits early, doesn't check other fields) assert_eq!(result.1.len(), 1); assert_eq!( result.1[0], ReplicateStatusCause::PreComputeDatasetFilenameMissing("dataset_1".to_string()) ); - + // No dataset should be added assert_eq!(result.0.datasets.len(), 0); }); @@ -817,22 +860,27 @@ mod tests { let mut env_vars = setup_basic_env_vars(); env_vars.insert(IsDatasetRequired.name(), "false".to_string()); env_vars.extend(setup_input_files_env_vars(0)); - + // Set up bulk dataset with filename but missing URL (second field checked) env_vars.insert(IexecBulkSliceSize.name(), "1".to_string()); - env_vars.insert(IexecDatasetFilename(1).name(), "incomplete-dataset.txt".to_string()); + env_vars.insert( + IexecDatasetFilename(1).name(), + "incomplete-dataset.txt".to_string(), + ); // Missing URL, checksum, and key - but should only report URL error temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should only collect error for the first missing field (URL) assert_eq!(result.1.len(), 1); assert_eq!( result.1[0], - ReplicateStatusCause::PreComputeDatasetUrlMissing("incomplete-dataset.txt".to_string()) + ReplicateStatusCause::PreComputeDatasetUrlMissing( + "incomplete-dataset.txt".to_string() + ) ); - + // No dataset should be added assert_eq!(result.0.datasets.len(), 0); }); @@ -847,13 +895,25 @@ mod tests { temp_env::with_vars(to_temp_env_vars(env_vars), || { let result = PreComputeArgs::read_args(); - + // Should collect errors for all missing input files assert_eq!(result.1.len(), 3); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(1))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2))); - assert!(result.1.contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(3))); - + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(1)) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(2)) + ); + assert!( + result + .1 + .contains(&ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(3)) + ); + // Input files should be empty assert_eq!(result.0.input_files.len(), 0); }); From 07332fcbe3da451ff96460f1d96447d6a8e9c938 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Fri, 24 Oct 2025 00:22:47 +0200 Subject: [PATCH 03/10] fix: handle missing IexecBulkSliceSize by defaulting to 0 instead of error --- pre-compute/src/compute/pre_compute_args.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index b1d15a1..811fb07 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -85,12 +85,13 @@ impl PreComputeArgs { let iexec_bulk_slice_size = std::env::var(TeeSessionEnvironmentVariable::IexecBulkSliceSize.name()) - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or_else(|| { - exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); - 0 - }); // TODO: replace with a more specific error + .map(|s| { + s.parse::().unwrap_or_else(|_| { + exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); + 0 + }) + }) + .unwrap_or(0); // TODO: replace with a more specific error let mut datasets = Vec::with_capacity(iexec_bulk_slice_size + 1); From b16ac0863e5d9e3922db52623f3c3dfcea0fc158 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Mon, 27 Oct 2025 16:36:45 +0100 Subject: [PATCH 04/10] Refactor: use match pattern for error handling and add logs for read_args --- pre-compute/src/compute/pre_compute_args.rs | 172 +++++++++++++++----- 1 file changed, 127 insertions(+), 45 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index 811fb07..3e6b75d 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -1,6 +1,7 @@ use crate::compute::dataset::Dataset; use crate::compute::errors::ReplicateStatusCause; use crate::compute::utils::env_utils::{TeeSessionEnvironmentVariable, get_env_var_or_error}; +use log::{error, info}; /// Represents parameters required for pre-compute tasks in a Trusted Execution Environment (TEE). /// @@ -59,52 +60,89 @@ impl PreComputeArgs { /// let args = PreComputeArgs::read_args(); /// ``` pub fn read_args() -> (PreComputeArgs, Vec) { + info!("Starting to read pre-compute arguments from environment variables"); let mut exit_causes: Vec = vec![]; let output_dir = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecPreComputeOut, ReplicateStatusCause::PreComputeOutputPathMissing, ) { - Ok(output_dir) => output_dir, - Err(exit_cause) => return (PreComputeArgs::default(), vec![exit_cause]), + Ok(output_dir) => { + info!("Successfully read output directory: {output_dir}"); + output_dir + } + Err(e) => { + error!("Failed to read output directory: {e:?}"); + return (PreComputeArgs::default(), vec![e]); + } }; - let is_dataset_required = get_env_var_or_error( + let is_dataset_required = match get_env_var_or_error( TeeSessionEnvironmentVariable::IsDatasetRequired, ReplicateStatusCause::PreComputeIsDatasetRequiredMissing, - ) - .and_then(|s| { - s.to_lowercase() - .parse::() - .map_err(|_| ReplicateStatusCause::PreComputeIsDatasetRequiredMissing) - }) - .unwrap_or_else(|e| { - exit_causes.push(e); - false - }); + ) { + Ok(s) => match s.to_lowercase().parse::() { + Ok(value) => { + info!("Dataset required: {value}"); + value + } + Err(_) => { + error!("Invalid boolean format for IS_DATASET_REQUIRED: {s}"); + exit_causes.push(ReplicateStatusCause::PreComputeIsDatasetRequiredMissing); + false + } + }, + Err(e) => { + error!("Failed to read IS_DATASET_REQUIRED: {e:?}"); + exit_causes.push(e); + false + } + }; - let iexec_bulk_slice_size = - std::env::var(TeeSessionEnvironmentVariable::IexecBulkSliceSize.name()) - .map(|s| { - s.parse::().unwrap_or_else(|_| { - exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); - 0 - }) - }) - .unwrap_or(0); // TODO: replace with a more specific error + let iexec_bulk_slice_size = match get_env_var_or_error( + TeeSessionEnvironmentVariable::IexecBulkSliceSize, + ReplicateStatusCause::PreComputeFailedUnknownIssue, + ) { + Ok(s) => match s.parse::() { + Ok(value) => { + info!("Bulk slice size: {value}"); + value + } + Err(_) => { + error!("Invalid numeric format for IEXEC_BULK_SLICE_SIZE: {s}"); + exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); + 0 + } + }, + Err(e) => { + error!("Failed to read IEXEC_BULK_SLICE_SIZE: {e:?}"); + exit_causes.push(e); + 0 + } + }; // TODO: replace with a more specific error let mut datasets = Vec::with_capacity(iexec_bulk_slice_size + 1); // Read datasets let start_index = if is_dataset_required { 0 } else { 1 }; + info!( + "Reading datasets from index {start_index} to {iexec_bulk_slice_size} (is_dataset_required: {is_dataset_required})" + ); + for i in start_index..=iexec_bulk_slice_size { + info!("Processing dataset at index {i}"); + let filename = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetFilename(i), ReplicateStatusCause::PreComputeDatasetFilenameMissing(format!("dataset_{i}")), ) { - Ok(filename) => filename, - Err(exit_cause) => { - exit_causes.push(exit_cause); + Ok(filename) => { + info!("Dataset {i} filename: {filename}"); + filename + } + Err(e) => { + error!("Failed to read dataset {i} filename: {e:?}"); + exit_causes.push(e); continue; } }; @@ -113,9 +151,13 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecDatasetUrl(i), ReplicateStatusCause::PreComputeDatasetUrlMissing(filename.clone()), ) { - Ok(url) => url, - Err(exit_cause) => { - exit_causes.push(exit_cause); + Ok(url) => { + info!("Dataset {i} URL: {url}"); + url + } + Err(e) => { + error!("Failed to read dataset {i} URL: {e:?}"); + exit_causes.push(e); continue; } }; @@ -124,9 +166,13 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecDatasetChecksum(i), ReplicateStatusCause::PreComputeDatasetChecksumMissing(filename.clone()), ) { - Ok(checksum) => checksum, - Err(exit_cause) => { - exit_causes.push(exit_cause); + Ok(checksum) => { + info!("Dataset {i} checksum: {checksum}"); + checksum + } + Err(e) => { + error!("Failed to read dataset {i} checksum: {e:?}"); + exit_causes.push(e); continue; } }; @@ -135,39 +181,75 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecDatasetKey(i), ReplicateStatusCause::PreComputeDatasetKeyMissing(filename.clone()), ) { - Ok(key) => key, - Err(exit_cause) => { - exit_causes.push(exit_cause); + Ok(key) => { + info!("Dataset {i} key successfully read"); + key + } + Err(e) => { + error!("Failed to read dataset {i} key: {e:?}"); + exit_causes.push(e); continue; } }; + info!("Successfully loaded dataset {i} ({filename})"); datasets.push(Dataset::new(url, checksum, filename, key)); } + + info!("Successfully loaded {} datasets", datasets.len()); - let input_files_nb = get_env_var_or_error( + let input_files_nb = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecInputFilesNumber, ReplicateStatusCause::PreComputeInputFilesNumberMissing, - ) - .and_then(|s| { - s.parse::() - .map_err(|_| ReplicateStatusCause::PreComputeInputFilesNumberMissing) - }) - .unwrap_or_else(|e| { - exit_causes.push(e); - 0 - }); + ) { + Ok(s) => match s.parse::() { + Ok(value) => { + info!("Number of input files: {value}"); + value + } + Err(_) => { + error!("Invalid numeric format for IEXEC_INPUT_FILES_NUMBER: {s}"); + exit_causes.push(ReplicateStatusCause::PreComputeInputFilesNumberMissing); + 0 + } + }, + Err(e) => { + error!("Failed to read IEXEC_INPUT_FILES_NUMBER: {e:?}"); + exit_causes.push(e); + 0 + } + }; + info!("Reading {input_files_nb} input file URLs"); let input_files: Vec = (1..=input_files_nb) .filter_map(|i| { get_env_var_or_error( TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), ) - .map_err(|e| exit_causes.push(e)) + .map_err(|e| { + error!("Failed to read input file {i} URL: {e:?}"); + exit_causes.push(e) + }) .ok() + .map(|url| { + info!("Input file {i} URL: {url}"); + url + }) }) .collect(); + + info!("Successfully loaded {} input files", input_files.len()); + + if !exit_causes.is_empty() { + error!( + "Encountered {} error(s) while reading pre-compute arguments", + exit_causes.len() + ); + } else { + info!("Successfully read all pre-compute arguments without errors"); + } + ( PreComputeArgs { output_dir, From 5b1038d21276532bf4229e65847dd39f3ad3c634 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Mon, 27 Oct 2025 16:38:58 +0100 Subject: [PATCH 05/10] style: format the code --- pre-compute/src/compute/pre_compute_args.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index 3e6b75d..735fce1 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -128,10 +128,10 @@ impl PreComputeArgs { info!( "Reading datasets from index {start_index} to {iexec_bulk_slice_size} (is_dataset_required: {is_dataset_required})" ); - + for i in start_index..=iexec_bulk_slice_size { info!("Processing dataset at index {i}"); - + let filename = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetFilename(i), ReplicateStatusCause::PreComputeDatasetFilenameMissing(format!("dataset_{i}")), @@ -195,7 +195,7 @@ impl PreComputeArgs { info!("Successfully loaded dataset {i} ({filename})"); datasets.push(Dataset::new(url, checksum, filename, key)); } - + info!("Successfully loaded {} datasets", datasets.len()); let input_files_nb = match get_env_var_or_error( @@ -238,9 +238,9 @@ impl PreComputeArgs { }) }) .collect(); - + info!("Successfully loaded {} input files", input_files.len()); - + if !exit_causes.is_empty() { error!( "Encountered {} error(s) while reading pre-compute arguments", @@ -249,7 +249,7 @@ impl PreComputeArgs { } else { info!("Successfully read all pre-compute arguments without errors"); } - + ( PreComputeArgs { output_dir, From ca2eee9c2503ee22338f2ae2612aa7cc65ebd578 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Tue, 28 Oct 2025 11:14:55 +0100 Subject: [PATCH 06/10] refactor: remove logs --- pre-compute/src/compute/pre_compute_args.rs | 82 ++++----------------- 1 file changed, 16 insertions(+), 66 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index 735fce1..94332ef 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -67,12 +67,8 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecPreComputeOut, ReplicateStatusCause::PreComputeOutputPathMissing, ) { - Ok(output_dir) => { - info!("Successfully read output directory: {output_dir}"); - output_dir - } + Ok(output_dir) => output_dir, Err(e) => { - error!("Failed to read output directory: {e:?}"); return (PreComputeArgs::default(), vec![e]); } }; @@ -82,18 +78,13 @@ impl PreComputeArgs { ReplicateStatusCause::PreComputeIsDatasetRequiredMissing, ) { Ok(s) => match s.to_lowercase().parse::() { - Ok(value) => { - info!("Dataset required: {value}"); - value - } + Ok(value) => value, Err(_) => { - error!("Invalid boolean format for IS_DATASET_REQUIRED: {s}"); exit_causes.push(ReplicateStatusCause::PreComputeIsDatasetRequiredMissing); false } }, Err(e) => { - error!("Failed to read IS_DATASET_REQUIRED: {e:?}"); exit_causes.push(e); false } @@ -104,18 +95,13 @@ impl PreComputeArgs { ReplicateStatusCause::PreComputeFailedUnknownIssue, ) { Ok(s) => match s.parse::() { - Ok(value) => { - info!("Bulk slice size: {value}"); - value - } + Ok(value) => value, Err(_) => { - error!("Invalid numeric format for IEXEC_BULK_SLICE_SIZE: {s}"); exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); 0 } }, Err(e) => { - error!("Failed to read IEXEC_BULK_SLICE_SIZE: {e:?}"); exit_causes.push(e); 0 } @@ -125,23 +111,13 @@ impl PreComputeArgs { // Read datasets let start_index = if is_dataset_required { 0 } else { 1 }; - info!( - "Reading datasets from index {start_index} to {iexec_bulk_slice_size} (is_dataset_required: {is_dataset_required})" - ); - for i in start_index..=iexec_bulk_slice_size { - info!("Processing dataset at index {i}"); - let filename = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetFilename(i), ReplicateStatusCause::PreComputeDatasetFilenameMissing(format!("dataset_{i}")), ) { - Ok(filename) => { - info!("Dataset {i} filename: {filename}"); - filename - } + Ok(filename) => filename, Err(e) => { - error!("Failed to read dataset {i} filename: {e:?}"); exit_causes.push(e); continue; } @@ -151,12 +127,8 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecDatasetUrl(i), ReplicateStatusCause::PreComputeDatasetUrlMissing(filename.clone()), ) { - Ok(url) => { - info!("Dataset {i} URL: {url}"); - url - } + Ok(url) => url, Err(e) => { - error!("Failed to read dataset {i} URL: {e:?}"); exit_causes.push(e); continue; } @@ -166,12 +138,8 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecDatasetChecksum(i), ReplicateStatusCause::PreComputeDatasetChecksumMissing(filename.clone()), ) { - Ok(checksum) => { - info!("Dataset {i} checksum: {checksum}"); - checksum - } + Ok(checksum) => checksum, Err(e) => { - error!("Failed to read dataset {i} checksum: {e:?}"); exit_causes.push(e); continue; } @@ -181,66 +149,48 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecDatasetKey(i), ReplicateStatusCause::PreComputeDatasetKeyMissing(filename.clone()), ) { - Ok(key) => { - info!("Dataset {i} key successfully read"); - key - } + Ok(key) => key, Err(e) => { - error!("Failed to read dataset {i} key: {e:?}"); exit_causes.push(e); continue; } }; - info!("Successfully loaded dataset {i} ({filename})"); datasets.push(Dataset::new(url, checksum, filename, key)); } - info!("Successfully loaded {} datasets", datasets.len()); - let input_files_nb = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecInputFilesNumber, ReplicateStatusCause::PreComputeInputFilesNumberMissing, ) { Ok(s) => match s.parse::() { - Ok(value) => { - info!("Number of input files: {value}"); - value - } + Ok(value) => value, Err(_) => { - error!("Invalid numeric format for IEXEC_INPUT_FILES_NUMBER: {s}"); exit_causes.push(ReplicateStatusCause::PreComputeInputFilesNumberMissing); 0 } }, Err(e) => { - error!("Failed to read IEXEC_INPUT_FILES_NUMBER: {e:?}"); exit_causes.push(e); 0 } }; - info!("Reading {input_files_nb} input file URLs"); let input_files: Vec = (1..=input_files_nb) .filter_map(|i| { - get_env_var_or_error( + match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), - ) - .map_err(|e| { - error!("Failed to read input file {i} URL: {e:?}"); - exit_causes.push(e) - }) - .ok() - .map(|url| { - info!("Input file {i} URL: {url}"); - url - }) + ) { + Ok(url) => Some(url), + Err(e) => { + exit_causes.push(e); + None + } + } }) .collect(); - info!("Successfully loaded {} input files", input_files.len()); - if !exit_causes.is_empty() { error!( "Encountered {} error(s) while reading pre-compute arguments", From da73cf1dc481c490e5f1d6e1e287e5c9b78e406e Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Tue, 28 Oct 2025 14:10:27 +0100 Subject: [PATCH 07/10] refactor: improve error handling and logging for input files --- pre-compute/src/compute/errors.rs | 2 +- pre-compute/src/compute/pre_compute_app.rs | 14 +++++--- pre-compute/src/compute/pre_compute_args.rs | 36 +++++++++++++-------- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/pre-compute/src/compute/errors.rs b/pre-compute/src/compute/errors.rs index 751f0eb..fffe05a 100644 --- a/pre-compute/src/compute/errors.rs +++ b/pre-compute/src/compute/errors.rs @@ -28,7 +28,7 @@ pub enum ReplicateStatusCause { #[error("IS_DATASET_REQUIRED environment variable is missing")] PreComputeIsDatasetRequiredMissing, #[error("Input file download failed for input {0}")] - PreComputeInputFileDownloadFailed(usize), + PreComputeInputFileDownloadFailed(String), #[error("Input files number related environment variable is missing")] PreComputeInputFilesNumberMissing, #[error("Invalid dataset checksum for dataset {0}")] diff --git a/pre-compute/src/compute/pre_compute_app.rs b/pre-compute/src/compute/pre_compute_app.rs index adb679f..dbde843 100644 --- a/pre-compute/src/compute/pre_compute_app.rs +++ b/pre-compute/src/compute/pre_compute_app.rs @@ -114,7 +114,7 @@ impl PreComputeAppTrait for PreComputeApp { /// # Returns /// /// - `Ok(())` if all files are downloaded successfully. - /// - `Err(ReplicateStatusCause::PreComputeInputFileDownloadFailed)` if any file fails to download. + /// - `Err(ReplicateStatusCause::PreComputeInputFileDownloadFailed(url))` if any file fails to download. /// /// # Panics /// @@ -126,13 +126,13 @@ impl PreComputeAppTrait for PreComputeApp { let args = &self.pre_compute_args; let chain_task_id: &str = &self.chain_task_id; - for (index, url) in args.input_files.iter().enumerate() { + for url in args.input_files.iter() { info!("Downloading input file [chainTaskId:{chain_task_id}, url:{url}]"); let filename = sha256(url.to_string()); if download_file(url, &args.output_dir, &filename).is_none() { exit_causes.push(ReplicateStatusCause::PreComputeInputFileDownloadFailed( - index, + url.to_string(), )); } } @@ -317,7 +317,9 @@ mod tests { let result = app.download_input_files(); assert_eq!( result.unwrap_err(), - vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed(0)] + vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed( + "https://invalid-url-that-should-fail.com/file.txt".to_string() + )] ); } @@ -339,7 +341,9 @@ mod tests { let result = app.download_input_files(); assert_eq!( result.unwrap_err(), - vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed(1)] + vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed( + "https://invalid-url-that-should-fail.com/file.txt".to_string() + )] ); // First file should be downloaded with SHA256 filename diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index 94332ef..dc6b977 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -69,6 +69,7 @@ impl PreComputeArgs { ) { Ok(output_dir) => output_dir, Err(e) => { + error!("Failed to read output directory: {e:?}"); return (PreComputeArgs::default(), vec![e]); } }; @@ -80,11 +81,13 @@ impl PreComputeArgs { Ok(s) => match s.to_lowercase().parse::() { Ok(value) => value, Err(_) => { + error!("Invalid boolean format for IS_DATASET_REQUIRED: {s}"); exit_causes.push(ReplicateStatusCause::PreComputeIsDatasetRequiredMissing); false } }, Err(e) => { + error!("Failed to read IS_DATASET_REQUIRED: {e:?}"); exit_causes.push(e); false } @@ -97,11 +100,13 @@ impl PreComputeArgs { Ok(s) => match s.parse::() { Ok(value) => value, Err(_) => { + error!("Invalid numeric format for IEXEC_BULK_SLICE_SIZE: {s}"); exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); 0 } }, Err(e) => { + error!("Failed to read IEXEC_BULK_SLICE_SIZE: {e:?}"); exit_causes.push(e); 0 } @@ -118,6 +123,7 @@ impl PreComputeArgs { ) { Ok(filename) => filename, Err(e) => { + error!("Failed to read dataset {i} filename: {e:?}"); exit_causes.push(e); continue; } @@ -129,6 +135,7 @@ impl PreComputeArgs { ) { Ok(url) => url, Err(e) => { + error!("Failed to read dataset {i} URL: {e:?}"); exit_causes.push(e); continue; } @@ -140,6 +147,7 @@ impl PreComputeArgs { ) { Ok(checksum) => checksum, Err(e) => { + error!("Failed to read dataset {i} checksum: {e:?}"); exit_causes.push(e); continue; } @@ -151,6 +159,7 @@ impl PreComputeArgs { ) { Ok(key) => key, Err(e) => { + error!("Failed to read dataset {i} key: {e:?}"); exit_causes.push(e); continue; } @@ -166,30 +175,31 @@ impl PreComputeArgs { Ok(s) => match s.parse::() { Ok(value) => value, Err(_) => { + error!("Invalid numeric format for IEXEC_INPUT_FILES_NUMBER: {s}"); exit_causes.push(ReplicateStatusCause::PreComputeInputFilesNumberMissing); 0 } }, Err(e) => { + error!("Failed to read IEXEC_INPUT_FILES_NUMBER: {e:?}"); exit_causes.push(e); 0 } }; - let input_files: Vec = (1..=input_files_nb) - .filter_map(|i| { - match get_env_var_or_error( - TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), - ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), - ) { - Ok(url) => Some(url), - Err(e) => { - exit_causes.push(e); - None - } + let mut input_files: Vec = Vec::new(); + for i in 1..=input_files_nb { + match get_env_var_or_error( + TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), + ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), + ) { + Ok(url) => input_files.push(url), + Err(e) => { + error!("Failed to read input file {i} URL: {e:?}"); + exit_causes.push(e) } - }) - .collect(); + } + } if !exit_causes.is_empty() { error!( From 83b9d01f087c827e310c145a8d3034f96cf08d56 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Tue, 28 Oct 2025 17:03:55 +0100 Subject: [PATCH 08/10] refactor: move output_dir handling from PreComputeArgs to PreComputeApp --- pre-compute/src/compute/pre_compute_app.rs | 16 +++++++++++- pre-compute/src/compute/pre_compute_args.rs | 29 +++++---------------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_app.rs b/pre-compute/src/compute/pre_compute_app.rs index dbde843..f8b7c39 100644 --- a/pre-compute/src/compute/pre_compute_app.rs +++ b/pre-compute/src/compute/pre_compute_app.rs @@ -1,5 +1,6 @@ use crate::compute::errors::ReplicateStatusCause; use crate::compute::pre_compute_args::PreComputeArgs; +use crate::compute::utils::env_utils::{TeeSessionEnvironmentVariable, get_env_var_or_error}; use crate::compute::utils::file_utils::{download_file, write_file}; use crate::compute::utils::hash_utils::sha256; use log::{error, info}; @@ -56,7 +57,20 @@ impl PreComputeAppTrait for PreComputeApp { /// app.run(); /// ``` fn run(&mut self) -> Result<(), Vec> { - let (args, mut exit_causes) = PreComputeArgs::read_args(); + let (mut args, mut exit_causes): (PreComputeArgs, Vec); + match get_env_var_or_error( + TeeSessionEnvironmentVariable::IexecPreComputeOut, + ReplicateStatusCause::PreComputeOutputPathMissing, + ) { + Ok(output_dir) => { + (args, exit_causes) = PreComputeArgs::read_args(); + args.output_dir = output_dir; + } + Err(e) => { + error!("Failed to read output directory: {e:?}"); + return Err(vec![e]); + } + }; self.pre_compute_args = args; if let Err(exit_cause) = self.check_output_folder() { diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index dc6b977..f9976e7 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -63,17 +63,6 @@ impl PreComputeArgs { info!("Starting to read pre-compute arguments from environment variables"); let mut exit_causes: Vec = vec![]; - let output_dir = match get_env_var_or_error( - TeeSessionEnvironmentVariable::IexecPreComputeOut, - ReplicateStatusCause::PreComputeOutputPathMissing, - ) { - Ok(output_dir) => output_dir, - Err(e) => { - error!("Failed to read output directory: {e:?}"); - return (PreComputeArgs::default(), vec![e]); - } - }; - let is_dataset_required = match get_env_var_or_error( TeeSessionEnvironmentVariable::IsDatasetRequired, ReplicateStatusCause::PreComputeIsDatasetRequiredMissing, @@ -212,7 +201,7 @@ impl PreComputeArgs { ( PreComputeArgs { - output_dir, + output_dir: String::new(), is_dataset_required, input_files, iexec_bulk_slice_size, @@ -230,7 +219,6 @@ mod tests { use crate::compute::utils::env_utils::TeeSessionEnvironmentVariable::*; use std::collections::HashMap; - const OUTPUT_DIR: &str = "/iexec_out"; const DATASET_URL: &str = "https://dataset.url"; const DATASET_KEY: &str = "datasetKey123"; const DATASET_CHECKSUM: &str = "0x123checksum"; @@ -238,7 +226,6 @@ mod tests { fn setup_basic_env_vars() -> HashMap { let mut vars = HashMap::new(); - vars.insert(IexecPreComputeOut.name(), OUTPUT_DIR.to_string()); vars.insert(IsDatasetRequired.name(), "true".to_string()); vars.insert(IexecInputFilesNumber.name(), "0".to_string()); vars.insert(IexecBulkSliceSize.name(), "0".to_string()); // Default to no bulk processing @@ -303,7 +290,7 @@ mod tests { assert!(result.1.is_empty()); let args = result.0; - assert_eq!(args.output_dir, OUTPUT_DIR); + assert_eq!(args.output_dir, ""); assert!(!args.is_dataset_required); assert_eq!(args.input_files.len(), 1); assert_eq!(args.input_files[0], "https://input-1.txt"); @@ -325,7 +312,7 @@ mod tests { assert!(result.1.is_empty()); let args = result.0; - assert_eq!(args.output_dir, OUTPUT_DIR); + assert_eq!(args.output_dir, ""); assert!(args.is_dataset_required); assert_eq!(args.datasets[0].url, DATASET_URL.to_string()); assert_eq!(args.datasets[0].key, DATASET_KEY.to_string()); @@ -351,7 +338,7 @@ mod tests { assert!(result.1.is_empty()); let args = result.0; - assert_eq!(args.output_dir, OUTPUT_DIR); + assert_eq!(args.output_dir, ""); assert!(!args.is_dataset_required); assert_eq!(args.input_files.len(), 3); assert_eq!(args.input_files[0], "https://input-1.txt"); @@ -429,7 +416,7 @@ mod tests { assert!(result.1.is_empty()); let args = result.0; - assert_eq!(args.output_dir, OUTPUT_DIR); + assert_eq!(args.output_dir, ""); assert!(!args.is_dataset_required); assert_eq!(args.iexec_bulk_slice_size, 3); assert_eq!(args.datasets.len(), 3); @@ -468,7 +455,7 @@ mod tests { assert!(result.1.is_empty()); let args = result.0; - assert_eq!(args.output_dir, OUTPUT_DIR); + assert_eq!(args.output_dir, ""); assert!(args.is_dataset_required); assert_eq!(args.iexec_bulk_slice_size, 2); assert_eq!(args.datasets.len(), 3); // 1 regular + 2 bulk datasets @@ -592,10 +579,6 @@ mod tests { #[test] fn read_args_fails_when_dataset_env_var_missing() { let missing_env_var_causes = vec![ - ( - IexecPreComputeOut, - ReplicateStatusCause::PreComputeOutputPathMissing, - ), ( IsDatasetRequired, ReplicateStatusCause::PreComputeIsDatasetRequiredMissing, From a93ec8f7cd8c247f57fbc75d6afca549a64f4b24 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Tue, 28 Oct 2025 17:34:20 +0100 Subject: [PATCH 09/10] refacto: simplify vector initialization and Bulk Slice parsing --- pre-compute/src/compute/pre_compute_app.rs | 2 +- pre-compute/src/compute/pre_compute_args.rs | 19 ++++++------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_app.rs b/pre-compute/src/compute/pre_compute_app.rs index f8b7c39..e569fc1 100644 --- a/pre-compute/src/compute/pre_compute_app.rs +++ b/pre-compute/src/compute/pre_compute_app.rs @@ -136,7 +136,7 @@ impl PreComputeAppTrait for PreComputeApp { /// - `pre_compute_args` is `None`. /// - `chain_task_id` is `None`. fn download_input_files(&self) -> Result<(), Vec> { - let mut exit_causes: Vec = vec![]; + let mut exit_causes: Vec = Vec::new(); let args = &self.pre_compute_args; let chain_task_id: &str = &self.chain_task_id; diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index f9976e7..a7ebc25 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -61,7 +61,7 @@ impl PreComputeArgs { /// ``` pub fn read_args() -> (PreComputeArgs, Vec) { info!("Starting to read pre-compute arguments from environment variables"); - let mut exit_causes: Vec = vec![]; + let mut exit_causes: Vec = Vec::new(); let is_dataset_required = match get_env_var_or_error( TeeSessionEnvironmentVariable::IsDatasetRequired, @@ -86,19 +86,12 @@ impl PreComputeArgs { TeeSessionEnvironmentVariable::IexecBulkSliceSize, ReplicateStatusCause::PreComputeFailedUnknownIssue, ) { - Ok(s) => match s.parse::() { - Ok(value) => value, - Err(_) => { - error!("Invalid numeric format for IEXEC_BULK_SLICE_SIZE: {s}"); - exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); - 0 - } - }, - Err(e) => { - error!("Failed to read IEXEC_BULK_SLICE_SIZE: {e:?}"); - exit_causes.push(e); + Ok(s) => s.parse::().unwrap_or_else(|_| { + error!("Invalid numeric format for IEXEC_BULK_SLICE_SIZE: {s}"); + exit_causes.push(ReplicateStatusCause::PreComputeFailedUnknownIssue); 0 - } + }), + Err(_) => 0, }; // TODO: replace with a more specific error let mut datasets = Vec::with_capacity(iexec_bulk_slice_size + 1); From 81d1240357256a83614a2399b3e5d2a072b73ce7 Mon Sep 17 00:00:00 2001 From: nabil-Tounarti Date: Wed, 29 Oct 2025 09:34:10 +0100 Subject: [PATCH 10/10] docs: update documentation --- pre-compute/src/compute/pre_compute_app.rs | 31 ++++++++++++--------- pre-compute/src/compute/pre_compute_args.rs | 24 ++++++++-------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/pre-compute/src/compute/pre_compute_app.rs b/pre-compute/src/compute/pre_compute_app.rs index e569fc1..f10eaf6 100644 --- a/pre-compute/src/compute/pre_compute_app.rs +++ b/pre-compute/src/compute/pre_compute_app.rs @@ -38,15 +38,19 @@ impl PreComputeAppTrait for PreComputeApp { /// Runs the complete pre-compute pipeline. /// /// This method orchestrates the entire pre-compute process: - /// 1. Reads configuration arguments - /// 2. Validates the output folder exists - /// 3. Downloads and decrypts the dataset (if required) - /// 4. Downloads all input files + /// 1. Reads the output directory from environment variable `IEXEC_PRE_COMPUTE_OUT` + /// 2. Reads and validates configuration arguments from environment variables + /// 3. Validates the output folder exists + /// 4. Downloads and decrypts all datasets (if required) + /// 5. Downloads all input files + /// + /// The method collects all errors encountered during execution and returns them together, + /// allowing partial completion when possible (e.g., if one dataset fails, others are still processed). /// /// # Returns /// /// - `Ok(())` if all operations completed successfully - /// - `Err(ReplicateStatusCause)` if any step failed + /// - `Err(Vec)` containing all errors encountered during execution /// /// # Example /// @@ -123,18 +127,19 @@ impl PreComputeAppTrait for PreComputeApp { /// Downloads the input files listed in `pre_compute_args.input_files` to the specified `output_dir`. /// /// Each URL is hashed (SHA-256) to generate a unique local filename. - /// If any download fails, the function returns an error. + /// The method continues downloading all files even if some downloads fail. /// - /// # Returns + /// # Behavior /// - /// - `Ok(())` if all files are downloaded successfully. - /// - `Err(ReplicateStatusCause::PreComputeInputFileDownloadFailed(url))` if any file fails to download. + /// - Downloads continue even when individual files fail + /// - Successfully downloaded files are saved with SHA-256 hashed filenames + /// - All download failures are collected and returned together /// - /// # Panics + /// # Returns /// - /// This function panics if: - /// - `pre_compute_args` is `None`. - /// - `chain_task_id` is `None`. + /// - `Ok(())` if all files are downloaded successfully + /// - `Err(Vec)` containing a `PreComputeInputFileDownloadFailed` error + /// for each file that failed to download fn download_input_files(&self) -> Result<(), Vec> { let mut exit_causes: Vec = Vec::new(); let args = &self.pre_compute_args; diff --git a/pre-compute/src/compute/pre_compute_args.rs b/pre-compute/src/compute/pre_compute_args.rs index a7ebc25..671271f 100644 --- a/pre-compute/src/compute/pre_compute_args.rs +++ b/pre-compute/src/compute/pre_compute_args.rs @@ -23,10 +23,12 @@ pub struct PreComputeArgs { impl PreComputeArgs { /// Constructs a validated `PreComputeArgs` instance by reading and validating environment variables. /// + /// This method collects all errors encountered during validation instead of failing on the first error, + /// allowing for complete error reporting and partial processing where possible. + /// /// # Environment Variables /// This method reads the following environment variables: /// - Required for all tasks: - /// - `IEXEC_PRE_COMPUTE_OUT`: Output directory path /// - `IEXEC_DATASET_REQUIRED`: Boolean ("true"/"false") indicating dataset requirement /// - `IEXEC_INPUT_FILES_NUMBER`: Number of input files to load /// - `IEXEC_BULK_SLICE_SIZE`: Number of bulk datasets (0 means no bulk processing) @@ -42,22 +44,22 @@ impl PreComputeArgs { /// - `IEXEC_DATASET_#_KEY`: Dataset decryption key /// - Input file URLs (`IEXEC_INPUT_FILE_URL_1`, `IEXEC_INPUT_FILE_URL_2`, etc.) /// - /// # Errors - /// Returns `ReplicateStatusCause` error variants for: - /// - Missing required environment variables - /// - Invalid boolean values in `IEXEC_DATASET_REQUIRED` - /// - Invalid numeric format in `IEXEC_INPUT_FILES_NUMBER` or `IEXEC_BULK_SLICE_SIZE` - /// - Missing dataset parameters when required - /// - Missing input file URLs - /// - Missing bulk dataset parameters when bulk processing is enabled + /// # Returns + /// + /// Returns a tuple containing: + /// - `PreComputeArgs`: The constructed arguments (with `output_dir` set to empty string) + /// - `Vec`: A vector of all errors encountered (empty if successful) /// /// # Example /// /// ```rust /// use tee_worker_pre_compute::compute::pre_compute_args::PreComputeArgs; /// - /// // Typically called with task ID from execution context - /// let args = PreComputeArgs::read_args(); + /// let (mut args, errors) = PreComputeArgs::read_args(); + /// if !errors.is_empty() { + /// eprintln!("Encountered {} error(s) while reading arguments", errors.len()); + /// } + /// args.output_dir = "/path/to/output".to_string(); // Set output_dir separately /// ``` pub fn read_args() -> (PreComputeArgs, Vec) { info!("Starting to read pre-compute arguments from environment variables");