[dds] Expand truncated strings from dart:developer log() calls in DAP output

Change-Id: I31338b00daff33a5428e4b2229273ee8289f1c5e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206540
Reviewed-by: Ben Konyi <[email protected]>

Author: DanTup

pkg/dds/lib/src/dap/adapters/dart.dart

@@ -139,6 +139,9 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
   /// processed its initial paused state).
   Future<void> get debuggerInitialized => _debuggerInitializedCompleter.future;
 
+  bool get evaluateToStringInDebugViews =>
+      args.evaluateToStringInDebugViews ?? false;
+
   /// [attachRequest] is called by the client when it wants us to to attach to
   /// an existing app. This will only be called once (and only one of this or
   /// launchRequest will be called).
@@ -430,7 +433,7 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
       final resultString = await _converter.convertVmInstanceRefToDisplayString(
         thread,
         result,
-        allowCallingToString: true,
+        allowCallingToString: evaluateToStringInDebugViews,
       );
       // TODO(dantup): We may need to store `expression` with this data
       // to allow building nested evaluateNames.
@@ -854,7 +857,8 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
             thread,
             variable.value,
             name: variable.name,
-            allowCallingToString: index <= maxToStringsPerEvaluation,
+            allowCallingToString: evaluateToStringInDebugViews &&
+                index <= maxToStringsPerEvaluation,
           );
         }
 
@@ -880,6 +884,7 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
         variables.addAll(await _converter.convertVmInstanceToVariablesList(
           thread,
           object,
+          allowCallingToString: evaluateToStringInDebugViews,
           startItem: childStart,
           numItems: childCount,
         ));
@@ -981,8 +986,15 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
       if (ref == null || ref.kind == vm.InstanceKind.kNull) {
         return null;
       }
-      // TODO(dantup): This should handle truncation and complex types.
-      return ref.valueAsString;
+      return _converter.convertVmInstanceRefToDisplayString(
+        thread,
+        ref,
+        // Always allow calling toString() here as the user expects the full
+        // string they logged regardless of the evaluateToStringInDebugViews
+        // setting.
+        allowCallingToString: true,
+        includeQuotesAroundString: false,
+      );
     }
 
     var loggerName = await asString(record.loggerName);
@@ -996,13 +1008,13 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
     final prefix = '[$loggerName] ';
 
     if (message != null) {
-      sendPrefixedOutput('stdout', prefix, '$message\n');
+      sendPrefixedOutput('console', prefix, '$message\n');
     }
     if (error != null) {
-      sendPrefixedOutput('stderr', prefix, '$error\n');
+      sendPrefixedOutput('console', prefix, '$error\n');
     }
     if (stack != null) {
-      sendPrefixedOutput('stderr', prefix, '$stack\n');
+      sendPrefixedOutput('console', prefix, '$stack\n');
     }
   }
 

pkg/dds/lib/src/dap/protocol_converter.dart

