Fix a crash during execution context detection (#2286)

* Make dwds tolerant to failure of detecting dart execution context id

* Update changelog

* Addressed CR comments

* Cleanup

* Cleanup

Author: Anna Gringauze

dwds/CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 23.0.0-wip
 - Restructure `LoadStrategy` to provide build settings. - [#2270](https://github.com/dart-lang/webdev/pull/2270)
 - Add `FrontendServerLegacyStrategyProvider` and update bootstrap generation logic for `LegacyStrategy` - [#2285](https://github.com/dart-lang/webdev/pull/2285)
+- Tolerate failures to detect a dart execution context. - [#2286](https://github.com/dart-lang/webdev/pull/2286)
 
 ## 22.1.0
 - Update `package:vm_service` constraint to `^13.0.0`. - [#2265](https://github.com/dart-lang/webdev/pull/2265)

dwds/lib/src/connections/debug_connection.dart

@@ -37,7 +37,7 @@ class DebugConnection {
   VmService get vmService => _appDebugServices.dwdsVmClient.client;
 
   Future<void> close() => _closed ??= () async {
-        _appDebugServices.chromeProxyService.remoteDebugger.close();
+        await _appDebugServices.chromeProxyService.remoteDebugger.close();
         await _appDebugServices.close();
         _onDoneCompleter.complete();
       }();

dwds/lib/src/debugging/execution_context.dart

@@ -9,8 +9,9 @@ import 'package:dwds/src/debugging/remote_debugger.dart';
 import 'package:logging/logging.dart';
 
 abstract class ExecutionContext {
-  /// Returns the context ID that contains the running Dart application.
-  Future<int> get id;
+  /// Returns the context ID that contains the running Dart application,
+  /// if available.
+  Future<int?> get id;
 }
 
 /// The execution context in which to do remote evaluations.
@@ -23,12 +24,12 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
   /// Context can be null if an error has occurred and we cannot detect
   /// and parse the context ID.
   late StreamQueue<int> _contexts;
+  final _contextController = StreamController<int>();
+  final _seenContexts = <int>[];
 
   int? _id;
 
-  @override
-  Future<int> get id async {
-    if (_id != null) return _id!;
+  Future<int?> _lookUpId() async {
     _logger.fine('Looking for Dart execution context...');
     const timeoutInMs = 100;
     while (await _contexts.hasNext.timeout(
@@ -41,6 +42,7 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
       },
     )) {
       final context = await _contexts.next;
+      _seenContexts.add(context);
       _logger.fine('Checking context id: $context');
       try {
         final result = await _remoteDebugger.sendCommand(
@@ -51,24 +53,34 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
           },
         );
         if (result.result?['result']?['value'] != null) {
-          _logger.fine('Found valid execution context: $context');
-          _id = context;
-          break;
+          _logger.fine('Found dart execution context: $context');
+          return context;
         }
       } catch (_) {
         // Errors may be thrown if we attempt to evaluate in a stale a context.
         // Ignore and continue.
-        _logger.fine('Invalid execution context: $context');
+        _logger.fine('Stale execution context: $context');
+        _seenContexts.remove(context);
       }
     }
+    return null;
+  }
+
+  @override
+  Future<int?> get id async {
+    if (_id != null) return _id;
+
+    _id = await _lookUpId();
     if (_id == null) {
-      throw StateError('No context with the running Dart application.');
+      // Add seen contexts back to the queue in case the injected
+      // client is still loading, so the next call to `id` succeeds.
+      _seenContexts.forEach(_contextController.add);
+      _seenContexts.clear();
     }
-    return _id!;
+    return _id;
   }
 
   RemoteDebuggerExecutionContext(this._id, this._remoteDebugger) {
-    final contextController = StreamController<int>();
     _remoteDebugger
         .eventStream('Runtime.executionContextsCleared', (e) => e)
         .listen((_) => _id = null);
@@ -86,8 +98,8 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
       }
       return parsedId;
     }).listen((e) {
-      if (e != null) contextController.add(e);
+      if (e != null) _contextController.add(e);
     });
-    _contexts = StreamQueue(contextController.stream);
+    _contexts = StreamQueue(_contextController.stream);
   }
 }

dwds/lib/src/debugging/remote_debugger.dart

@@ -72,5 +72,5 @@ abstract class RemoteDebugger {
 
   Stream<T> eventStream<T>(String method, WipEventTransformer<T> transformer);
 
-  void close();
+  Future<void> close();
 }

dwds/lib/src/debugging/webkit_debugger.dart

@@ -32,7 +32,7 @@ class WebkitDebugger implements RemoteDebugger {
       _wipDebugger.sendCommand(command, params: params);
 
   @override
-  void close() => _closed ??= _wipDebugger.connection.close();
+  Future<void> close() => _closed ??= _wipDebugger.connection.close();
 
   @override
   Future<void> disable() => _wipDebugger.disable();

dwds/lib/src/dwds_vm_client.dart

@@ -213,6 +213,19 @@ void _recordDwdsStats(DwdsStats dwdsStats, String screen) {
   }
 }
 
+Future<int> tryGetContextId(
+  ChromeProxyService chromeProxyService, {
+  int retries = 3,
+}) async {
+  const waitInMs = 50;
+  for (var retry = 0; retry < retries; retry++) {
+    final tryId = await chromeProxyService.executionContext.id;
+    if (tryId != null) return tryId;
+    await Future.delayed(const Duration(milliseconds: waitInMs));
+  }
+  throw StateError('No context with the running Dart application.');
+}
+
 Future<Map<String, dynamic>> _hotRestart(
   ChromeProxyService chromeProxyService,
   VmService client,
@@ -223,7 +236,7 @@ Future<Map<String, dynamic>> _hotRestart(
   await _disableBreakpointsAndResume(client, chromeProxyService);
   try {
     _logger.info('Attempting to get execution context ID.');
-    await chromeProxyService.executionContext.id;
+    await tryGetContextId(chromeProxyService);
     _logger.info('Got execution context ID.');
   } on StateError catch (e) {
     // We couldn't find the execution context. `hotRestart` may have been

dwds/lib/src/loaders/legacy.dart

@@ -66,6 +66,7 @@ class LegacyStrategy extends LoadStrategy {
   ///   packages/path/path -> d348c2a4647e998011fe305f74f22961
   ///
   final Future<Map<String, String>> Function(MetadataProvider metadataProvider)
+      // ignore: unused_field
       _digestsProvider;
 
   /// Returns the module for the corresponding server path.

dwds/lib/src/servers/extension_debugger.dart

@@ -192,7 +192,7 @@ class ExtensionDebugger implements RemoteDebugger {
   int newId() => _completerId++;
 
   @override
-  void close() => _closed ??= () {
+  Future<void> close() => _closed ??= () {
         _closeController.add({});
         return Future.wait([
           sseConnection.sink.close(),