Unverified Commit 1c2f478e authored by Andrew Davies's avatar Andrew Davies Committed by GitHub

[frdp][connection] Exception checks in _getDartVm (#22386)

Through some integration testing checks, it looks like it's possible
that connecting to a new instance of the Dart VM may fail even after the
Dart VM instance has been discovered.

This means that _getDartVm should be checked in more than just one
place.  This changes the function to return a null value in the event of
a 'well-known' exception (`HttpException` or `TimeoutException`
specifically).

As a result, code now calling _getDartVm checks for null and handles it
as appropriate, and the DartVm RPC calls are all updated with
consistent timeout params.
parent fd8a9603
...@@ -120,9 +120,12 @@ class DartVm { ...@@ -120,9 +120,12 @@ class DartVm {
/// ///
/// This is not limited to Isolates running Flutter, but to any Isolate on the /// This is not limited to Isolates running Flutter, but to any Isolate on the
/// VM. /// VM.
Future<List<IsolateRef>> getMainIsolatesByPattern(Pattern pattern) async { Future<List<IsolateRef>> getMainIsolatesByPattern(
Pattern pattern, {
Duration timeout = _kRpcTimeout,
}) async {
final Map<String, dynamic> jsonVmRef = final Map<String, dynamic> jsonVmRef =
await invokeRpc('getVM', timeout: _kRpcTimeout); await invokeRpc('getVM', timeout: timeout);
final List<IsolateRef> result = <IsolateRef>[]; final List<IsolateRef> result = <IsolateRef>[];
for (Map<String, dynamic> jsonIsolate in jsonVmRef['isolates']) { for (Map<String, dynamic> jsonIsolate in jsonVmRef['isolates']) {
final String name = jsonIsolate['name']; final String name = jsonIsolate['name'];
...@@ -160,10 +163,12 @@ class DartVm { ...@@ -160,10 +163,12 @@ class DartVm {
/// the flutter view's name), then the flutter view's ID will be added /// the flutter view's name), then the flutter view's ID will be added
/// instead. If none of these things can be found (isolate has no name or the /// instead. If none of these things can be found (isolate has no name or the
/// flutter view has no ID), then the result will not be added to the list. /// flutter view has no ID), then the result will not be added to the list.
Future<List<FlutterView>> getAllFlutterViews() async { Future<List<FlutterView>> getAllFlutterViews({
Duration timeout = _kRpcTimeout,
}) async {
final List<FlutterView> views = <FlutterView>[]; final List<FlutterView> views = <FlutterView>[];
final Map<String, dynamic> rpcResponse = final Map<String, dynamic> rpcResponse =
await invokeRpc('_flutter.listViews', timeout: _kRpcTimeout); await invokeRpc('_flutter.listViews', timeout: timeout);
for (Map<String, dynamic> jsonView in rpcResponse['views']) { for (Map<String, dynamic> jsonView in rpcResponse['views']) {
final FlutterView flutterView = FlutterView._fromJson(jsonView); final FlutterView flutterView = FlutterView._fromJson(jsonView);
if (flutterView != null) { if (flutterView != null) {
......
...@@ -249,17 +249,17 @@ class FuchsiaRemoteConnection { ...@@ -249,17 +249,17 @@ class FuchsiaRemoteConnection {
Pattern pattern, Pattern pattern,
Duration timeout = _kIsolateFindTimeout, Duration timeout = _kIsolateFindTimeout,
]) async { ]) async {
final Completer<List<IsolateRef>> completer = final Completer<List<IsolateRef>> completer = Completer<List<IsolateRef>>();
Completer<List<IsolateRef>>();
_onDartVmEvent.listen( _onDartVmEvent.listen(
(DartVmEvent event) async { (DartVmEvent event) async {
if (event.eventType == DartVmEventType.started) { if (event.eventType == DartVmEventType.started) {
_log.fine('New VM found on port: ${event.servicePort}. Searching ' _log.fine('New VM found on port: ${event.servicePort}. Searching '
'for Isolate: $pattern'); 'for Isolate: $pattern');
final DartVm vmService = await _getDartVm(event.uri.port); final DartVm vmService = await _getDartVm(event.uri.port);
// If the VM service is null, set the result to the empty list.
final List<IsolateRef> result = await vmService final List<IsolateRef> result = await vmService
.getMainIsolatesByPattern(pattern) ?.getMainIsolatesByPattern(pattern, timeout: timeout) ??
.timeout(timeout); <IsolateRef>[];
if (result.isNotEmpty) { if (result.isNotEmpty) {
if (!completer.isCompleted) { if (!completer.isCompleted) {
completer.complete(result); completer.complete(result);
...@@ -301,6 +301,9 @@ class FuchsiaRemoteConnection { ...@@ -301,6 +301,9 @@ class FuchsiaRemoteConnection {
<Future<List<IsolateRef>>>[]; <Future<List<IsolateRef>>>[];
for (PortForwarder fp in _dartVmPortMap.values) { for (PortForwarder fp in _dartVmPortMap.values) {
final DartVm vmService = await _getDartVm(fp.port).timeout(timeout); final DartVm vmService = await _getDartVm(fp.port).timeout(timeout);
if (vmService == null) {
continue;
}
isolates.add(vmService.getMainIsolatesByPattern(pattern)); isolates.add(vmService.getMainIsolatesByPattern(pattern));
} }
final List<IsolateRef> result = await Future.wait(isolates) final List<IsolateRef> result = await Future.wait(isolates)
...@@ -379,16 +382,11 @@ class FuchsiaRemoteConnection { ...@@ -379,16 +382,11 @@ class FuchsiaRemoteConnection {
} }
for (PortForwarder pf in _dartVmPortMap.values) { for (PortForwarder pf in _dartVmPortMap.values) {
// When raising an HttpException this means that there is no instance of
// the Dart VM to communicate with. The TimeoutException is raised when
// the Dart VM instance is shut down in the middle of communicating.
try {
final DartVm service = await _getDartVm(pf.port); final DartVm service = await _getDartVm(pf.port);
result.add(await vmFunction(service)); if (service == null) {
} on HttpException {
await shutDownPortForwarder(pf);
} on TimeoutException {
await shutDownPortForwarder(pf); await shutDownPortForwarder(pf);
} else {
result.add(await vmFunction(service));
} }
} }
_stalePorts.forEach(_dartVmPortMap.remove); _stalePorts.forEach(_dartVmPortMap.remove);
...@@ -407,10 +405,25 @@ class FuchsiaRemoteConnection { ...@@ -407,10 +405,25 @@ class FuchsiaRemoteConnection {
return uri; return uri;
} }
/// Attempts to create a connection to a Dart VM.
///
/// Returns null if either there is an [HttpException] or a
/// [TimeoutException], else a [DartVm] instance.
Future<DartVm> _getDartVm(int port) async { Future<DartVm> _getDartVm(int port) async {
if (!_dartVmCache.containsKey(port)) { if (!_dartVmCache.containsKey(port)) {
// When raising an HttpException this means that there is no instance of
// the Dart VM to communicate with. The TimeoutException is raised when
// the Dart VM instance is shut down in the middle of communicating.
try {
final DartVm dartVm = await DartVm.connect(_getDartVmUri(port)); final DartVm dartVm = await DartVm.connect(_getDartVmUri(port));
_dartVmCache[port] = dartVm; _dartVmCache[port] = dartVm;
} on HttpException {
_log.warning('HTTP Exception encountered connecting to new VM');
return null;
} on TimeoutException {
_log.warning('TimeoutException encountered connecting to new VM');
return null;
}
} }
return _dartVmCache[port]; return _dartVmCache[port];
} }
...@@ -506,7 +519,6 @@ class FuchsiaRemoteConnection { ...@@ -506,7 +519,6 @@ class FuchsiaRemoteConnection {
final int lastSpace = trimmed.lastIndexOf(' '); final int lastSpace = trimmed.lastIndexOf(' ');
final String lastWord = trimmed.substring(lastSpace + 1); final String lastWord = trimmed.substring(lastSpace + 1);
if ((lastWord != '.') && (lastWord != '..')) { if ((lastWord != '.') && (lastWord != '..')) {
final int value = int.tryParse(lastWord); final int value = int.tryParse(lastWord);
if (value != null) { if (value != null) {
ports.add(value); ports.add(value);
......
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