Skip to content

Commit

Permalink
[flutter_tools] remove all body_might_complete_normally_catch_error i…
Browse files Browse the repository at this point in the history
…gnores (#115184)

* remove all body_might_complete_normally_catch_error ignores

* add a test
  • Loading branch information
christopherfujino authored Nov 16, 2022
1 parent d7454d5 commit a2233ea
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 30 deletions.
14 changes: 4 additions & 10 deletions packages/flutter_tools/lib/src/proxied_devices/devices.dart
Original file line number Diff line number Diff line change
Expand Up @@ -522,16 +522,13 @@ class ProxiedPortForwarder extends DevicePortForwarder {
socket.listen((Uint8List data) {
unawaited(connection.sendRequest('proxy.write', <String, Object>{
'id': id,
}, data)
// TODO(srawlins): Fix this static issue,
// https://github.com/flutter/flutter/issues/105750.
// ignore: body_might_complete_normally_catch_error
.catchError((Object error, StackTrace stackTrace) {
}, data).catchError((Object error, StackTrace stackTrace) {
// Log the error, but proceed normally. Network failure should not
// crash the tool. If this is critical, the place where the connection
// is being used would crash.
_logger.printWarning('Write to remote proxy error: $error');
_logger.printTrace('Write to remote proxy error: $error, stack trace: $stackTrace');
return null;
}));
});
_connectedSockets.add(socket);
Expand All @@ -543,15 +540,12 @@ class ProxiedPortForwarder extends DevicePortForwarder {
// Send a proxy disconnect event just in case.
unawaited(connection.sendRequest('proxy.disconnect', <String, Object>{
'id': id,
})
// TODO(srawlins): Fix this static issue,
// https://github.com/flutter/flutter/issues/105750.
// ignore: body_might_complete_normally_catch_error
.catchError((Object error, StackTrace stackTrace) {
}).catchError((Object error, StackTrace stackTrace) {
// Ignore the error here. There might be a race condition when the
// remote end also disconnects. In any case, this request is just to
// notify the remote end to disconnect and we should not crash when
// there is an error here.
return null;
}));
_connectedSockets.remove(socket);
}));
Expand Down
11 changes: 7 additions & 4 deletions packages/flutter_tools/lib/src/run_hot.dart
Original file line number Diff line number Diff line change
Expand Up @@ -629,16 +629,19 @@ class HotRunner extends ResidentRunner {
if (uiIsolatesIds.contains(isolateRef.id)) {
continue;
}
operations.add(device.vmService!.service.kill(isolateRef.id!)
// TODO(srawlins): Fix this static issue,
// https://github.com/flutter/flutter/issues/105750.
// ignore: body_might_complete_normally_catch_error
operations.add(
device.vmService!.service.kill(isolateRef.id!)
// Since we never check the value of this Future, only await its
// completion, make its type nullable so we can return null when
// catching errors.
.then<vm_service.Success?>((vm_service.Success success) => success)
.catchError((dynamic error, StackTrace stackTrace) {
// Do nothing on a SentinelException since it means the isolate
// has already been killed.
// Error code 105 indicates the isolate is not yet runnable, and might
// be triggered if the tool is attempting to kill the asset parsing
// isolate before it has finished starting up.
return null;
}, test: (dynamic error) => error is vm_service.SentinelException
|| (error is vm_service.RPCError && error.code == 105)));
}
Expand Down
9 changes: 3 additions & 6 deletions packages/flutter_tools/lib/src/test/flutter_web_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -685,16 +685,13 @@ class BrowserManager {
);
final Completer<BrowserManager> completer = Completer<BrowserManager>();

unawaited(chrome.onExit.then((int? browserExitCode) {
unawaited(chrome.onExit.then<Object?>((int? browserExitCode) {
throwToolExit('${runtime.name} exited with code $browserExitCode before connecting.');
})
// TODO(srawlins): Fix this static issue,
// https://github.com/flutter/flutter/issues/105750.
// ignore: body_might_complete_normally_catch_error
.catchError((Object error, StackTrace stackTrace) {
}).catchError((Object error, StackTrace stackTrace) {
if (!completer.isCompleted) {
completer.completeError(error, stackTrace);
}
return null;
}));
unawaited(future.then((WebSocketChannel webSocket) {
if (completer.isCompleted) {
Expand Down
8 changes: 3 additions & 5 deletions packages/flutter_tools/lib/src/vmservice.dart
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ Future<vm_service.VmService> setUpVmService(
// Each service registration requires a request to the attached VM service. Since the
// order of these requests does not matter, store each future in a list and await
// all at the end of this method.
final List<Future<vm_service.Success>> registrationRequests = <Future<vm_service.Success>>[];
final List<Future<vm_service.Success?>> registrationRequests = <Future<vm_service.Success?>>[];
if (reloadSources != null) {
vmService.registerServiceCallback('reloadSources', (Map<String, Object?> params) async {
final String isolateId = _validateRpcStringParam('reloadSources', params, 'isolateId');
Expand Down Expand Up @@ -289,10 +289,8 @@ Future<vm_service.VmService> setUpVmService(
// thrown if we're already subscribed.
registrationRequests.add(vmService
.streamListen(vm_service.EventStreams.kExtension)
// TODO(srawlins): Fix this static issue,
// https://github.com/flutter/flutter/issues/105750.
// ignore: body_might_complete_normally_catch_error
.catchError((Object? error) {}, test: (Object? error) => error is vm_service.RPCError)
.then<vm_service.Success?>((vm_service.Success success) => success)
.catchError((Object? error) => null, test: (Object? error) => error is vm_service.RPCError)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,28 @@ void main() {
expect(fakeSocket.closeCalled, true);
});

testWithoutContext('handles errors', () async {
final FakeServerSocket fakeServerSocket = FakeServerSocket(200);
final ProxiedPortForwarder portForwarder = ProxiedPortForwarder(
FakeDaemonConnection(
handledRequests: <String, Object?>{
'proxy.connect': '1', // id
},
),
logger: bufferLogger,
createSocketServer: (Logger logger, int? hostPort) async =>
fakeServerSocket,
);
final int result = await portForwarder.forward(100);
expect(result, 200);

final FakeSocket fakeSocket = FakeSocket();
fakeServerSocket.controller.add(fakeSocket);

fakeSocket.controller.add(Uint8List.fromList(<int>[1, 2, 3]));
await pumpEventQueue();
});

testWithoutContext('forwards the port from the remote end with device id', () async {
final FakeServerSocket fakeServerSocket = FakeServerSocket(400);
final ProxiedPortForwarder portForwarder = ProxiedPortForwarder(
Expand Down Expand Up @@ -321,3 +343,33 @@ class FakeSocket extends Fake implements Socket {
@override
void destroy() {}
}

class FakeDaemonConnection extends Fake implements DaemonConnection {
FakeDaemonConnection({
this.handledRequests = const <String, Object?>{},
this.daemonEventStreams = const <String, List<DaemonEventData>>{},
});

/// Mapping of method name to returned object from the [sendRequest] method.
final Map<String, Object?> handledRequests;

final Map<String, List<DaemonEventData>> daemonEventStreams;

@override
Stream<DaemonEventData> listenToEvent(String eventToListen) {
final List<DaemonEventData>? iterable = daemonEventStreams[eventToListen];
if (iterable != null) {
return Stream<DaemonEventData>.fromIterable(iterable);
}
return const Stream<DaemonEventData>.empty();
}

@override
Future<Object?> sendRequest(String method, [Object? params, List<int>? binary]) async {
final Object? response = handledRequests[method];
if (response != null) {
return response;
}
throw Exception('"$method" request failed');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,10 @@ Future<ProcessTestResult> runFlutter(
}
process.stdin.write('q');
return -1; // discarded
})
// TODO(srawlins): Fix this static issue,
// https://github.com/flutter/flutter/issues/105750.
// ignore: body_might_complete_normally_catch_error
.catchError((Object error) { /* ignore errors here, they will be reported on the next line */ }));
}).catchError((Object error) {
// ignore errors here, they will be reported on the next line
return -1; // discarded
}));
final int exitCode = await process.exitCode;
if (streamingLogs) {
debugPrint('${stamp()} (process terminated with exit code $exitCode)');
Expand Down

0 comments on commit a2233ea

Please sign in to comment.