[flutter_tools] remove all body_might_complete_normally_catch_error ignores (#115184)

* remove all body_might_complete_normally_catch_error ignores

* add a test

Author: christopherfujino

packages/flutter_tools/lib/src/proxied_devices/devices.dart

@@ -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);
@@ -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);
       }));

packages/flutter_tools/lib/src/run_hot.dart

@@ -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)));
       }

packages/flutter_tools/lib/src/test/flutter_web_platform.dart

@@ -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) {

packages/flutter_tools/lib/src/vmservice.dart

@@ -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');
@@ -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)
     );
   }
 

packages/flutter_tools/test/general.shard/proxied_devices/proxied_devices_test.dart

@@ -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(
@@ -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');
+  }
+}

packages/flutter_tools/test/integration.shard/overall_experience_test.dart

@@ -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)');