Use retry callback block for admin API in API exporter tests. (#8485)

dart-lang · Jan 23, 2025 · ac0d446 · ac0d446
1 parent e684190
commit ac0d446
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 60 deletions.
diff --git a/app/lib/tool/utils/pub_api_client.dart b/app/lib/tool/utils/pub_api_client.dart
@@ -2,7 +2,9 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'dart:async';
 import 'dart:convert';
+import 'dart:io';
 import 'dart:typed_data';
 
 import 'package:_pub_shared/data/package_api.dart';
@@ -11,6 +13,7 @@ import 'package:gcloud/service_scope.dart';
 import 'package:http/http.dart' as http;
 import 'package:meta/meta.dart';
 import 'package:pub_dev/frontend/handlers/experimental.dart';
+import 'package:retry/retry.dart';
 
 import '../../frontend/handlers/pubapi.client.dart';
 import '../../service/services.dart';
@@ -91,6 +94,7 @@ class _FakeTimeClient implements http.Client {
 
 /// Creates a pub.dev API client and executes [fn], making sure that the HTTP
 /// resources are freed after the callback finishes.
+/// The callback [fn] is retried on the transient network errors.
 ///
 /// If [bearerToken], [sessionId] or [csrfToken] is specified, the corresponding
 /// HTTP header will be sent alongside the request.
@@ -109,16 +113,40 @@ Future<R> withHttpPubApiClient<R>({
     cookieProvider: () async => {
       if (experimental != null) experimentalCookieName: experimental.join(':'),
     },
+    client: http.Client(),
   );
-  try {
-    final apiClient = PubApiClient(
-      pubHostedUrl ?? activeConfiguration.primaryApiUri!.toString(),
-      client: httpClient,
-    );
-    return await fn(apiClient);
-  } finally {
-    httpClient.close();
+  return await retry(
+    () async {
+      try {
+        final apiClient = PubApiClient(
+          pubHostedUrl ?? activeConfiguration.primaryApiUri!.toString(),
+          client: httpClient,
+        );
+        return await fn(apiClient);
+      } finally {
+        httpClient.close();
+      }
+    },
+    maxAttempts: 3,
+    retryIf: _retryIf,
+  );
+}
+
+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 RequestException) {
+    final status = e.status;
+    return status >= 500; // 5xx errors are retried
   }
+  return false;
 }
 
 extension PubApiClientExt on PubApiClient {

diff --git a/app/test/admin/exported_api_sync_test.dart b/app/test/admin/exported_api_sync_test.dart
@@ -19,13 +19,17 @@ void main() {
       List<String>? packages,
       bool forceWrite = false,
     }) async {
-      final api = createPubApiClient(authToken: siteAdminToken);
-      await api.adminInvokeAction(
-        'exported-api-sync',
-        AdminInvokeActionArguments(arguments: {
-          'packages': packages?.join(' ') ?? 'ALL',
-          if (forceWrite) 'force-write': 'true',
-        }),
+      await withHttpPubApiClient(
+        bearerToken: siteAdminToken,
+        fn: (api) async {
+          await api.adminInvokeAction(
+            'exported-api-sync',
+            AdminInvokeActionArguments(arguments: {
+              'packages': packages?.join(' ') ?? 'ALL',
+              if (forceWrite) 'force-write': 'true',
+            }),
+          );
+        },
       );
     }
 

diff --git a/app/test/package/api_export/api_exporter_test.dart b/app/test/package/api_export/api_exporter_test.dart
@@ -293,17 +293,19 @@ Future<void> _testExportedApiSynchronization(
     // recently created files as a guard against race conditions.
     fakeTime.elapseSync(days: 1);
 
-    final adminApi = createPubApiClient(
-      authToken: createFakeServiceAccountToken(email: '[email protected]'),
-    );
-    await adminApi.adminInvokeAction(
-      'moderate-package-version',
-      AdminInvokeActionArguments(arguments: {
-        'case': 'none',
-        'package': 'bar',
-        'version': '2.0.0',
-        'state': 'true',
-      }),
+    await withHttpPubApiClient(
+      bearerToken: createFakeServiceAccountToken(email: '[email protected]'),
+      fn: (adminApi) async {
+        await adminApi.adminInvokeAction(
+          'moderate-package-version',
+          AdminInvokeActionArguments(arguments: {
+            'case': 'none',
+            'package': 'bar',
+            'version': '2.0.0',
+            'state': 'true',
+          }),
+        );
+      },
     );
 
     // Synchronize again
@@ -330,18 +332,19 @@ Future<void> _testExportedApiSynchronization(
 
   _log.info('## Version reinstated');
   {
-    final adminApi = createPubApiClient(
-      authToken: createFakeServiceAccountToken(email: '[email protected]'),
-    );
-    await adminApi.adminInvokeAction(
-      'moderate-package-version',
-      AdminInvokeActionArguments(arguments: {
-        'case': 'none',
-        'package': 'bar',
-        'version': '2.0.0',
-        'state': 'false',
-      }),
-    );
+    await withHttpPubApiClient(
+        bearerToken: createFakeServiceAccountToken(email: '[email protected]'),
+        fn: (adminApi) async {
+          await adminApi.adminInvokeAction(
+            'moderate-package-version',
+            AdminInvokeActionArguments(arguments: {
+              'case': 'none',
+              'package': 'bar',
+              'version': '2.0.0',
+              'state': 'false',
+            }),
+          );
+        });
 
     // Synchronize again
     await synchronize();
@@ -371,17 +374,18 @@ Future<void> _testExportedApiSynchronization(
     // recently created files as a guard against race conditions.
     fakeTime.elapseSync(days: 1);
 
-    final adminApi = createPubApiClient(
-      authToken: createFakeServiceAccountToken(email: '[email protected]'),
-    );
-    await adminApi.adminInvokeAction(
-      'moderate-package',
-      AdminInvokeActionArguments(arguments: {
-        'case': 'none',
-        'package': 'bar',
-        'state': 'true',
-      }),
-    );
+    await withHttpPubApiClient(
+        bearerToken: createFakeServiceAccountToken(email: '[email protected]'),
+        fn: (adminApi) async {
+          await adminApi.adminInvokeAction(
+            'moderate-package',
+            AdminInvokeActionArguments(arguments: {
+              'case': 'none',
+              'package': 'bar',
+              'state': 'true',
+            }),
+          );
+        });
 
     // Synchronize again
     await synchronize();
@@ -402,17 +406,18 @@ Future<void> _testExportedApiSynchronization(
 
   _log.info('## Package reinstated');
   {
-    final adminApi = createPubApiClient(
-      authToken: createFakeServiceAccountToken(email: '[email protected]'),
-    );
-    await adminApi.adminInvokeAction(
-      'moderate-package',
-      AdminInvokeActionArguments(arguments: {
-        'case': 'none',
-        'package': 'bar',
-        'state': 'false',
-      }),
-    );
+    await withHttpPubApiClient(
+        bearerToken: createFakeServiceAccountToken(email: '[email protected]'),
+        fn: (adminApi) async {
+          await adminApi.adminInvokeAction(
+            'moderate-package',
+            AdminInvokeActionArguments(arguments: {
+              'case': 'none',
+              'package': 'bar',
+              'state': 'false',
+            }),
+          );
+        });
 
     // Synchronize again
     await synchronize();