[dwds] Add unbatching retry mechanism for failing batched expression evals (#2552)

[dwds] Add unbatching retry mechanism for failing batched expression evals.

---------

Co-authored-by: Nate Biggs <[email protected]>

Author: biggs0125

dwds/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 - Update to be forward compatible with changes to `package:shelf_web_socket`.
 - Added support for some debugging APIs with the DDC library bundle format. - [#2537](https://github.com/dart-lang/webdev/issues/2537),[#2544](https://github.com/dart-lang/webdev/issues/2544)
+- Fix issue where batched expression evals were failing if any subexpression failed. - [#2551](https://github.com/dart-lang/webdev/issues/2551)
 
 ## 24.2.0
 

dwds/lib/src/services/batched_expression_evaluator.dart

@@ -134,38 +134,37 @@ class BatchedExpressionEvaluator extends ExpressionEvaluator {
       first.scope,
     );
 
+    final listId = list.objectId;
+    if (listId == null) {
+      for (final request in requests) {
+        safeUnawaited(_evaluateBatch([request]));
+      }
+      return;
+    }
+
     for (var i = 0; i < requests.length; i++) {
       final request = requests[i];
       if (request.completer.isCompleted) continue;
       _logger.fine('Getting result out of a batch for ${request.expression}');
 
-      final listId = list.objectId;
-      if (listId == null) {
-        final error = createError(
-          EvaluationErrorKind.internal,
-          'No batch result object ID.',
-        );
-        request.completer.complete(error);
-      } else {
-        safeUnawaited(
-          _inspector
-              .getProperties(
-            listId,
-            offset: i,
-            count: 1,
-            length: requests.length,
-          )
-              .then((v) {
-            final result = v.first.value!;
-            _logger.fine(
-              'Got result out of a batch for ${request.expression}: $result',
-            );
-            request.completer.complete(result);
-          }),
-          onError: (error, stackTrace) =>
-              request.completer.completeError(error, stackTrace),
-        );
-      }
+      safeUnawaited(
+        _inspector
+            .getProperties(
+          listId,
+          offset: i,
+          count: 1,
+          length: requests.length,
+        )
+            .then((v) {
+          final result = v.first.value!;
+          _logger.fine(
+            'Got result out of a batch for ${request.expression}: $result',
+          );
+          request.completer.complete(result);
+        }),
+        onError: (error, stackTrace) =>
+            request.completer.completeError(error, stackTrace),
+      );
     }
   }
 }

dwds/test/expression_evaluator_test.dart

@@ -13,6 +13,7 @@ import 'package:dwds/src/debugging/location.dart';
 import 'package:dwds/src/debugging/skip_list.dart';
 import 'package:dwds/src/services/batched_expression_evaluator.dart';
 import 'package:dwds/src/services/expression_evaluator.dart';
+import 'package:dwds/src/utilities/shared.dart';
 
 import 'package:test/test.dart';
 import 'package:vm_service/vm_service.dart' hide LogRecord;
@@ -36,6 +37,7 @@ void main() async {
 
     late StreamController<DebuggerPausedEvent> pausedController;
     late StreamController<Event> debugEventController;
+    late FakeInspector inspector;
     setUp(() async {
       final assetReader = FakeAssetReader(sourceMap: '');
       final toolConfiguration = TestToolConfiguration.withLoadStrategy(
@@ -64,8 +66,7 @@ void main() async {
         skipLists,
         root,
       );
-      final inspector =
-          FakeInspector(webkitDebugger, fakeIsolate: simpleIsolate);
+      inspector = FakeInspector(webkitDebugger, fakeIsolate: simpleIsolate);
       debugger.updateInspector(inspector);
 
       _evaluator = ExpressionEvaluator(
@@ -192,6 +193,21 @@ void main() async {
         );
       });
 
+      test('retries failed batched expression', () async {
+        safeUnawaited(
+          evaluator.evaluateExpression('2', 'main.dart', 'true', {}),
+        );
+
+        await evaluator.evaluateExpression('2', 'main.dart', 'false', {});
+        expect(inspector.functionsCalled.length, 3);
+        expect(
+          inspector.functionsCalled[0].contains('return [ true, false ];'),
+          true,
+        );
+        expect(inspector.functionsCalled[1].contains('return true;'), true);
+        expect(inspector.functionsCalled[2].contains('return false;'), true);
+      });
+
       test('returns error if closed', () async {
         evaluator.close();
         final result =

dwds/test/fixtures/fakes.dart

@@ -48,6 +48,7 @@ Isolate get simpleIsolate => Isolate(
 
 class FakeInspector implements AppInspector {
   final WebkitDebugger _remoteDebugger;
+  final List<String> functionsCalled = [];
   FakeInspector(this._remoteDebugger, {required this.fakeIsolate});
 
   Isolate fakeIsolate;
@@ -61,8 +62,10 @@ class FakeInspector implements AppInspector {
   Future<RemoteObject> callFunction(
     String function,
     Iterable<String> argumentIds,
-  ) async =>
-      RemoteObject({'type': 'string', 'value': 'true'});
+  ) async {
+    functionsCalled.add(function);
+    return RemoteObject({'type': 'string', 'value': 'true'});
+  }
 
   @override
   Future<void> initialize() async => {};
@@ -473,3 +476,7 @@ final fakeWipResponse = WipResponse({
   'id': 1,
   'result': {'fake': ''},
 });
+
+final fakeFailingWipResponse = WipResponse({
+  'result': 'Error: Bad request',
+});