@@ -57,21 +57,32 @@ class ProtocolConverter {
     required bool allowCallingToString,
     bool includeQuotesAroundString = true,
   }) async {
-    final canCallToString = allowCallingToString &&
-        (_adapter.args.evaluateToStringInDebugViews ?? false);
-
-    if (ref.kind == 'String' || ref.valueAsString != null) {
-      var stringValue = ref.valueAsString.toString();
-      if (ref.valueAsStringIsTruncated ?? false) {
-        stringValue = '$stringValue…';
-      }
-      if (ref.kind == 'String' && includeQuotesAroundString) {
-        stringValue = '"$stringValue"';
-      }
-      return stringValue;
+    final isTruncated = ref.valueAsStringIsTruncated ?? false;
+    if (ref.kind == vm.InstanceKind.kString && isTruncated) {
+      // Call toString() if allowed, otherwise (or if it returns null) fall back
+      // to the truncated value with "…" suffix.
+      var stringValue = allowCallingToString
+          ? await _callToString(
+              thread,
+              ref,
+              includeQuotesAroundString: includeQuotesAroundString,
+            )
+          : null;
+      stringValue ??= '${ref.valueAsString}…';
+
+      return includeQuotesAroundString ? '"$stringValue"' : stringValue;
+    } else if (ref.kind == vm.InstanceKind.kString) {
+      // Untruncated strings.
+      return includeQuotesAroundString
+          ? '"${ref.valueAsString}"'
+          : ref.valueAsString.toString();
+    } else if (ref.valueAsString != null) {
+      return isTruncated
+          ? '${ref.valueAsString}…'
+          : ref.valueAsString.toString();
     } else if (ref.kind == 'PlainInstance') {
       var stringValue = ref.classRef?.name ?? '<unknown instance>';
-      if (canCallToString) {
+      if (allowCallingToString) {
         final toStringValue = await _callToString(
           thread,
           ref,
@@ -99,6 +110,7 @@ class ProtocolConverter {
   Future<List<dap.Variable>> convertVmInstanceToVariablesList(
     ThreadInfo thread,
     vm.Instance instance, {
+    required bool allowCallingToString,
     int? startItem = 0,
     int? numItems,
   }) async {
@@ -112,7 +124,7 @@ class ProtocolConverter {
         await convertVmResponseToVariable(
           thread,
           instance,
-          allowCallingToString: true,
+          allowCallingToString: allowCallingToString,
         )
       ];
     } else if (elements != null) {
@@ -121,10 +133,15 @@ class ProtocolConverter {
       return Future.wait(elements
           .cast<vm.Response>()
           .sublist(start, numItems != null ? start + numItems : null)
-          .mapIndexed((index, response) async => convertVmResponseToVariable(
-              thread, response,
+          .mapIndexed(
+            (index, response) => convertVmResponseToVariable(
+              thread,
+              response,
               name: '${start + index}',
-              allowCallingToString: index <= maxToStringsPerEvaluation)));
+              allowCallingToString:
+                  allowCallingToString && index <= maxToStringsPerEvaluation,
+            ),
+          ));
     } else if (associations != null) {
       // For maps, create a variable for each entry (in the requested subset).
       // Use the keys and values to create a display string in the form
@@ -135,13 +152,14 @@ class ProtocolConverter {
       return Future.wait(associations
           .sublist(start, numItems != null ? start + numItems : null)
           .mapIndexed((index, mapEntry) async {
-        final allowCallingToString = index <= maxToStringsPerEvaluation;
+        final callToString =
+            allowCallingToString && index <= maxToStringsPerEvaluation;
         final keyDisplay = await convertVmResponseToDisplayString(
             thread, mapEntry.key,
-            allowCallingToString: allowCallingToString);
+            allowCallingToString: callToString);
         final valueDisplay = await convertVmResponseToDisplayString(
             thread, mapEntry.value,
-            allowCallingToString: allowCallingToString);
+            allowCallingToString: callToString);
         return dap.Variable(
           name: '${start + index}',
           value: '$keyDisplay -> $valueDisplay',
@@ -154,7 +172,8 @@ class ProtocolConverter {
           (index, field) async => convertVmResponseToVariable(
               thread, field.value,
               name: field.decl?.name ?? '<unnamed field>',
-              allowCallingToString: index <= maxToStringsPerEvaluation)));
+              allowCallingToString:
+                  allowCallingToString && index <= maxToStringsPerEvaluation)));
 
       // Also evaluate the getters if evaluateGettersInDebugViews=true enabled.
       final service = _adapter.vmService;
@@ -177,7 +196,8 @@ class ProtocolConverter {
             thread,
             response,
             name: getterName,
-            allowCallingToString: index <= maxToStringsPerEvaluation,
+            allowCallingToString:
+                allowCallingToString && index <= maxToStringsPerEvaluation,
           );
         }
 
@@ -397,18 +417,26 @@ class ProtocolConverter {
     if (service == null) {
       return null;
     }
-    final result = await service.invoke(
+    var result = await service.invoke(
       thread.isolate.id!,
       ref.id!,
       'toString',
       [],
       disableBreakpoints: true,
     );
 
+    // If the response is a string and is truncated, use getObject() to get the
+    // full value.
+    if (result is vm.InstanceRef &&
+        result.kind == 'String' &&
+        (result.valueAsStringIsTruncated ?? false)) {
+      result = await service.getObject(thread.isolate.id!, result.id!);
+    }
+
     return convertVmResponseToDisplayString(
       thread,
       result,
-      allowCallingToString: false,
+      allowCallingToString: false, // Don't allow recursing.
       includeQuotesAroundString: includeQuotesAroundString,
     );
   }

pkg/dds/test/dap/integration/debug_logging_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:test/test.dart';
 
+import 'test_client.dart';
 import 'test_support.dart';
 
 main() {
@@ -36,6 +37,33 @@ void main(List<String> args) async {
         'Exited.',
       ]);
     });
+
+    test('prints long messages from dart:developer log()', () async {
+      // Make a long message that's more than 255 chars (where the VM truncates
+      // log strings by default).
+      final longMessage = 'this is a test' * 20;
+      final testFile = dap.createTestFile('''
+import 'dart:developer';
+
+void main(List<String> args) async {
+  log('$longMessage');
+  // Prevent us exiting before the async log messages may have completed.
+  // The test will terminate the script early once the expectations are met.
+  await Future.delayed(const Duration(seconds: 30));
+}
+    ''');
+      final expectedLogMessage = '[log] $longMessage\n';
+
+      final consoleOutputs = dap.client.outputEvents
+          .where((event) => event.category == 'console')
+          .map((event) => event.output);
+
+      await Future.wait([
+        expectLater(consoleOutputs, emitsThrough(expectedLogMessage)),
+        dap.client.start(file: testFile),
+      ]);
+      await dap.client.terminate();
+    });
     // These tests can be slow due to starting up the external server process.
   }, timeout: Timeout.none);
 }

pkg/dds/test/dap/integration/no_debug_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:test/test.dart';
 
+import 'test_client.dart';
 import 'test_support.dart';
 
 main() {

pkg/dds/test/dap/integration/test_client.dart

@@ -54,21 +54,6 @@ class DapTestClient {
   Stream<OutputEventBody> get outputEvents => events('output')
       .map((e) => OutputEventBody.fromJson(e.body as Map<String, Object?>));
 
-  /// Collects all output events until the program terminates.
-  Future<List<OutputEventBody>> collectOutput(
-      {File? file, Future<Response> Function()? launch}) async {
-    final outputEventsFuture = outputEvents.toList();
-
-    // Launch script and wait for termination.
-    await Future.wait([
-      event('terminated'),
-      initialize(),
-      launch?.call() ?? this.launch(file!.path),
-    ], eagerError: true);
-
-    return outputEventsFuture;
-  }
-
   /// Sends a continue request for the given thread.
   ///
   /// Returns a Future that completes when the server returns a corresponding
@@ -207,6 +192,19 @@ class DapTestClient {
       sendRequest(StackTraceArguments(
           threadId: threadId, startFrame: startFrame, levels: numFrames));
 
+  /// Initializes the debug adapter and launches [file] or calls the custom
+  /// [launch] method.
+  Future<void> start({
+    File? file,
+    Future<Response> Function()? launch,
+  }) {
+    // Launch script and wait for termination.
+    return Future.wait([
+      initialize(),
+      launch?.call() ?? this.launch(file!.path),
+    ], eagerError: true);
+  }
+
   /// Sends a stepIn request for the given thread.
   ///
   /// Returns a Future that completes when the server returns a corresponding
@@ -433,6 +431,21 @@ extension DapTestClientExtension on DapTestClient {
     return ThreadsResponseBody.fromJson(response.body as Map<String, Object?>);
   }
 
+  /// Collects all output events until the program terminates.
+  ///
+  /// These results include all events in the order they are recieved, including
+  /// console, stdout and stderr.
+  Future<List<OutputEventBody>> collectOutput({
+    File? file,
+    Future<Response> Function()? launch,
+  }) async {
+    final outputEventsFuture = outputEvents.toList();
+
+    await start(file: file, launch: launch);
+
+    return outputEventsFuture;
+  }
+
   /// A helper that fetches scopes for a frame, checks for one with the name
   /// [expectedName] and verifies its variables.
   Future<Scope> expectScopeVariables(