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..fffe05a 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(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 825f3ef..f10eaf6 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}; @@ -9,9 +10,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], @@ -37,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 /// @@ -55,17 +60,46 @@ 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 (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() { + 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. @@ -93,31 +127,40 @@ 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)` 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`. - fn download_input_files(&self) -> Result<(), ReplicateStatusCause> { + /// - `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; let chain_task_id: &str = &self.chain_task_id; - for url in &args.input_files { + 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() { - return Err(ReplicateStatusCause::PreComputeInputFileDownloadFailed); + exit_causes.push(ReplicateStatusCause::PreComputeInputFileDownloadFailed( + url.to_string(), + )); } } - 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 +336,14 @@ mod tests { let result = app.download_input_files(); assert_eq!( result.unwrap_err(), - ReplicateStatusCause::PreComputeInputFileDownloadFailed + vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed( + "https://invalid-url-that-should-fail.com/file.txt".to_string() + )] ); } #[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 +352,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 +360,18 @@ mod tests { let result = app.download_input_files(); assert_eq!( result.unwrap_err(), - ReplicateStatusCause::PreComputeInputFileDownloadFailed + vec![ReplicateStatusCause::PreComputeInputFileDownloadFailed( + "https://invalid-url-that-should-fail.com/file.txt".to_string() + )] ); // 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..671271f 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). /// @@ -22,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) @@ -41,95 +44,166 @@ 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() -> Result { - let output_dir = get_env_var_or_error( - TeeSessionEnvironmentVariable::IexecPreComputeOut, - ReplicateStatusCause::PreComputeOutputPathMissing, - )?; + pub fn read_args() -> (PreComputeArgs, Vec) { + info!("Starting to read pre-compute arguments from environment variables"); + let mut exit_causes: Vec = Vec::new(); - let is_dataset_required_str = get_env_var_or_error( + let is_dataset_required = match 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 = - 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(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 + } + }; + + let iexec_bulk_slice_size = match get_env_var_or_error( + TeeSessionEnvironmentVariable::IexecBulkSliceSize, + ReplicateStatusCause::PreComputeFailedUnknownIssue, + ) { + 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); // 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(e) => { + error!("Failed to read dataset {i} filename: {e:?}"); + exit_causes.push(e); + 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(e) => { + error!("Failed to read dataset {i} URL: {e:?}"); + exit_causes.push(e); + 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(e) => { + error!("Failed to read dataset {i} checksum: {e:?}"); + exit_causes.push(e); + continue; + } + }; + + let key = match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecDatasetKey(i), ReplicateStatusCause::PreComputeDatasetKeyMissing(filename.clone()), - )?; + ) { + Ok(key) => key, + Err(e) => { + error!("Failed to read dataset {i} key: {e:?}"); + exit_causes.push(e); + continue; + } + }; datasets.push(Dataset::new(url, checksum, filename, key)); } - let input_files_nb_str = get_env_var_or_error( + let input_files_nb = match 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); + ) { + 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 mut input_files: Vec = Vec::new(); for i in 1..=input_files_nb { - let url = get_env_var_or_error( + match get_env_var_or_error( TeeSessionEnvironmentVariable::IexecInputFileUrlPrefix(i), ReplicateStatusCause::PreComputeAtLeastOneInputFileUrlMissing(i), - )?; - input_files.push(url); + ) { + Ok(url) => input_files.push(url), + Err(e) => { + error!("Failed to read input file {i} URL: {e:?}"); + exit_causes.push(e) + } + } + } + + 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"); } - Ok(PreComputeArgs { - output_dir, - is_dataset_required, - input_files, - iexec_bulk_slice_size, - datasets, - }) + ( + PreComputeArgs { + output_dir: String::new(), + is_dataset_required, + input_files, + iexec_bulk_slice_size, + datasets, + }, + exit_causes, + ) } } @@ -140,7 +214,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"; @@ -148,7 +221,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 @@ -210,10 +282,10 @@ 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_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"); @@ -232,10 +304,10 @@ 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_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()); @@ -258,10 +330,10 @@ 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_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"); @@ -283,8 +355,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 +369,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 +388,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,10 +408,10 @@ 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_eq!(args.output_dir, ""); assert!(!args.is_dataset_required); assert_eq!(args.iexec_bulk_slice_size, 3); assert_eq!(args.datasets.len(), 3); @@ -375,10 +447,10 @@ 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_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 @@ -405,10 +477,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 +496,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 +517,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 +538,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 +559,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() + )] ); }); } @@ -496,10 +574,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, @@ -532,13 +606,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 +622,321 @@ 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