diff --git a/packages/flutter_tools/lib/src/devtools_launcher.dart b/packages/flutter_tools/lib/src/devtools_launcher.dart index 832b44be78d..4d4328bb055 100644 --- a/packages/flutter_tools/lib/src/devtools_launcher.dart +++ b/packages/flutter_tools/lib/src/devtools_launcher.dart @@ -6,7 +6,6 @@ import 'dart:async'; -import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; import 'package:process/process.dart'; @@ -19,8 +18,8 @@ import 'resident_runner.dart'; /// An implementation of the devtools launcher that uses the server package. /// -/// This is implemented in isolated to prevent the flutter_tool from needing -/// a devtools dep in google3. +/// This is implemented in `isolated/` to prevent the flutter_tool from needing +/// a devtools dependency in google3. class DevtoolsServerLauncher extends DevtoolsLauncher { DevtoolsServerLauncher({ @required Platform platform, @@ -28,22 +27,26 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { @required String pubExecutable, @required Logger logger, @required PersistentToolState persistentToolState, + @visibleForTesting io.HttpClient httpClient, }) : _processManager = processManager, _pubExecutable = pubExecutable, _logger = logger, _platform = platform, - _persistentToolState = persistentToolState; + _persistentToolState = persistentToolState, + _httpClient = httpClient ?? io.HttpClient(); final ProcessManager _processManager; final String _pubExecutable; final Logger _logger; final Platform _platform; final PersistentToolState _persistentToolState; + final io.HttpClient _httpClient; io.Process _devToolsProcess; static final RegExp _serveDevToolsPattern = RegExp(r'Serving DevTools at ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+)'); + static const String _pubHostedUrlKey = 'PUB_HOSTED_URL'; @override Future launch(Uri vmServiceUri) async { @@ -51,15 +54,33 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { // this method is guaranteed not to return a Future that throws. try { bool offline = false; + bool useOverrideUrl = false; try { - const String pubHostedUrlKey = 'PUB_HOSTED_URL'; - if (_platform.environment.containsKey(pubHostedUrlKey)) { - await http.head(Uri.parse(_platform.environment[pubHostedUrlKey])); + Uri uri; + if (_platform.environment.containsKey(_pubHostedUrlKey)) { + useOverrideUrl = true; + uri = Uri.parse(_platform.environment[_pubHostedUrlKey]); } else { - await http.head(Uri.https('pub.dev', '')); + uri = Uri.https('pub.dev', ''); + } + final io.HttpClientRequest request = await _httpClient.headUrl(uri); + final io.HttpClientResponse response = await request.close(); + await response.drain(); + if (response.statusCode != io.HttpStatus.ok) { + offline = true; } } on Exception { offline = true; + } on ArgumentError { + if (!useOverrideUrl) { + rethrow; + } + // The user supplied a custom pub URL that was invalid, pretend to be offline + // and inform them that the URL was invalid. + offline = true; + _logger.printError( + 'PUB_HOSTED_URL was set to an invalid URL: "${_platform.environment[_pubHostedUrlKey]}".' + ); } if (offline) { diff --git a/packages/flutter_tools/test/general.shard/artifact_updater_test.dart b/packages/flutter_tools/test/general.shard/artifact_updater_test.dart index 506a741d0a2..8ed4fac82eb 100644 --- a/packages/flutter_tools/test/general.shard/artifact_updater_test.dart +++ b/packages/flutter_tools/test/general.shard/artifact_updater_test.dart @@ -40,7 +40,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(logger.statusText, contains('test message')); @@ -67,7 +67,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(logger.statusText, contains('test message')); @@ -88,7 +88,7 @@ void main() { operatingSystemUtils: operatingSystemUtils, platform: testPlatform, httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///test.zip'), response: const FakeResponse( + FakeRequest(Uri.parse('http://test.zip'), response: const FakeResponse( headers: >{ 'x-goog-hash': [], } @@ -100,7 +100,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(logger.statusText, contains('test message')); @@ -119,7 +119,7 @@ void main() { operatingSystemUtils: operatingSystemUtils, platform: testPlatform, httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///test.zip'), response: const FakeResponse( + FakeRequest(Uri.parse('http://test.zip'), response: const FakeResponse( body: [0], headers: >{ 'x-goog-hash': [ @@ -135,7 +135,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(logger.statusText, contains('test message')); @@ -154,7 +154,7 @@ void main() { operatingSystemUtils: operatingSystemUtils, platform: testPlatform, httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///test.zip'), response: const FakeResponse( + FakeRequest(Uri.parse('http://test.zip'), response: const FakeResponse( body: [0], headers: >{ 'x-goog-hash': [ @@ -163,7 +163,7 @@ void main() { ], } )), - FakeRequest(Uri.parse('http:///test.zip'), response: const FakeResponse( + FakeRequest(Uri.parse('http://test.zip'), response: const FakeResponse( headers: >{ 'x-goog-hash': [ 'foo-bar-baz', @@ -178,7 +178,7 @@ void main() { await expectLater(() async => await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ), throwsToolExit(message: 'k7iFrf4SQT9WfcQ==')); // validate that the hash mismatch message is included. }); @@ -197,8 +197,8 @@ void main() { operatingSystemUtils: operatingSystemUtils, platform: testPlatform, httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///test.zip'), responseError: const HttpException('')), - FakeRequest(Uri.parse('http:///test.zip')), + FakeRequest(Uri.parse('http://test.zip'), responseError: const HttpException('')), + FakeRequest(Uri.parse('http://test.zip')), ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), @@ -206,7 +206,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); @@ -224,8 +224,8 @@ void main() { operatingSystemUtils: operatingSystemUtils, platform: testPlatform, httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///test.zip'), response: const FakeResponse(statusCode: HttpStatus.preconditionFailed)), - FakeRequest(Uri.parse('http:///test.zip'), response: const FakeResponse(statusCode: HttpStatus.preconditionFailed)), + FakeRequest(Uri.parse('http://test.zip'), response: const FakeResponse(statusCode: HttpStatus.preconditionFailed)), + FakeRequest(Uri.parse('http://test.zip'), response: const FakeResponse(statusCode: HttpStatus.preconditionFailed)), ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), @@ -233,7 +233,7 @@ void main() { await expectLater(() async => await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ), throwsToolExit()); @@ -281,7 +281,7 @@ void main() { operatingSystemUtils: operatingSystemUtils, platform: testPlatform, httpClient: FakeHttpClient.list([ - FakeRequest(Uri.parse('http:///test.zip'), responseError: ArgumentError()), + FakeRequest(Uri.parse('http://test.zip'), responseError: ArgumentError()), ]), tempStorage: fileSystem.currentDirectory.childDirectory('temp') ..createSync(), @@ -289,7 +289,7 @@ void main() { await expectLater(() async => await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ), throwsA(isA())); @@ -314,7 +314,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(logger.statusText, contains('test message')); @@ -338,7 +338,7 @@ void main() { await artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(logger.statusText, contains('test message')); @@ -362,7 +362,7 @@ void main() { expect(artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ), throwsA(isA())); expect(fileSystem.file('te,[/test'), isNot(exists)); @@ -386,7 +386,7 @@ void main() { expect(artifactUpdater.downloadZipArchive( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ), throwsA(isA())); expect(fileSystem.file('te,[/test'), isNot(exists)); @@ -409,7 +409,7 @@ void main() { await artifactUpdater.downloadZippedTarball( 'test message', - Uri.parse('http:///test.zip'), + Uri.parse('http://test.zip'), fileSystem.currentDirectory.childDirectory('out'), ); expect(fileSystem.file('out/test'), exists); diff --git a/packages/flutter_tools/test/general.shard/devfs_test.dart b/packages/flutter_tools/test/general.shard/devfs_test.dart index 7220b4bb0bc..85b6ac82a87 100644 --- a/packages/flutter_tools/test/general.shard/devfs_test.dart +++ b/packages/flutter_tools/test/general.shard/devfs_test.dart @@ -259,6 +259,7 @@ void main() { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost( requests: [createDevFSRequest], ); + setHttpAddress(Uri.parse('http://localhost'), fakeVmServiceHost.vmService); final DevFS devFS = DevFS( fakeVmServiceHost.vmService, diff --git a/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart b/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart index 8190f0c710e..d88adb1487b 100644 --- a/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart +++ b/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart @@ -17,6 +17,7 @@ import 'package:flutter_tools/src/resident_runner.dart'; import '../src/common.dart'; import '../src/context.dart'; +import '../src/fake_http_client.dart'; void main() { BufferLogger logger; @@ -34,6 +35,65 @@ void main() { ); }); + testWithoutContext('DevtoolsLauncher does not launch devtools if unable to reach pub.dev', () async { + final DevtoolsLauncher launcher = DevtoolsServerLauncher( + pubExecutable: 'pub', + logger: logger, + platform: platform, + persistentToolState: persistentToolState, + httpClient: FakeHttpClient.list([ + FakeRequest( + Uri.https('pub.dev', ''), + method: HttpMethod.head, + response: const FakeResponse(statusCode: HttpStatus.internalServerError), + ), + ]), + processManager: FakeProcessManager.list([]), + ); + + final DevToolsServerAddress address = await launcher.serve(); + expect(address, isNull); + }); + + testWithoutContext('DevtoolsLauncher pings PUB_HOSTED_URL instead of pub.dev for online check', () async { + final DevtoolsLauncher launcher = DevtoolsServerLauncher( + pubExecutable: 'pub', + logger: logger, + platform: FakePlatform(environment: { + 'PUB_HOSTED_URL': 'https://pub2.dev' + }), + persistentToolState: persistentToolState, + httpClient: FakeHttpClient.list([ + FakeRequest( + Uri.https('pub2.dev', ''), + method: HttpMethod.head, + response: const FakeResponse(statusCode: HttpStatus.internalServerError), + ), + ]), + processManager: FakeProcessManager.list([]), + ); + + final DevToolsServerAddress address = await launcher.serve(); + expect(address, isNull); + }); + + testWithoutContext('DevtoolsLauncher handles an invalid PUB_HOSTED_URL', () async { + final DevtoolsLauncher launcher = DevtoolsServerLauncher( + pubExecutable: 'pub', + logger: logger, + platform: FakePlatform(environment: { + 'PUB_HOSTED_URL': r'not_an_http_url' + }), + persistentToolState: persistentToolState, + httpClient: FakeHttpClient.list([]), + processManager: FakeProcessManager.list([]), + ); + + final DevToolsServerAddress address = await launcher.serve(); + expect(address, isNull); + expect(logger.errorText, contains('PUB_HOSTED_URL was set to an invalid URL: "not_an_http_url".')); + }); + testWithoutContext('DevtoolsLauncher launches DevTools through pub and saves the URI', () async { final Completer completer = Completer(); final DevtoolsLauncher launcher = DevtoolsServerLauncher( @@ -41,6 +101,7 @@ void main() { logger: logger, platform: platform, persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.list([ const FakeCommand( command: [ @@ -85,6 +146,7 @@ void main() { logger: logger, platform: platform, persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.list([ const FakeCommand( command: [ @@ -129,6 +191,7 @@ void main() { logger: logger, platform: platform, persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.list([ const FakeCommand( command: [ @@ -178,6 +241,7 @@ void main() { logger: logger, platform: platform, persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.list([ const FakeCommand( command: [ @@ -209,6 +273,7 @@ void main() { logger: logger, platform: platform, persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.list([ const FakeCommand( command: [ @@ -253,6 +318,7 @@ void main() { logger: logger, platform: platform, persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.list([ const FakeCommand( command: [ diff --git a/packages/flutter_tools/test/general.shard/testbed_test.dart b/packages/flutter_tools/test/general.shard/testbed_test.dart index 297bd0eae36..e4521e09faa 100644 --- a/packages/flutter_tools/test/general.shard/testbed_test.dart +++ b/packages/flutter_tools/test/general.shard/testbed_test.dart @@ -66,7 +66,7 @@ void main() { final Testbed testbed = Testbed(); await testbed.run(() async { final HttpClient client = HttpClient(); - final HttpClientRequest request = await client.getUrl(null); + final HttpClientRequest request = await client.getUrl(Uri.parse('http://foo.dev')); final HttpClientResponse response = await request.close(); expect(response.statusCode, HttpStatus.ok); diff --git a/packages/flutter_tools/test/src/fake_http_client.dart b/packages/flutter_tools/test/src/fake_http_client.dart index b5c27f915b9..f1c5a4c7a6a 100644 --- a/packages/flutter_tools/test/src/fake_http_client.dart +++ b/packages/flutter_tools/test/src/fake_http_client.dart @@ -263,6 +263,12 @@ class FakeHttpClient implements HttpClient { int _requestCount = 0; _FakeHttpClientRequest _findRequest(HttpMethod method, Uri uri) { + // Ensure the fake client throws similar errors to the real client. + if (uri.host.isEmpty) { + throw ArgumentError('No host specified in URI $uri'); + } else if (uri.scheme != 'http' && uri.scheme != 'https') { + throw ArgumentError("Unsupported scheme '${uri.scheme}' in URI $uri"); + } final String methodString = _toMethodString(method); if (_any) { return _FakeHttpClientRequest(