From 46a453e6af072e7681c5f7ea1ee8777c3a85eb64 Mon Sep 17 00:00:00 2001
From: Niklas Saari <niklas.saari@tutanota.com>
Date: Thu, 26 Sep 2024 18:14:07 +0300
Subject: [PATCH] Improve the error handling of file upload

---
 src/bin/cli.rs       | 21 +++++++++++++--------
 src/build_process.rs |  2 +-
 src/errors/base.rs   |  6 ++++++
 src/storages/s3.rs   |  8 ++++----
 4 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/src/bin/cli.rs b/src/bin/cli.rs
index 0ad9548..70c3689 100644
--- a/src/bin/cli.rs
+++ b/src/bin/cli.rs
@@ -1,6 +1,7 @@
 use ainigma::{
     build_process::{build_task, TaskBuildProcessOutput},
     config::{read_check_toml, ConfigError, ModuleConfiguration},
+    errors::CloudStorageError,
     moodle::create_exam,
     storages::{CloudStorage, FileObjects, S3Storage},
 };
@@ -116,6 +117,10 @@ fn s3_upload(
             return Err(e.into());
         }
     }
+    tracing::info!(
+        "Strating the file upload into the bucket: {}",
+        config.deployment.upload.bucket_name.as_str()
+    );
 
     for mut file in files {
         let module_nro = config
@@ -130,16 +135,14 @@ fn s3_upload(
             file.uiid
         );
         let future = async {
-            match FileObjects::new(dst_location, file.get_resource_files()) {
+            match FileObjects::new(dst_location, file.get_resource_files())
+                .map_err(CloudStorageError::FileObjectError)
+            {
                 Ok(files) => {
                     let items = storage
                         .upload(files, config.deployment.upload.use_pre_signed)
-                        .await
-                        .unwrap_or_else(|error| {
-                            tracing::error!("Error when uploading the files: {}", error);
-                            <_>::default()
-                        });
-                    file.refresh_files(items);
+                        .await?;
+                    file.update_files(items);
                     Ok(file)
                 }
                 Err(error) => {
@@ -162,10 +165,12 @@ fn s3_upload(
                     }
                 }
             }
+            tracing::info!("All {} files uploaded successfully.", files.len());
             Ok(files)
         }
         Err(error) => {
-            tracing::error!("Error when uploading the files: {}", error);
+            tracing::error!("Overall file upload process resulted with error: {}", error);
+            tracing::error!("There is a chance that you are rate limited by the cloud storage. Please try again later.");
             Err(error.into())
         }
     }
diff --git a/src/build_process.rs b/src/build_process.rs
index d096c19..0cd5828 100644
--- a/src/build_process.rs
+++ b/src/build_process.rs
@@ -138,7 +138,7 @@ impl TaskBuildProcessOutput {
             .find(|output| matches!(output.kind, OutputKind::Readme(_)))
     }
 
-    pub fn refresh_files(&mut self, items: Vec<OutputItem>) {
+    pub fn update_files(&mut self, items: Vec<OutputItem>) {
         for item in items {
             if let Some(index) = self.files.iter().position(|x| x.kind == item.kind) {
                 self.files[index] = item;
diff --git a/src/errors/base.rs b/src/errors/base.rs
index 28c7a5e..6bbefcc 100644
--- a/src/errors/base.rs
+++ b/src/errors/base.rs
@@ -38,6 +38,12 @@ pub enum CloudStorageError {
     FileReadError(String),
     #[error("Failed to parse URL: {0}")]
     UrlParseError(String),
+    // wrap FileObjectError
+    #[error(transparent)]
+    FileObjectError(#[from] FileObjectError),
+    // upload error
+    #[error("Failed to upload file: {0}")]
+    UploadError(String),
 }
 
 #[derive(Error, Debug)]
diff --git a/src/storages/s3.rs b/src/storages/s3.rs
index a8e22a7..0546f7b 100644
--- a/src/storages/s3.rs
+++ b/src/storages/s3.rs
@@ -107,7 +107,7 @@ impl CloudStorage for S3Storage {
             .await;
         match exist {
             Ok(resp) => {
-                tracing::info!("Bucket identified: {:?}!", resp);
+                tracing::debug!("Bucket identified: {:?}!", resp);
                 // TODO - Implement the lifecycle configuration
                 // tracing::info!("The result of the bucket lifecycle update: {:?}", result);
                 Ok(())
@@ -125,7 +125,7 @@ impl CloudStorage for S3Storage {
         files: FileObjects,
         pre_signed_urls: bool,
     ) -> Result<Vec<OutputItem>, CloudStorageError> {
-        tracing::info!(
+        tracing::debug!(
             "Starting to upload {} files in to the S3 bucket '{}' in path '{}'.",
             files.len(),
             self.bucket,
@@ -152,7 +152,7 @@ impl CloudStorage for S3Storage {
                             .await;
                         match response {
                             Ok(r) => {
-                                tracing::info!(
+                                tracing::debug!(
                                     "Created or updated the file with expiration: {}",
                                     r.expiration.unwrap_or_default()
                                 );
@@ -222,7 +222,7 @@ impl CloudStorage for S3Storage {
         match upload_results {
             Ok(_) => {
                 let mut vec = shared_vec.lock().await;
-                tracing::info!("Uploaded {} files successfully.", vec.len());
+                tracing::debug!("Uploaded {} files successfully.", vec.len());
                 Ok(core::mem::take(&mut vec))
             }
             Err(e) => {