From e1fa035b698a9648d8f303db9faf65d0013f496b Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Thu, 28 Sep 2017 17:37:34 -0700 Subject: [PATCH] Retry on failed download. (#12293) --- packages/flutter_tools/lib/src/base/net.dart | 57 ++++++--- .../flutter_tools/test/base/net_test.dart | 115 ++++++++++++++++++ packages/flutter_tools/test/cache_test.dart | 1 + packages/flutter_tools/test/src/context.dart | 5 +- 4 files changed, 160 insertions(+), 18 deletions(-) create mode 100644 packages/flutter_tools/test/base/net_test.dart diff --git a/packages/flutter_tools/lib/src/base/net.dart b/packages/flutter_tools/lib/src/base/net.dart index abce28132c5..1d9adfb8c80 100644 --- a/packages/flutter_tools/lib/src/base/net.dart +++ b/packages/flutter_tools/lib/src/base/net.dart @@ -4,39 +4,62 @@ import 'dart:async'; +import '../base/context.dart'; import '../globals.dart'; import 'common.dart'; import 'io.dart'; const int kNetworkProblemExitCode = 50; +typedef HttpClient HttpClientFactory(); + /// Download a file from the given URL and return the bytes. Future> fetchUrl(Uri url) async { - printTrace('Downloading $url.'); + int attempts = 0; + int duration = 1; + while (true) { + attempts += 1; + final List result = await _attempt(url); + if (result != null) + return result; + printStatus('Download failed -- attempting retry $attempts in $duration second${ duration == 1 ? "" : "s"}...'); + await new Future.delayed(new Duration(seconds: duration)); + if (duration < 64) + duration *= 2; + } +} - final HttpClient httpClient = new HttpClient(); +Future> _attempt(Uri url) async { + printTrace('Downloading: $url'); + HttpClient httpClient; + if (context[HttpClientFactory] != null) { + httpClient = context[HttpClientFactory](); + } else { + httpClient = new HttpClient(); + } final HttpClientRequest request = await httpClient.getUrl(url); final HttpClientResponse response = await request.close(); - - printTrace('Received response statusCode=${response.statusCode}'); if (response.statusCode != 200) { - throwToolExit( - 'Download failed: $url\n' - ' because (${response.statusCode}) ${response.reasonPhrase}', - exitCode: kNetworkProblemExitCode, - ); + if (response.statusCode > 0 && response.statusCode < 500) { + throwToolExit( + 'Download failed.\n' + 'URL: $url\n' + 'Error: ${response.statusCode} ${response.reasonPhrase}', + exitCode: kNetworkProblemExitCode, + ); + } + // 5xx errors are server errors and we can try again + printTrace('Download error: ${response.statusCode} ${response.reasonPhrase}'); + return null; } - + printTrace('Received response from server, collecting bytes...'); try { final BytesBuilder responseBody = new BytesBuilder(copy: false); await for (List chunk in response) - responseBody.add(chunk); - + responseBody.add(chunk); return responseBody.takeBytes(); - } on IOException catch (e) { - throw new ToolExit( - 'Download failed: $url\n $e', - exitCode: kNetworkProblemExitCode, - ); + } on IOException catch (error) { + printTrace('Download error: $error'); + return null; } } diff --git a/packages/flutter_tools/test/base/net_test.dart b/packages/flutter_tools/test/base/net_test.dart new file mode 100644 index 00000000000..384075055f0 --- /dev/null +++ b/packages/flutter_tools/test/base/net_test.dart @@ -0,0 +1,115 @@ +// Copyright 2017 The Chromium Authors. 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 'package:flutter_tools/src/base/io.dart' as io; +import 'package:flutter_tools/src/base/net.dart'; +import 'package:quiver/testing/async.dart'; +import 'package:test/test.dart'; + +import '../src/context.dart'; + +void main() { + testUsingContext('retry from 500', () async { + String error; + new FakeAsync().run((FakeAsync time) { + fetchUrl(Uri.parse('http://example.invalid/')).then((List value) { + error = 'test completed unexpectedly'; + }, onError: (dynamic error) { + error = 'test failed unexpectedly'; + }); + 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); + }, overrides: { + HttpClientFactory: () => () => new MockHttpClient(500), + }); + + testUsingContext('retry from network error', () async { + String error; + new FakeAsync().run((FakeAsync time) { + fetchUrl(Uri.parse('http://example.invalid/')).then((List value) { + error = 'test completed unexpectedly'; + }, onError: (dynamic error) { + error = 'test failed unexpectedly'; + }); + 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); + }, overrides: { + HttpClientFactory: () => () => new MockHttpClient(200), + }); +} + +class MockHttpClient implements io.HttpClient { + MockHttpClient(this.statusCode); + + final int statusCode; + + @override + Future getUrl(Uri url) async { + return new MockHttpClientRequest(statusCode); + } + + @override + dynamic noSuchMethod(Invocation invocation) { + throw 'io.HttpClient - $invocation'; + } +} + +class MockHttpClientRequest implements io.HttpClientRequest { + MockHttpClientRequest(this.statusCode); + + final int statusCode; + + @override + Future close() async { + return new MockHttpClientResponse(statusCode); + } + + @override + dynamic noSuchMethod(Invocation invocation) { + throw 'io.HttpClientRequest - $invocation'; + } +} + +class MockHttpClientResponse implements io.HttpClientResponse { + MockHttpClientResponse(this.statusCode); + + @override + final int statusCode; + + @override + String get reasonPhrase => ''; + + @override + StreamSubscription> listen(void onData(List event), { + Function onError, void onDone(), bool cancelOnError + }) { + return new Stream>.fromFuture(new Future>.error(const io.SocketException('test'))) + .listen(onData, onError: onError, onDone: onDone, cancelOnError: cancelOnError); + } + + @override + dynamic noSuchMethod(Invocation invocation) { + throw 'io.HttpClientResponse - $invocation'; + } +} diff --git a/packages/flutter_tools/test/cache_test.dart b/packages/flutter_tools/test/cache_test.dart index b921260f232..a4ef9410374 100644 --- a/packages/flutter_tools/test/cache_test.dart +++ b/packages/flutter_tools/test/cache_test.dart @@ -48,6 +48,7 @@ void main() { Platform: () => new FakePlatform()..environment = {'FLUTTER_ALREADY_LOCKED': 'true'}, }); }); + group('Cache', () { test('should not be up to date, if some cached artifact is not', () { final CachedArtifact artifact1 = new MockCachedArtifact(); diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart index 6d89f620913..8913488b1e3 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/packages/flutter_tools/test/src/context.dart @@ -60,7 +60,8 @@ void _defaultInitializeContext(AppContext testContext) { ..putIfAbsent(SimControl, () => new MockSimControl()) ..putIfAbsent(Usage, () => new MockUsage()) ..putIfAbsent(FlutterVersion, () => new MockFlutterVersion()) - ..putIfAbsent(Clock, () => const Clock()); + ..putIfAbsent(Clock, () => const Clock()) + ..putIfAbsent(HttpClient, () => new MockHttpClient()); } void testUsingContext(String description, dynamic testMethod(), { @@ -244,3 +245,5 @@ class MockUsage implements Usage { class MockFlutterVersion extends Mock implements FlutterVersion {} class MockClock extends Mock implements Clock {} + +class MockHttpClient extends Mock implements HttpClient {}