diff --git a/app/lib/tool/utils/pub_api_client.dart b/app/lib/tool/utils/pub_api_client.dart index 35b82ec584..ff98f5de70 100644 --- 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 withHttpPubApiClient({ 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 index 0d68a5f87a..158cec1e41 100644 --- 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? 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 index 850a178871..885ee69be1 100644 --- 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 _testExportedApiSynchronization( // recently created files as a guard against race conditions. fakeTime.elapseSync(days: 1); - final adminApi = createPubApiClient( - authToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), - ); - await adminApi.adminInvokeAction( - 'moderate-package-version', - AdminInvokeActionArguments(arguments: { - 'case': 'none', - 'package': 'bar', - 'version': '2.0.0', - 'state': 'true', - }), + await withHttpPubApiClient( + bearerToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), + 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 _testExportedApiSynchronization( _log.info('## Version reinstated'); { - final adminApi = createPubApiClient( - authToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), - ); - await adminApi.adminInvokeAction( - 'moderate-package-version', - AdminInvokeActionArguments(arguments: { - 'case': 'none', - 'package': 'bar', - 'version': '2.0.0', - 'state': 'false', - }), - ); + await withHttpPubApiClient( + bearerToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), + 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 _testExportedApiSynchronization( // recently created files as a guard against race conditions. fakeTime.elapseSync(days: 1); - final adminApi = createPubApiClient( - authToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), - ); - await adminApi.adminInvokeAction( - 'moderate-package', - AdminInvokeActionArguments(arguments: { - 'case': 'none', - 'package': 'bar', - 'state': 'true', - }), - ); + await withHttpPubApiClient( + bearerToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), + 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 _testExportedApiSynchronization( _log.info('## Package reinstated'); { - final adminApi = createPubApiClient( - authToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), - ); - await adminApi.adminInvokeAction( - 'moderate-package', - AdminInvokeActionArguments(arguments: { - 'case': 'none', - 'package': 'bar', - 'state': 'false', - }), - ); + await withHttpPubApiClient( + bearerToken: createFakeServiceAccountToken(email: 'admin@pub.dev'), + fn: (adminApi) async { + await adminApi.adminInvokeAction( + 'moderate-package', + AdminInvokeActionArguments(arguments: { + 'case': 'none', + 'package': 'bar', + 'state': 'false', + }), + ); + }); // Synchronize again await synchronize();