[flutter_tool] Move http request close under try-catch (#38894)

packages/flutter_tools/lib/src/base/net.dart

@@ -48,12 +48,14 @@ Future<List<int>> _attempt(Uri url, { bool onlyHeaders = false }) async {
     httpClient = HttpClient();
   }
   HttpClientRequest request;
+  HttpClientResponse response;
   try {
     if (onlyHeaders) {
       request = await httpClient.headUrl(url);
     } else {
       request = await httpClient.getUrl(url);
     }
+    response = await request.close();
   } on ArgumentError catch (error) {
     final String overrideUrl = platform.environment['FLUTTER_STORAGE_BASE_URL'];
     if (overrideUrl != null && url.toString().contains(overrideUrl)) {
@@ -82,7 +84,8 @@ Future<List<int>> _attempt(Uri url, { bool onlyHeaders = false }) async {
     printTrace('Download error: $error');
     return null;
   }
-  final HttpClientResponse response = await request.close();
+  assert(response != null);
+
   // If we're making a HEAD request, we're only checking to see if the URL is
   // valid.
   if (onlyHeaders) {

packages/flutter_tools/test/general.shard/base/net_test.dart

@@ -34,7 +34,7 @@ void main() {
     expect(testLogger.errorText, isEmpty);
     expect(error, isNull);
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClient(500),
+    HttpClientFactory: () => () => FakeHttpClient(500),
   });
 
   testUsingContext('retry from network error', () async {
@@ -57,7 +57,7 @@ void main() {
     expect(testLogger.errorText, isEmpty);
     expect(error, isNull);
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClient(200),
+    HttpClientFactory: () => () => FakeHttpClient(200),
   });
 
   testUsingContext('retry from SocketException', () async {
@@ -81,7 +81,7 @@ void main() {
     expect(error, isNull);
     expect(testLogger.traceText, contains('Download error: SocketException'));
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClientThrowing(
+    HttpClientFactory: () => () => FakeHttpClientThrowing(
       const io.SocketException('test exception handling'),
     ),
   });
@@ -101,7 +101,7 @@ void main() {
     expect(error, startsWith('test failed'));
     expect(testLogger.traceText, contains('HandshakeException'));
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClientThrowing(
+    HttpClientFactory: () => () => FakeHttpClientThrowing(
       const io.HandshakeException('test exception handling'),
     ),
   });
@@ -122,7 +122,7 @@ void main() {
     expect(testLogger.errorText, contains('Invalid argument'));
     expect(error, contains('FLUTTER_STORAGE_BASE_URL'));
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClientThrowing(
+    HttpClientFactory: () => () => FakeHttpClientThrowing(
       ArgumentError('test exception handling'),
     ),
     Platform: () => FakePlatform.fromPlatform(const LocalPlatform())
@@ -152,7 +152,33 @@ void main() {
     expect(error, isNull);
     expect(testLogger.traceText, contains('Download error: HttpException'));
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClientThrowing(
+    HttpClientFactory: () => () => FakeHttpClientThrowing(
+      const io.HttpException('test exception handling'),
+    ),
+  });
+
+  testUsingContext('retry from HttpException when request throws', () async {
+    String error;
+    FakeAsync().run((FakeAsync time) {
+      fetchUrl(Uri.parse('http://example.invalid/')).then((List<int> value) {
+        error = 'test completed unexpectedly';
+      }, onError: (dynamic exception) {
+        error = 'test failed unexpectedly: $exception';
+      });
+      expect(testLogger.statusText, '');
+      time.elapse(const Duration(milliseconds: 10000));
+      expect(testLogger.statusText,
+        'Download failed -- attempting retry 1 in 1 second...\n'
+        'Download failed -- attempting retry 2 in 2 seconds...\n'
+        'Download failed -- attempting retry 3 in 4 seconds...\n'
+        'Download failed -- attempting retry 4 in 8 seconds...\n',
+      );
+    });
+    expect(testLogger.errorText, isEmpty);
+    expect(error, isNull);
+    expect(testLogger.traceText, contains('Download error: HttpException'));
+  }, overrides: <Type, Generator>{
+    HttpClientFactory: () => () => FakeHttpClientThrowingRequest(
       const io.HttpException('test exception handling'),
     ),
   });
@@ -178,7 +204,7 @@ void main() {
     expect(error, isNull);
     expect(actualResult, isNull);
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClient(500),
+    HttpClientFactory: () => () => FakeHttpClient(500),
   });
 
   testUsingContext('remote file non-existant', () async {
@@ -186,7 +212,7 @@ void main() {
     final bool result = await doesRemoteFileExist(invalid);
     expect(result, false);
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClient(404),
+    HttpClientFactory: () => () => FakeHttpClient(404),
   });
 
   testUsingContext('remote file server error', () async {
@@ -194,7 +220,7 @@ void main() {
     final bool result = await doesRemoteFileExist(valid);
     expect(result, false);
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClient(500),
+    HttpClientFactory: () => () => FakeHttpClient(500),
   });
 
   testUsingContext('remote file exists', () async {
@@ -202,12 +228,12 @@ void main() {
     final bool result = await doesRemoteFileExist(valid);
     expect(result, true);
   }, overrides: <Type, Generator>{
-    HttpClientFactory: () => () => MockHttpClient(200),
+    HttpClientFactory: () => () => FakeHttpClient(200),
   });
 }
 
-class MockHttpClientThrowing implements io.HttpClient {
-  MockHttpClientThrowing(this.exception);
+class FakeHttpClientThrowing implements io.HttpClient {
+  FakeHttpClientThrowing(this.exception);
 
   final Object exception;
 
@@ -222,19 +248,19 @@ class MockHttpClientThrowing implements io.HttpClient {
   }
 }
 
-class MockHttpClient implements io.HttpClient {
-  MockHttpClient(this.statusCode);
+class FakeHttpClient implements io.HttpClient {
+  FakeHttpClient(this.statusCode);
 
   final int statusCode;
 
   @override
   Future<io.HttpClientRequest> getUrl(Uri url) async {
-    return MockHttpClientRequest(statusCode);
+    return FakeHttpClientRequest(statusCode);
   }
 
   @override
   Future<io.HttpClientRequest> headUrl(Uri url) async {
-    return MockHttpClientRequest(statusCode);
+    return FakeHttpClientRequest(statusCode);
   }
 
   @override
@@ -243,14 +269,46 @@ class MockHttpClient implements io.HttpClient {
   }
 }
 
-class MockHttpClientRequest implements io.HttpClientRequest {
-  MockHttpClientRequest(this.statusCode);
+class FakeHttpClientThrowingRequest implements io.HttpClient {
+  FakeHttpClientThrowingRequest(this.exception);
+
+  final Object exception;
+
+  @override
+  Future<io.HttpClientRequest> getUrl(Uri url) async {
+    return FakeHttpClientRequestThrowing(exception);
+  }
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) {
+    throw 'io.HttpClient - $invocation';
+  }
+}
+
+class FakeHttpClientRequest implements io.HttpClientRequest {
+  FakeHttpClientRequest(this.statusCode);
 
   final int statusCode;
 
   @override
   Future<io.HttpClientResponse> close() async {
-    return MockHttpClientResponse(statusCode);
+    return FakeHttpClientResponse(statusCode);
+  }
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) {
+    throw 'io.HttpClientRequest - $invocation';
+  }
+}
+
+class FakeHttpClientRequestThrowing implements io.HttpClientRequest {
+  FakeHttpClientRequestThrowing(this.exception);
+
+  final Object exception;
+
+  @override
+  Future<io.HttpClientResponse> close() async {
+    throw exception;
   }
 
   @override
@@ -259,8 +317,8 @@ class MockHttpClientRequest implements io.HttpClientRequest {
   }
 }
 
-class MockHttpClientResponse implements io.HttpClientResponse {
-  MockHttpClientResponse(this.statusCode);
+class FakeHttpClientResponse implements io.HttpClientResponse {
+  FakeHttpClientResponse(this.statusCode);
 
   @override
   final int statusCode;