Refactor HTTP retries (#3325)

lib/src/authentication/client.dart

@@ -48,19 +48,14 @@ class _AuthenticatedClient extends http.BaseClient {
           await _credential!.getAuthorizationHeaderValue();
     }
 
-    try {
-      final response = await _inner.send(request);
-      if (response.statusCode == 401) {
-        _detectInvalidCredentials = true;
-        _throwAuthException(response);
-      }
-      return response;
-    } on PubHttpException catch (e) {
-      if (e.response.statusCode == 403) {
-        _throwAuthException(e.response);
-      }
-      rethrow;
+    final response = await _inner.send(request);
+    if (response.statusCode == 401) {
+      _detectInvalidCredentials = true;
+    }
+    if (response.statusCode == 401 || response.statusCode == 403) {
+      _throwAuthException(response);
     }
+    return response;
   }
 
   /// Throws [AuthenticationException] that includes response status code and
@@ -127,7 +122,7 @@ Future<T> withAuthenticatedClient<T>(
   Future<T> Function(http.Client) fn,
 ) async {
   final credential = systemCache.tokenStore.findCredential(hostedUrl);
-  final client = _AuthenticatedClient(httpClient, credential);
+  final client = _AuthenticatedClient(globalHttpClient, credential);
 
   try {
     return await fn(client);

lib/src/command.dart

@@ -238,7 +238,7 @@ and attaching the relevant parts of that log file.
           log.message('Logs written to $transcriptPath.');
         }
       }
-      httpClient.close();
+      globalHttpClient.close();
     }
   }
 
@@ -253,7 +253,9 @@ and attaching the relevant parts of that log file.
       exception = exception.innerError!;
     }
 
