From 90a8fe99ff871b8dee19ccaebabe967e9a33d179 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Istv=C3=A1n=20So=C3=B3s?= <isoos@users.noreply.github.com>
Date: Fri, 6 Dec 2024 07:41:28 +0100
Subject: [PATCH] Retry the tryDelete and tryInfo operations on storage
 buckets. (#8365)

---
 app/lib/shared/storage.dart | 83 +++++++++++++++++++------------------
 app/lib/shared/utils.dart   |  4 +-
 2 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/app/lib/shared/storage.dart b/app/lib/shared/storage.dart
index 2e7af04ec..e9e936109 100644
--- a/app/lib/shared/storage.dart
+++ b/app/lib/shared/storage.dart
@@ -75,22 +75,34 @@ extension StorageExt on Storage {
 extension BucketExt on Bucket {
   /// Returns an [ObjectInfo] if [name] exists, `null` otherwise.
   Future<ObjectInfo?> tryInfo(String name) async {
-    try {
-      return await info(name);
-    } on DetailedApiRequestError catch (e) {
-      if (e.status == 404) return null;
-      rethrow;
-    }
+    return await retry(
+      () async {
+        try {
+          return await info(name);
+        } on DetailedApiRequestError catch (e) {
+          if (e.status == 404) return null;
+          rethrow;
+        }
+      },
+      maxAttempts: 3,
+      retryIf: _retryIf,
+    );
   }
 
   /// Deletes [name] if it exists, ignores 404 otherwise.
   Future<void> tryDelete(String name) async {
-    try {
-      return await delete(name);
-    } on DetailedApiRequestError catch (e) {
-      if (e.status == 404) return null;
-      rethrow;
-    }
+    return await retry(
+      () async {
+        try {
+          return await delete(name);
+        } on DetailedApiRequestError catch (e) {
+          if (e.status == 404) return null;
+          rethrow;
+        }
+      },
+      maxAttempts: 3,
+      retryIf: _retryIf,
+    );
   }
 
   Future uploadPublic(String objectName, int length,
@@ -139,22 +151,7 @@ extension BucketExt on Bucket {
         return builder.toBytes();
       },
       maxAttempts: 3,
-      retryIf: (e) {
-        if (e is TimeoutException) {
-          return true; // Timeouts we can retry
-        }
-        if (e is IOException) {
-          return true; // I/O issues are worth retrying
-        }
-        if (e is http.ClientException) {
-          return true; // HTTP issues are worth retrying
-        }
-        if (e is DetailedApiRequestError) {
-          final status = e.status;
-          return status == null || status >= 500; // 5xx errors are retried
-        }
-        return e is ApiRequestError; // Unknown API errors are retried
-      },
+      retryIf: _retryIf,
     );
   }
 
@@ -164,6 +161,23 @@ extension BucketExt on Bucket {
   }
 }
 
+bool _retryIf(Exception e) {
+  if (e is TimeoutException) {
+    return true; // Timeouts we can retry
+  }
+  if (e is IOException) {
+    return true; // I/O issues are worth retrying
+  }
+  if (e is http.ClientException) {
+    return true; // HTTP issues are worth retrying
+  }
+  if (e is DetailedApiRequestError) {
+    final status = e.status;
+    return status == null || status >= 500; // 5xx errors are retried
+  }
+  return e is ApiRequestError; // Unknown API errors are retried
+}
+
 /// Returns a valid `gs://` URI for a given [bucket] + [path] combination.
 String bucketUri(Bucket bucket, String path) =>
     'gs://${bucket.bucketName}/$path';
@@ -264,18 +278,7 @@ Future uploadWithRetry(Bucket bucket, String objectName, int length,
       await sink.close();
     },
     description: 'Upload to $objectName',
-    shouldRetryOnError: (e) {
-      // upstream proxy or rate limit issue
-      if (e is DetailedApiRequestError) {
-        return _retryStatusCodes.contains(e.status);
-      }
-      // network connection failures
-      if (e is http.ClientException || e is SocketException) {
-        return true;
-      }
-      // otherwise no retry
-      return false;
-    },
+    shouldRetryOnError: _retryIf,
     sleep: Duration(seconds: 10),
   );
 }
diff --git a/app/lib/shared/utils.dart b/app/lib/shared/utils.dart
index 0e5ec1f9e..12ef7d7fd 100644
--- a/app/lib/shared/utils.dart
+++ b/app/lib/shared/utils.dart
@@ -176,14 +176,14 @@ List<T> boundedList<T>(List<T> list, {int? offset, int? limit}) {
 Future<R> retryAsync<R>(
   Future<R> Function() body, {
   int maxAttempt = 3,
-  bool Function(Object)? shouldRetryOnError,
+  bool Function(Exception)? shouldRetryOnError,
   String description = 'Async operation',
   Duration sleep = const Duration(seconds: 1),
 }) async {
   for (int i = 1;; i++) {
     try {
       return await body();
-    } catch (e, st) {
+    } on Exception catch (e, st) {
       _logger.info('$description failed (attempt: $i of $maxAttempt).', e, st);
       if (i < maxAttempt &&
           (shouldRetryOnError == null || shouldRetryOnError(e))) {