Do not mark service extensions as 'added' so eagerly (#9271)

Author: srawlins

packages/devtools_app/lib/src/screens/network/network_service.dart

@@ -2,9 +2,9 @@
 // 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';
 import 'package:vm_service/vm_service.dart';
 
-import '../../service/vm_service_wrapper.dart';
 import '../../shared/globals.dart';
 import '../../shared/primitives/utils.dart';
 import '../../shared/utils/utils.dart';

packages/devtools_app/lib/src/service/vm_service_wrapper.dart

@@ -519,24 +519,3 @@ class TrackedFuture<T> {
   final String name;
   final Future<T> future;
 }
-
-extension RpcErrorExtension on RPCError {
-  /// Whether this [RPCError] is some kind of "VM Service connection has gone"
-  /// error that may occur if the VM is shut down.
-  bool get isServiceDisposedError {
-    if (code == RPCErrorKind.kServiceDisappeared.code ||
-        code == RPCErrorKind.kConnectionDisposed.code) {
-      return true;
-    }
-
-    if (code == RPCErrorKind.kServerError.code) {
-      // Always ignore "client is closed" and "closed with pending request"
-      // errors because these can always occur during shutdown if we were
-      // just starting to send (or had just sent) a request.
-      return message.contains('The client is closed') ||
-          message.contains('The client closed with pending request') ||
-          message.contains('Service connection dispose');
-    }
-    return false;
-  }
-}

packages/devtools_app/lib/src/shared/diagnostics/inspector_service.dart

@@ -22,7 +22,6 @@ import 'package:flutter/cupertino.dart';
 import 'package:flutter/foundation.dart';
 import 'package:vm_service/vm_service.dart';
 
-import '../../service/vm_service_wrapper.dart';
 import '../console/primitives/simple_items.dart';
 import '../globals.dart';
 import '../utils/utils.dart';

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -39,7 +39,8 @@ TODO: Remove this section if there are not any general updates.
 
 ## Network profiler updates
 
-TODO: Remove this section if there are not any general updates.
+* Fixed network logging after a hot restart. -
+  [#9271](https://github.com/flutter/devtools/pull/9271).
 
 ## Logging updates
 

packages/devtools_app_shared/lib/service.dart

@@ -10,6 +10,7 @@ export 'src/service/flutter_version.dart';
 export 'src/service/isolate_manager.dart' hide TestIsolateManager;
 export 'src/service/isolate_state.dart';
 export 'src/service/resolved_uri_manager.dart';
+export 'src/service/rpc_error_extension.dart';
 export 'src/service/service_extension_manager.dart'
     hide TestServiceExtensionManager;
 export 'src/service/service_manager.dart';

packages/devtools_app_shared/lib/src/service/rpc_error_extension.dart

@@ -0,0 +1,26 @@
+// Copyright 2025 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:vm_service/vm_service.dart';
+
+extension RpcErrorExtension on RPCError {
+  /// Whether this [RPCError] is some kind of "VM Service connection has gone"
+  /// error that may occur if the VM is shut down.
+  bool get isServiceDisposedError {
+    if (code == RPCErrorKind.kServiceDisappeared.code ||
+        code == RPCErrorKind.kConnectionDisposed.code) {
+      return true;
+    }
+
+    if (code == RPCErrorKind.kServerError.code) {
+      // Always ignore "client is closed" and "closed with pending request"
+      // errors because these can always occur during shutdown if we were
+      // just starting to send (or had just sent) a request.
+      return message.contains('The client is closed') ||
+          message.contains('The client closed with pending request') ||
+          message.contains('Service connection dispose');
+    }
+    return false;
+  }
+}

packages/devtools_app_shared/lib/src/service/service_extension_manager.dart

@@ -14,6 +14,7 @@ import '../utils/auto_dispose.dart';
 import 'connected_app.dart';
 import 'constants.dart';
 import 'isolate_manager.dart';
+import 'rpc_error_extension.dart';
 import 'service_extensions.dart' as extensions;
 import 'service_utils.dart';
 
@@ -245,7 +246,7 @@ final class ServiceExtensionManager with DisposerMixin {
   }
 
   Future<void> _addServiceExtension(String name) async {
-    if (!_serviceExtensions.add(name)) {
+    if (_serviceExtensions.contains(name)) {
       // If the service extension was already added we do not need to add it
       // again. This can happen depending on the timing between when extension
       // added events were received and when we requested the list of all
@@ -254,68 +255,94 @@ final class ServiceExtensionManager with DisposerMixin {
     }
     _hasServiceExtension(name).value = true;
 
-    if (_enabledServiceExtensions.containsKey(name)) {
+    final enabledServiceExtension = _enabledServiceExtensions[name];
+    if (enabledServiceExtension != null) {
       // Restore any previously enabled states by calling their service
       // extension. This will restore extension states on the device after a hot
       // restart. [_enabledServiceExtensions] will be empty on page refresh or
       // initial start.
       try {
-        return await _callServiceExtension(
+        final called = await _callServiceExtensionIfReady(
           name,
-          _enabledServiceExtensions[name]!.value,
+          enabledServiceExtension.value,
         );
+        if (called) {
+          // Only mark `name` as an "added service extension" if it was truly
+          // added. If it was added, then subsequent calls to
+          // `_addServiceExtension` with `name` will return early. If it was not
+          // really added, then subsequent calls to `_addServiceExtension` with
+          // `name` will proceed as usual.
+          _serviceExtensions.add(name);
+        }
+        return;
       } on SentinelException catch (_) {
         // Service extension stopped existing while calling, so do nothing.
         // This typically happens during hot restarts.
       }
     } else {
       // Set any extensions that are already enabled on the device. This will
       // enable extension states in DevTools on page refresh or initial start.
-      return await _restoreExtensionFromDevice(name);
+      final restored = await _restoreExtensionFromDeviceIfReady(name);
+      if (restored) {
+        // Only mark `name` as an "added service extension" if it was truly
+        // restored. If it was restored, then subsequent calls to
+        // `_addServiceExtension` with `name` will return early. If it was not
+        // really restored, then subsequent calls to `_addServiceExtension`
+        // with `name` will proceed as usual.
+        _serviceExtensions.add(name);
+      }
     }
   }
 
   IsolateRef? get _mainIsolate => _isolateManager.mainIsolate.value;
 
-  Future<void> _restoreExtensionFromDevice(String name) async {
+  /// Restores the service extension named [name] from the device.
+  ///
+  /// Returns whether isolates in the connected app are prepared for the restore.
+  Future<bool> _restoreExtensionFromDeviceIfReady(String name) async {
     final isolateRef = _isolateManager.mainIsolate.value;
-    if (isolateRef == null) return;
+    if (isolateRef == null) return false;
 
     if (!extensions.serviceExtensionsAllowlist.containsKey(name)) {
-      return;
+      return true;
     }
     final expectedValueType =
         extensions.serviceExtensionsAllowlist[name]!.values.first.runtimeType;
 
-    Future<void> restore() async {
+    /// Restores the service extension named [name].
+    ///
+    /// Returns whether isolates in the connected app are prepared for the
+    /// restore.
+    Future<bool> restore() async {
       // The restore request is obsolete if the isolate has changed.
-      if (isolateRef != _mainIsolate) return;
+      if (isolateRef != _mainIsolate) return false;
       try {
         final response = await _service!.callServiceExtension(
           name,
           isolateId: isolateRef.id,
         );
 
-        if (isolateRef != _mainIsolate) return;
+        if (isolateRef != _mainIsolate) return false;
 
         switch (expectedValueType) {
           case const (bool):
             final enabled = response.json!['enabled'] == 'true' ? true : false;
             await _maybeRestoreExtension(name, enabled);
-            return;
           case const (String):
             final String? value = response.json!['value'];
             await _maybeRestoreExtension(name, value);
-            return;
           case const (int):
           case const (double):
             final value = num.parse(
               response.json![name.substring(name.lastIndexOf('.') + 1)],
             );
             await _maybeRestoreExtension(name, value);
-            return;
           default:
-            return;
+            return true;
+        }
+      } on RPCError catch (e) {
+        if (e.isServiceDisposedError) {
+          return false;
         }
       } catch (e) {
         // Do not report an error if the VMService has gone away or the
@@ -325,22 +352,25 @@ final class ServiceExtensionManager with DisposerMixin {
         // of allowed network related exceptions rather than ignoring all
         // exceptions.
       }
+      return true;
     }
 
-    if (isolateRef != _mainIsolate) return;
+    if (isolateRef != _mainIsolate) return false;
 
     final isolate = await _isolateManager.isolateState(isolateRef).isolate;
-    if (isolateRef != _mainIsolate) return;
+    if (isolateRef != _mainIsolate) return false;
 
     // Do not try to restore Dart IO extensions for a paused isolate.
     if (extensions.isDartIoExtension(name) &&
         isolate?.pauseEvent?.kind?.contains('Pause') == true) {
       _callbacksOnIsolateResume.putIfAbsent(isolateRef, () => []).add(restore);
+      return true;
     } else {
-      await restore();
+      return await restore();
     }
   }
 
+  /// Maybe restores the service extension named [name] with [value].
   Future<void> _maybeRestoreExtension(String name, Object? value) async {
     final extensionDescription = extensions.serviceExtensionsAllowlist[name];
     if (extensionDescription is extensions.ToggleableServiceExtension) {
@@ -362,14 +392,15 @@ final class ServiceExtensionManager with DisposerMixin {
     }
   }
 
-  Future<void> _callServiceExtension(String name, Object? value) async {
-    if (_service == null) {
-      return;
-    }
+  /// Calls the service extension named [name] with [value].
+  ///
+  /// Returns whether isolates in the connected app are prepared for the call.
+  Future<bool> _callServiceExtensionIfReady(String name, Object? value) async {
+    if (_service == null) return false;
 
-    final mainIsolate = _isolateManager.mainIsolate.value;
-    Future<void> callExtension() async {
-      if (_isolateManager.mainIsolate.value != mainIsolate) return;
+    final mainIsolate = _mainIsolate;
+    Future<bool> callExtension() async {
+      if (_mainIsolate != mainIsolate) return false;
 
       assert(value != null);
       try {
@@ -411,25 +442,29 @@ final class ServiceExtensionManager with DisposerMixin {
         }
       } on RPCError catch (e) {
         if (e.code == RPCErrorKind.kServerError.code) {
-          // Connection disappeared
-          return;
+          // The connection disappeared.
+          return false;
         }
         rethrow;
       }
+
+      return true;
     }
 
-    if (mainIsolate == null) return;
+    if (mainIsolate == null) return false;
+
     final isolate = await _isolateManager.isolateState(mainIsolate).isolate;
-    if (_isolateManager.mainIsolate.value != mainIsolate) return;
+    if (_mainIsolate != mainIsolate) return false;
 
     // Do not try to call Dart IO extensions for a paused isolate.
     if (extensions.isDartIoExtension(name) &&
         isolate?.pauseEvent?.kind?.contains('Pause') == true) {
       _callbacksOnIsolateResume
           .putIfAbsent(mainIsolate, () => [])
           .add(callExtension);
+      return true;
     } else {
-      await callExtension();
+      return await callExtension();
     }
   }
 
@@ -488,7 +523,7 @@ final class ServiceExtensionManager with DisposerMixin {
     bool callExtension = true,
   }) async {
     if (callExtension && _serviceExtensions.contains(name)) {
-      await _callServiceExtension(name, value);
+      await _callServiceExtensionIfReady(name, value);
     } else if (callExtension) {
       _log.info(
         'Attempted to call extension \'$name\', but no service with that name exists',