-    if (exception is HttpException ||
+    if (exception is PackageIntegrityException) {
+      return exit_codes.TEMP_FAIL;
+    } else if (exception is HttpException ||
         exception is http.ClientException ||
         exception is SocketException ||
         exception is TlsException ||

lib/src/command/lish.dart

@@ -93,34 +93,51 @@ class LishCommand extends PubCommand {
 
     try {
       await log.progress('Uploading', () async {
-        var newUri = host.resolve('api/packages/versions/new');
-        var response = await client.get(newUri, headers: pubApiHeaders);
-        var parameters = parseJsonResponse(response);
+        /// 1. Initiate upload
+        final parametersResponse =
+            await retryForHttp('initiating upload', () async {
+          final request =
+              http.Request('GET', host.resolve('api/packages/versions/new'));
+          request.attachPubApiHeaders();
+          request.attachMetadataHeaders();
+          return await client.fetch(request);
+        });
+        final parameters = parseJsonResponse(parametersResponse);
 
-        var url = _expectField(parameters, 'url', response);
-        if (url is! String) invalidServerResponse(response);
+        /// 2. Upload package
+        var url = _expectField(parameters, 'url', parametersResponse);
+        if (url is! String) invalidServerResponse(parametersResponse);
         cloudStorageUrl = Uri.parse(url);
-        // TODO(nweiz): Cloud Storage can provide an XML-formatted error. We
-        // should report that error and exit.
-        var request = http.MultipartRequest('POST', cloudStorageUrl!);
-
-        var fields = _expectField(parameters, 'fields', response);
-        if (fields is! Map) invalidServerResponse(response);
-        fields.forEach((key, value) {
-          if (value is! String) invalidServerResponse(response);
-          request.fields[key] = value;
+        final uploadResponse =
+            await retryForHttp('uploading package', () async {
+          // TODO(nweiz): Cloud Storage can provide an XML-formatted error. We
+          // should report that error and exit.
+          var request = http.MultipartRequest('POST', cloudStorageUrl!);
+
+          var fields = _expectField(parameters, 'fields', parametersResponse);
+          if (fields is! Map) invalidServerResponse(parametersResponse);
+          fields.forEach((key, value) {
+            if (value is! String) invalidServerResponse(parametersResponse);
+            request.fields[key] = value;
+          });
+
+          request.followRedirects = false;
+          request.files.add(http.MultipartFile.fromBytes('file', packageBytes,
+              filename: 'package.tar.gz'));
+          return await client.fetch(request);
         });
 
-        request.followRedirects = false;
-        request.files.add(http.MultipartFile.fromBytes('file', packageBytes,
-            filename: 'package.tar.gz'));
-        var postResponse =
-            await http.Response.fromStream(await client.send(request));
-
-        var location = postResponse.headers['location'];
-        if (location == null) throw PubHttpException(postResponse);
-        handleJsonSuccess(
-            await client.get(Uri.parse(location), headers: pubApiHeaders));
+        /// 3. Finalize publish
+        var location = uploadResponse.headers['location'];
+        if (location == null) throw PubHttpResponseException(uploadResponse);
+        final finalizeResponse =
+            await retryForHttp('finalizing publish', () async {
+          final request = http.Request('GET', Uri.parse(location));
+          request.attachPubApiHeaders();
+          request.attachMetadataHeaders();
+          return await client.fetch(request);
+        });
+        handleJsonSuccess(finalizeResponse);
       });
     } on AuthenticationException catch (error) {
       var msg = '';
@@ -138,7 +155,7 @@ class LishCommand extends PubCommand {
         msg += '\n${error.serverMessage!}\n';
       }
       dataError(msg + log.red('Authentication failed!'));
-    } on PubHttpException catch (error) {
+    } on PubHttpResponseException catch (error) {
       var url = error.response.request!.url;
       if (url == cloudStorageUrl) {
         // TODO(nweiz): the response may have XML-formatted information about
@@ -189,7 +206,7 @@ class LishCommand extends PubCommand {
           return _publishUsingClient(packageBytes, client);
         });
       }
-    } on PubHttpException catch (error) {
+    } on PubHttpResponseException catch (error) {
       var url = error.response.request!.url;
       if (Uri.parse(url.origin) == Uri.parse(host.origin)) {
         handleJsonError(error.response);

lib/src/command/login.dart

@@ -7,7 +7,6 @@ import 'dart:convert';
 
 import '../command.dart';
 import '../command_runner.dart';
-import '../http.dart';
 import '../log.dart' as log;
 import '../oauth2.dart' as oauth2;
 
@@ -46,9 +45,8 @@ class LoginCommand extends PubCommand {
 
   Future<_UserInfo?> _retrieveUserInfo() async {
     return await oauth2.withClient(cache, (client) async {
-      final discovery = await httpClient.get(Uri.https(
-          'accounts.google.com', '/.well-known/openid-configuration'));
-      final userInfoEndpoint = json.decode(discovery.body)['userinfo_endpoint'];
+      final discovery = await oauth2.fetchOidcDiscoveryDocument();
+      final userInfoEndpoint = discovery['userinfo_endpoint'];
       final userInfoRequest = await client.get(Uri.parse(userInfoEndpoint));
       if (userInfoRequest.statusCode != 200) return null;
       try {

lib/src/exceptions.dart

@@ -12,6 +12,7 @@ import 'package:stack_trace/stack_trace.dart';
 import 'package:yaml/yaml.dart';
 
 import 'dart.dart';
+import 'http.dart';
 
 /// An exception class for exceptions that are intended to be seen by the user.
 ///
@@ -106,12 +107,9 @@ class PackageNotFoundException extends WrappedException {
 }
 
 /// A class for exceptions where a package's checksum could not be validated.
-class PackageIntegrityException extends WrappedException {
-  PackageIntegrityException(
-    String message, {
-    Object? innerError,
-    StackTrace? innerTrace,
-  }) : super(message, innerError, innerTrace);
+class PackageIntegrityException extends PubHttpException {
+  PackageIntegrityException(String message)
+      : super(message, isIntermittent: true);
 }
 
 /// Returns whether [error] is a user-facing error object.