From 7e27bac66e0ffdf4453dbae103aa233520440673 Mon Sep 17 00:00:00 2001
From: Johannes Batzill <johannes.batzill@harness.io>
Date: Fri, 25 Oct 2024 19:08:17 +0000
Subject: [PATCH] fix: [CODE-2625]: Improve Error Handling for `blob` upload
 errors (#2870)

---
 blob/filesystem.go |  9 ++++++---
 blob/gcs.go        | 14 ++++++++------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/blob/filesystem.go b/blob/filesystem.go
index 52d94f818..3ecaed550 100644
--- a/blob/filesystem.go
+++ b/blob/filesystem.go
@@ -59,8 +59,8 @@ func (c FileSystemStore) Upload(ctx context.Context,
 	defer func() {
 		cErr := destinationFile.Close()
 		if cErr != nil {
-			log.Ctx(ctx).Err(cErr).
-				Msgf("failed to close destination file '%s' in directory '%s'", filePath, c.basePath)
+			log.Ctx(ctx).Warn().Err(cErr).
+				Msgf("failed to close destination file %q in directory %q", filePath, c.basePath)
 		}
 	}()
 
@@ -68,7 +68,10 @@ func (c FileSystemStore) Upload(ctx context.Context,
 		// Remove the file if it was created.
 		removeErr := os.Remove(fileDiskPath)
 		if removeErr != nil {
-			return fmt.Errorf("failed to remove file: %w", removeErr)
+			// Best effort attempt to remove the file on write failure.
+			log.Ctx(ctx).Warn().Err(removeErr).Msgf(
+				"failed to cleanup file %q in directory %q after write to filesystem failed with %s",
+				filePath, c.basePath, err)
 		}
 		return fmt.Errorf("failed to write file to filesystem: %w", err)
 	}
diff --git a/blob/gcs.go b/blob/gcs.go
index 7c841131b..fd4e5f03f 100644
--- a/blob/gcs.go
+++ b/blob/gcs.go
@@ -75,15 +75,17 @@ func (c *GCSStore) Upload(ctx context.Context, file io.Reader, filePath string)
 	defer func() {
 		cErr := wc.Close()
 		if cErr != nil {
-			log.Ctx(ctx).Err(cErr).
-				Msgf("failed to close gcs blob writer for file '%s' in bucket '%s'", filePath, c.config.Bucket)
+			log.Ctx(ctx).Warn().Err(cErr).
+				Msgf("failed to close gcs blob writer for file %q in bucket %q", filePath, c.config.Bucket)
 		}
 	}()
 	if _, err := io.Copy(wc, file); err != nil {
-		// Remove the file if it was created.
+		// Best effort attempt to delete the file on upload failure.
 		deleteErr := gcsClient.Bucket(c.config.Bucket).Object(filePath).Delete(ctx)
 		if deleteErr != nil {
-			return fmt.Errorf("failed to delete file: %s from bucket: %s %w", filePath, c.config.Bucket, deleteErr)
+			log.Ctx(ctx).Warn().Err(deleteErr).Msgf(
+				"failed to cleanup file %q from bucket %q after write to gcs failed with %s",
+				filePath, c.config.Bucket, err)
 		}
 		return fmt.Errorf("failed to write file to GCS: %w", err)
 	}
@@ -103,7 +105,7 @@ func (c *GCSStore) GetSignedURL(ctx context.Context, filePath string) (string, e
 		Expires: time.Now().Add(1 * time.Hour),
 	})
 	if err != nil {
-		return "", fmt.Errorf("failed to create signed URL for file: %s %w", filePath, err)
+		return "", fmt.Errorf("failed to create signed URL for file %q: %w", filePath, err)
 	}
 	return signedURL, nil
 }
@@ -120,7 +122,7 @@ func createNewImpersonatedClient(ctx context.Context, cfg Config) (*storage.Clie
 		Lifetime:        cfg.ImpersonationLifetime,
 	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to impersonate the client service account %s : %w", cfg.TargetPrincipal, err)
+		return nil, fmt.Errorf("failed to impersonate the client service account %q: %w", cfg.TargetPrincipal, err)
 	}
 
 	// Generate a new token