From 957c5dfe3fcfe5d67d25fb84e4a65d353c19abbd Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 3 Nov 2026 11:36:39 -0800 Subject: [PATCH] Fix ParallelWaitError in forEachIsolateHelper during service disconnect `forEachIsolateHelper` used `[for (final isolate in vmIsolates) callback(isolate)].wait`. In Dart 3, the `Iterable.wait` extension wraps any error from completed futures in a `ParallelWaitError`. When a target application exits or a VM Service connection is shut down while an operation is in flight (e.g. periodic polling in `refreshNetworkData`), VM Service methods like `getSocketProfileWrapper` throw `RPCError: (112) Service has disappeared`. Callers catch typed errors (e.g., `on RPCError catch (e)` in `network_service.dart` or `on SentinelException` / `on RPCError` in `service_extension_manager.dart`) and check `e.isServiceDisposedError` to safely swallow teardown exceptions. Because `ParallelWaitError` does not inherit from `RPCError` or `Exception`, it bypassed these typed catch blocks and escaped uncaught, causing flaky integration test failures in `network_screen_test.dart`. Using `Future.wait` preserves the underlying exception type thrown by the isolate callback. Also update `_enableNetworkTrafficRecordingOnAllIsolates` to use `Future.wait`. --- .../screens/network/network_controller.dart | 4 +- .../network/network_hot_restart_test.dart | 20 ++++ .../utils/hot_restart_network_vm_service.dart | 11 ++ packages/devtools_app_shared/CHANGELOG.md | 8 +- .../lib/src/service/service_utils.dart | 2 +- .../test/service/service_utils_test.dart | 110 ++++++++++++++++++ 6 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 packages/devtools_app_shared/test/service/service_utils_test.dart diff --git a/packages/devtools_app/lib/src/screens/network/network_controller.dart b/packages/devtools_app/lib/src/screens/network/network_controller.dart index 88490a270ca..b340f9cc6b3 100644 --- a/packages/devtools_app/lib/src/screens/network/network_controller.dart +++ b/packages/devtools_app/lib/src/screens/network/network_controller.dart @@ -377,10 +377,10 @@ class NetworkController extends DevToolsScreenController /// Enables HTTP timeline logging and socket profiling on all isolates. Future _enableNetworkTrafficRecordingOnAllIsolates() async { - await [ + await Future.wait([ http_service.toggleHttpRequestLogging(true), networkService.toggleSocketProfiling(true), - ].wait; + ]); } Future stopRecording() async { diff --git a/packages/devtools_app/test/screens/network/network_hot_restart_test.dart b/packages/devtools_app/test/screens/network/network_hot_restart_test.dart index cbf9cd68521..3e9cff107c3 100644 --- a/packages/devtools_app/test/screens/network/network_hot_restart_test.dart +++ b/packages/devtools_app/test/screens/network/network_hot_restart_test.dart @@ -373,6 +373,26 @@ void main() { }, ); + test( + 'refreshNetworkData swallows service disappeared error during isolate iteration', + () async { + final localVmService = HotRestartNetworkVmService(); + final localConnection = FakeServiceConnectionManager( + service: localVmService, + ); + final localController = await initNetworkLifecycleController( + vmService: localVmService, + fakeServiceConnection: localConnection, + ); + + localVmService.simulateServiceDisappeared = true; + await expectLater( + localController.networkService.refreshNetworkData(), + completes, + ); + }, + ); + test( 'updateLastHttpDataRefreshTime does not add entries for new isolates', () async { diff --git a/packages/devtools_app/test/screens/network/utils/hot_restart_network_vm_service.dart b/packages/devtools_app/test/screens/network/utils/hot_restart_network_vm_service.dart index ea1eed25d46..cdd175e5a5c 100644 --- a/packages/devtools_app/test/screens/network/utils/hot_restart_network_vm_service.dart +++ b/packages/devtools_app/test/screens/network/utils/hot_restart_network_vm_service.dart @@ -109,6 +109,10 @@ class HotRestartNetworkVmService extends FakeVmServiceWrapper { return Success(); } + /// When true, [getSocketProfileWrapper] throws an [RPCError] with + /// [RPCErrorKind.kServiceDisappeared], simulating service shutdown. + bool simulateServiceDisappeared = false; + @override Future forEachIsolate(Future Function(IsolateRef) callback) => callback(IsolateRef.parse({'id': _currentIsolateId, 'name': 'main'})!); @@ -173,6 +177,13 @@ class HotRestartNetworkVmService extends FakeVmServiceWrapper { @override Future getSocketProfileWrapper(String isolateId) async { + if (simulateServiceDisappeared) { + throw RPCError( + 'ext.dart.io.getSocketProfile', + RPCErrorKind.kServiceDisappeared.code, + 'Service has disappeared', + ); + } if (!(_socketProfilingEnabled[isolateId] ?? false)) { return SocketProfile(sockets: []); } diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md index 683bc2bdaf9..577b5c24924 100644 --- a/packages/devtools_app_shared/CHANGELOG.md +++ b/packages/devtools_app_shared/CHANGELOG.md @@ -4,10 +4,14 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. --> ## 0.5.2-wip +* Fix `forEachIsolateHelper` to rethrow isolate callback exceptions directly + using `Future.wait` instead of wrapping them in a `ParallelWaitError`. * Fix a `RangeError` thrown by `SplitPane` when the number of children changes between rebuilds. -* Fix garbage collection and timeout issues with the result list in `asyncEval` on both native VM and web. -* Safely handle RPC errors and unexpected exceptions when calling service extensions in `ServiceExtensionManager`. +* Fix garbage collection and timeout issues with the result list in `asyncEval` + on both native VM and web. +* Safely handle RPC errors and unexpected exceptions when calling service + extensions in `ServiceExtensionManager`. * The minimum Dart SDK version is bumped to 3.11.0. * The minimum Flutter SDK version is bumped to 3.41.0. * Fix `RootInfo.package` to be `null` when the root library is not a `package:` URI, diff --git a/packages/devtools_app_shared/lib/src/service/service_utils.dart b/packages/devtools_app_shared/lib/src/service/service_utils.dart index 5470605f681..82e00516528 100644 --- a/packages/devtools_app_shared/lib/src/service/service_utils.dart +++ b/packages/devtools_app_shared/lib/src/service/service_utils.dart @@ -88,6 +88,6 @@ Future forEachIsolateHelper( ) async { final vm = await vmService.getVM(); if (vm.isolates case final vmIsolates? when vmIsolates.isNotEmpty) { - await [for (final isolate in vmIsolates) callback(isolate)].wait; + await Future.wait([for (final isolate in vmIsolates) callback(isolate)]); } } diff --git a/packages/devtools_app_shared/test/service/service_utils_test.dart b/packages/devtools_app_shared/test/service/service_utils_test.dart new file mode 100644 index 00000000000..9d3c7c2933d --- /dev/null +++ b/packages/devtools_app_shared/test/service/service_utils_test.dart @@ -0,0 +1,110 @@ +// Copyright 2026 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. + +import 'package:devtools_app_shared/service.dart' hide SentinelException; +import 'package:flutter_test/flutter_test.dart'; +import 'package:vm_service/vm_service.dart'; + +void main() { + group('forEachIsolateHelper', () { + test('calls callback for each isolate', () async { + final isolates = [ + IsolateRef(id: 'isolate_1', number: '1', name: 'main_1'), + IsolateRef(id: 'isolate_2', number: '2', name: 'main_2'), + ]; + final fakeService = _FakeVmService(isolates: isolates); + final visited = []; + await forEachIsolateHelper(fakeService, (isolate) async { + visited.add(isolate.id!); + }); + expect(visited, equals(['isolate_1', 'isolate_2'])); + }); + + test('is a no-op when isolates list is empty or null', () async { + final emptyService = _FakeVmService(isolates: []); + var called = false; + await forEachIsolateHelper(emptyService, (isolate) async { + called = true; + }); + expect(called, isFalse); + + final nullService = _FakeVmService(isolates: null); + await forEachIsolateHelper(nullService, (isolate) async { + called = true; + }); + expect(called, isFalse); + }); + + test( + 'rethrows RPCError directly without wrapping in ParallelWaitError', + () async { + final isolates = [ + IsolateRef(id: 'isolate_1', number: '1', name: 'main_1'), + IsolateRef(id: 'isolate_2', number: '2', name: 'main_2'), + ]; + final fakeService = _FakeVmService(isolates: isolates); + final error = RPCError( + 'ext.dart.io.getSocketProfile', + RPCErrorKind.kServiceDisappeared.code, + 'Service has disappeared', + ); + + await expectLater( + () => forEachIsolateHelper( + fakeService, + (isolate) => Future.error(error), + ), + throwsA(same(error)), + ); + }, + ); + + test( + 'rethrows SentinelException directly without wrapping in ParallelWaitError', + () async { + final isolates = [ + IsolateRef(id: 'isolate_1', number: '1', name: 'main_1'), + ]; + final fakeService = _FakeVmService(isolates: isolates); + final sentinel = SentinelException.parse('getIsolate', { + 'type': 'Sentinel', + 'kind': 'Collected', + 'valueAsString': '', + }); + + await expectLater( + () => forEachIsolateHelper( + fakeService, + (isolate) => Future.error(sentinel), + ), + throwsA(same(sentinel)), + ); + }, + ); + + test( + 'VmServiceExtension.forEachIsolate delegates to forEachIsolateHelper', + () async { + final isolates = [ + IsolateRef(id: 'isolate_1', number: '1', name: 'main_1'), + ]; + final fakeService = _FakeVmService(isolates: isolates); + final visited = []; + await fakeService.forEachIsolate((isolate) async { + visited.add(isolate.id!); + }); + expect(visited, equals(['isolate_1'])); + }, + ); + }); +} + +class _FakeVmService extends Fake implements VmService { + _FakeVmService({required this.isolates}); + + final List? isolates; + + @override + Future getVM() async => VM(isolates: isolates); +}