Unverified Commit 89ef88c6 authored by Ben Konyi's avatar Ben Konyi Committed by GitHub

Ensure attaching to an application with an existing DDS instance is not...

Ensure attaching to an application with an existing DDS instance is not treated as a fatal error (#70847)
parent e9abf5a9
......@@ -8,7 +8,7 @@ environment:
dependencies:
flutter:
sdk: flutter
camera: 0.5.8+11
camera: 0.5.8+17
characters: 1.1.0-nullsafety.5 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
collection: 1.15.0-nullsafety.5 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
......@@ -19,4 +19,4 @@ dependencies:
flutter:
uses-material-design: true
# PUBSPEC CHECKSUM: aa03
# PUBSPEC CHECKSUM: 8f09
......@@ -20,7 +20,7 @@ dependencies:
analyzer: 0.40.6 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
async: 2.5.0-nullsafety.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
boolean_selector: 2.1.0-nullsafety.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
browser_launcher: 0.1.7 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
browser_launcher: 0.1.8 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
built_collection: 4.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
built_value: 7.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
charcode: 1.2.0-nullsafety.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
......@@ -97,4 +97,4 @@ dev_dependencies:
node_preamble: 1.4.12 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
# PUBSPEC CHECKSUM: bb6f
# PUBSPEC CHECKSUM: 7770
......@@ -19,7 +19,8 @@ class DartDevelopmentService {
final Logger logger;
dds.DartDevelopmentService _ddsInstance;
Uri get uri => _ddsInstance.uri;
Uri get uri => _ddsInstance?.uri ?? _existingDdsUri;
Uri _existingDdsUri;
Future<void> get done => _completer.future;
final Completer<void> _completer = Completer<void>();
......@@ -57,6 +58,11 @@ class DartDevelopmentService {
logger.printTrace('DDS is listening at ${_ddsInstance.uri}.');
} on dds.DartDevelopmentServiceException catch (e) {
logger.printTrace('Warning: Failed to start DDS: ${e.message}');
if (e.errorCode == dds.DartDevelopmentServiceException.existingDdsInstanceError) {
_existingDdsUri = Uri.parse(
e.message.split(' ').firstWhere((String e) => e.startsWith('http'))
);
}
if (!_completer.isCompleted) {
_completer.complete();
}
......
......@@ -318,7 +318,14 @@ known, it can be explicitly provided to attach via the command-line, e.g.
try {
app = await daemon.appDomain.launch(
runner,
runner.attach,
({Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<void> appStartedCompleter}) {
return runner.attach(
connectionInfoCompleter: connectionInfoCompleter,
appStartedCompleter: appStartedCompleter,
allowExistingDdsInstance: true,
);
},
device,
null,
true,
......@@ -354,6 +361,7 @@ known, it can be explicitly provided to attach via the command-line, e.g.
}));
result = await runner.attach(
appStartedCompleter: onAppStart,
allowExistingDdsInstance: true,
);
if (result != 0) {
throwToolExit(null, exitCode: result);
......
......@@ -748,6 +748,7 @@ class _ResidentWebRunner extends ResidentWebRunner {
Future<int> attach({
Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<void> appStartedCompleter,
bool allowExistingDdsInstance = false,
}) async {
if (_chromiumLauncher != null) {
final Chromium chrome = await _chromiumLauncher.connectedInstance;
......
......@@ -4,6 +4,7 @@
import 'dart:async';
import 'package:dds/dds.dart' as dds;
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:vm_service/vm_service.dart' as vm_service;
......@@ -221,6 +222,7 @@ class FlutterDevice {
int ddsPort,
bool disableServiceAuthCodes = false,
bool disableDds = false,
bool allowExistingDdsInstance = false,
bool ipv6 = false,
}) {
final Completer<void> completer = Completer<void>();
......@@ -231,8 +233,15 @@ class FlutterDevice {
// FYI, this message is used as a sentinel in tests.
globals.printTrace('Connecting to service protocol: $observatoryUri');
isWaitingForVm = true;
bool existingDds = false;
vm_service.VmService service;
if (!disableDds) {
void handleError(Exception e) {
globals.printTrace('Fail to connect to service protocol: $observatoryUri: $e');
if (!completer.isCompleted) {
completer.completeError('failed to connect to $observatoryUri');
}
}
// This first try block is meant to catch errors that occur during DDS startup
// (e.g., failure to bind to a port, failure to connect to the VM service,
// attaching to a VM service with existing clients, etc.).
......@@ -243,11 +252,16 @@ class FlutterDevice {
ipv6,
disableServiceAuthCodes,
);
} on Exception catch (e) {
globals.printTrace('Fail to connect to service protocol: $observatoryUri: $e');
if (!completer.isCompleted && !_isListeningForObservatoryUri) {
completer.completeError('failed to connect to $observatoryUri');
} on dds.DartDevelopmentServiceException catch (e) {
if (!allowExistingDdsInstance ||
(e.errorCode != dds.DartDevelopmentServiceException.existingDdsInstanceError)) {
handleError(e);
return;
} else {
existingDds = true;
}
} on Exception catch (e) {
handleError(e);
return;
}
}
......@@ -267,6 +281,7 @@ class FlutterDevice {
printStructuredErrorLogMethod: printStructuredErrorLogMethod,
device: device,
),
if (!existingDds)
device.dds.done.whenComplete(() => throw Exception('DDS shut down too early')),
]
) as vm_service.VmService;
......@@ -849,6 +864,7 @@ abstract class ResidentRunner {
Future<int> attach({
Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<void> appStartedCompleter,
bool allowExistingDdsInstance = false,
});
bool get supportsRestart => false;
......@@ -1201,6 +1217,7 @@ abstract class ResidentRunner {
Restart restart,
CompileExpression compileExpression,
GetSkSLMethod getSkSLMethod,
bool allowExistingDdsInstance,
}) async {
if (!debuggingOptions.debuggingEnabled) {
throw 'The service protocol is not enabled.';
......@@ -1214,6 +1231,7 @@ abstract class ResidentRunner {
compileExpression: compileExpression,
disableDds: debuggingOptions.disableDds,
ddsPort: debuggingOptions.ddsPort,
allowExistingDdsInstance: allowExistingDdsInstance,
hostVmServicePort: debuggingOptions.hostVmServicePort,
getSkSLMethod: getSkSLMethod,
printStructuredErrorLogMethod: printStructuredErrorLog,
......
......@@ -128,11 +128,13 @@ class ColdRunner extends ResidentRunner {
Future<int> attach({
Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<void> appStartedCompleter,
bool allowExistingDdsInstance = false,
}) async {
_didAttach = true;
try {
await connectToServiceProtocol(
getSkSLMethod: writeSkSL,
allowExistingDdsInstance: allowExistingDdsInstance,
);
} on Exception catch (error) {
globals.printError('Error connecting to the service protocol: $error');
......
......@@ -171,6 +171,7 @@ class HotRunner extends ResidentRunner {
Future<int> attach({
Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<void> appStartedCompleter,
bool allowExistingDdsInstance = false,
}) async {
_didAttach = true;
try {
......@@ -179,6 +180,7 @@ class HotRunner extends ResidentRunner {
restart: _restartService,
compileExpression: _compileExpressionService,
getSkSLMethod: writeSkSL,
allowExistingDdsInstance: allowExistingDdsInstance,
);
// Catches all exceptions, non-Exception objects are rethrown.
} catch (error) { // ignore: avoid_catches_without_on_clauses
......
......@@ -62,7 +62,7 @@ dependencies:
_fe_analyzer_shared: 12.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
analyzer: 0.40.6 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
boolean_selector: 2.1.0-nullsafety.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
browser_launcher: 0.1.7 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
browser_launcher: 0.1.8 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
built_collection: 4.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
built_value: 7.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
charcode: 1.2.0-nullsafety.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
......@@ -111,4 +111,4 @@ dartdoc:
# Exclude this package from the hosted API docs.
nodoc: true
# PUBSPEC CHECKSUM: 2f49
# PUBSPEC CHECKSUM: 8b4a
......@@ -179,7 +179,7 @@ void main() {
const String outputDill = '/tmp/output.dill';
final MockHotRunner mockHotRunner = MockHotRunner();
when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter')))
when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter'), allowExistingDdsInstance: true))
.thenAnswer((_) async => 0);
when(mockHotRunner.exited).thenReturn(false);
when(mockHotRunner.isWaitingForObservatory).thenReturn(false);
......@@ -311,7 +311,7 @@ void main() {
.thenReturn(<ForwardedPort>[ForwardedPort(hostPort, devicePort)]);
when(portForwarder.unforward(any))
.thenAnswer((_) async {});
when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter')))
when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter'), allowExistingDdsInstance: true))
.thenAnswer((_) async => 0);
when(mockHotRunnerFactory.build(
any,
......@@ -395,7 +395,7 @@ void main() {
.thenReturn(<ForwardedPort>[ForwardedPort(hostPort, devicePort)]);
when(portForwarder.unforward(any))
.thenAnswer((_) async {});
when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter')))
when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter'), allowExistingDdsInstance: true))
.thenAnswer((_) async => 0);
when(mockHotRunnerFactory.build(
any,
......
......@@ -129,6 +129,7 @@ class TestFlutterDevice extends FlutterDevice {
int hostVmServicePort,
int ddsPort,
bool ipv6 = false,
bool allowExistingDdsInstance = false,
}) async {
throw exception;
}
......
......@@ -598,6 +598,7 @@ class TestFlutterDevice extends FlutterDevice {
bool ipv6 = false,
int hostVmServicePort,
int ddsPort,
bool allowExistingDdsInstance = false,
}) async {
throw exception;
}
......
......@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/features.dart';
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:vm_service/vm_service.dart' as vm_service;
import 'package:dds/dds.dart' as dds;
import 'package:file/memory.dart';
import 'package:file_testing/file_testing.dart';
import 'package:flutter_tools/src/artifacts.dart';
......@@ -2673,6 +2674,36 @@ void main() {
}) async => mockVMService,
}));
testUsingContext('FlutterDevice handles existing DDS instance', () => testbed.run(() async {
fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[]);
final MockDevice mockDevice = MockDevice();
final MockDartDevelopmentService mockDds = MockDartDevelopmentService();
final MockDeviceLogReader mockLogReader = MockDeviceLogReader();
final Completer<void> noopCompleter = Completer<void>();
when(mockDevice.getLogReader(app: anyNamed('app'))).thenReturn(mockLogReader);
when(mockDevice.dds).thenReturn(mockDds);
when(mockDds.startDartDevelopmentService(any, any, any, any)).thenThrow(FakeDartDevelopmentServiceException());
when(mockDds.done).thenAnswer((_) => noopCompleter.future);
final TestFlutterDevice flutterDevice = TestFlutterDevice(
mockDevice,
observatoryUris: Stream<Uri>.value(testUri),
);
await flutterDevice.connect(allowExistingDdsInstance: true);
verify(mockLogReader.connectedVMService = mockVMService);
}, overrides: <Type, Generator>{
VMServiceConnector: () => (Uri httpUri, {
ReloadSources reloadSources,
Restart restart,
CompileExpression compileExpression,
GetSkSLMethod getSkSLMethod,
PrintStructuredErrorLogMethod printStructuredErrorLogMethod,
io.CompressionOptions compression,
Device device,
}) async => mockVMService,
}));
testUsingContext('nextPlatform moves through expected platforms', () {
expect(nextPlatform('android', TestFeatureFlags()), 'iOS');
expect(nextPlatform('iOS', TestFeatureFlags()), 'fuchsia');
......@@ -2694,6 +2725,14 @@ class MockUsage extends Mock implements Usage {}
class MockProcessManager extends Mock implements ProcessManager {}
class MockResidentCompiler extends Mock implements ResidentCompiler {}
class FakeDartDevelopmentServiceException implements dds.DartDevelopmentServiceException {
@override
final int errorCode = dds.DartDevelopmentServiceException.existingDdsInstanceError;
@override
final String message = 'A DDS instance is already connected at http://localhost:8181';
}
class TestFlutterDevice extends FlutterDevice {
TestFlutterDevice(Device device, { Stream<Uri> observatoryUris })
: super(device, buildInfo: BuildInfo.debug) {
......@@ -2737,6 +2776,7 @@ class FakeFlutterDevice extends FlutterDevice {
int hostVmServicePort,
int ddsPort,
PrintStructuredErrorLogMethod printStructuredErrorLogMethod,
bool allowExistingDdsInstance = false,
}) async { }
......
......@@ -353,5 +353,6 @@ class TestRunner extends Mock implements ResidentRunner {
Future<int> attach({
Completer<DebugConnectionInfo> connectionInfoCompleter,
Completer<void> appStartedCompleter,
bool allowExistingDdsInstance = false,
}) async => null;
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment