Fix exception mapping from JS to dart (#2004)

* Prepare to release dwds

* Use dev channel to run publish

* Correctly switch to using dev SDK for publish task

* Try disabling publsh verification

* Merge with master

* Prepare webdev for release

* Make test_common depend on published dwds for the webdev release

* Fix dwds pub get failure

* Add missing new line

* Build

* Fix exception mapping from JS to dart

* Fix exception mapping from JS to dart

* Fix pub upgrade failure

* Fix test failures

* Fix analyzer errors

* Fix typo

* Cleanup

* Address CR comments

Author: Anna Gringauze

dwds/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 18.0.0-dev
+
+- Fix failure to map JS exceptions to dart.
+
 ## 18.0.0
 
 - Cleanup `getObject` code for lists and maps.

dwds/lib/src/debugging/debugger.dart

@@ -8,6 +8,7 @@ import 'dart:math' as math;
 import 'package:dwds/src/debugging/dart_scope.dart';
 import 'package:dwds/src/debugging/frame_computer.dart';
 import 'package:dwds/src/debugging/location.dart';
+import 'package:dwds/src/debugging/metadata/class.dart';
 import 'package:dwds/src/debugging/remote_debugger.dart';
 import 'package:dwds/src/debugging/skip_list.dart';
 import 'package:dwds/src/loaders/strategy.dart';
@@ -613,10 +614,9 @@ class Debugger extends Domain {
       if (e.data is Map<String, dynamic>) {
         final map = e.data as Map<String, dynamic>;
         if (map['type'] == 'object') {
-          // The className here is generally 'DartError'.
           final obj = RemoteObject(map);
           exception = await inspector.instanceRefFor(obj);
-          if (exception != null && isNativeJsObject(exception)) {
+          if (exception != null && isNativeJsError(exception)) {
             if (obj.description != null) {
               // Create a string exception object.
               final description =
@@ -787,18 +787,21 @@ Future<T> sendCommandAndValidateResult<T>(
   return result;
 }
 
+/// Returns true for objects we display for the user.
 bool isDisplayableObject(Object? object) =>
-    object is Sentinel || object is InstanceRef && !isNativeJsObject(object);
+    object is Sentinel ||
+    object is InstanceRef &&
+        !isNativeJsObject(object) &&
+        !isNativeJsError(object);
 
+/// Returns true for non-dart JavaScript objects.
 bool isNativeJsObject(InstanceRef instanceRef) {
-  // New type representation of JS objects reifies them to a type suffixed with
-  // JavaScriptObject.
-  final className = instanceRef.classRef?.name;
-  return (className != null &&
-          className.endsWith('JavaScriptObject') &&
-          instanceRef.classRef?.library?.uri == 'dart:_interceptors') ||
-      // Old type representation still needed to support older SDK versions.
-      className == 'NativeJavaScriptObject';
+  return isNativeJsObjectRef(instanceRef.classRef);
+}
+
+/// Returns true of JavaScript exceptions.
+bool isNativeJsError(InstanceRef instanceRef) {
+  return instanceRef.classRef == classRefForNativeJsError;
 }
 
 /// Returns the Dart line number for the provided breakpoint.

dwds/lib/src/debugging/instance.dart

@@ -128,6 +128,9 @@ class InstanceHelper extends Domain {
     } else if (metaData.isRecord) {
       return await _recordInstanceFor(classRef, remoteObject,
           offset: offset, count: count, length: metaData.length);
+    } else if (metaData.isNativeError) {
+      return await _plainInstanceFor(classRefForNativeJsError, remoteObject,
+          offset: offset, count: count, length: metaData.length);
     } else {
       return await _plainInstanceFor(classRef, remoteObject,
           offset: offset, count: count, length: metaData.length);
@@ -630,6 +633,14 @@ class InstanceHelper extends Domain {
               classRef: metaData.classRef)
             ..length = metaData.length;
         }
+        if (metaData.isNativeError) {
+          return InstanceRef(
+              kind: InstanceKind.kPlainInstance,
+              id: objectId,
+              identityHashCode: remoteObject.objectId.hashCode,
+              classRef: classRefForNativeJsError)
+            ..length = metaData.length;
+        }
         return InstanceRef(
             kind: InstanceKind.kPlainInstance,
             id: objectId,

dwds/lib/src/debugging/metadata/class.dart

@@ -6,10 +6,12 @@ import 'package:dwds/src/debugging/remote_debugger.dart';
 import 'package:dwds/src/loaders/strategy.dart';
 import 'package:dwds/src/services/chrome_debug_exception.dart';
 import 'package:dwds/src/utilities/domain.dart';
+import 'package:logging/logging.dart';
 import 'package:vm_service/vm_service.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
 const _dartCoreLibrary = 'dart:core';
+const _dartInterceptorsLibrary = 'dart:_interceptors';
 
 /// A hard-coded ClassRef for the Closure class.
 final classRefForClosure = classRefFor(_dartCoreLibrary, 'Closure');
@@ -20,6 +22,31 @@ final classRefForString = classRefFor(_dartCoreLibrary, InstanceKind.kString);
 /// A hard-coded ClassRef for a (non-existent) class called Unknown.
 final classRefForUnknown = classRefFor(_dartCoreLibrary, 'Unknown');
 
+/// A hard-coded ClassRef for a JS exception.
+///
+/// Exceptions are instances of NativeError and its subtypes.
+/// We detect their common base type in class metadata and replace their
+/// classRef by hard-coded reference in instances and instance refs.
+///
+/// TODO(annagrin): this breaks on name changes for JS types.
+/// https://github.com/dart-lang/sdk/issues/51583
+final classRefForNativeJsError =
+    classRefFor(_dartInterceptorsLibrary, 'NativeError');
+
+/// Returns true for non-dart JavaScript classes.
+///
+/// TODO(annagrin): this breaks on name changes for JS types.
+/// https://github.com/dart-lang/sdk/issues/51583
+bool isNativeJsObjectRef(ClassRef? classRef) {
+  final className = classRef?.name;
+  final libraryUri = classRef?.library?.uri;
+  // Non-dart JS objects are all instances of JavaScriptObject
+  // and its subtypes with names that end with 'JavaScriptObject'.
+  return className != null &&
+      libraryUri == _dartInterceptorsLibrary &&
+      className.endsWith('JavaScriptObject');
+}
+
 ///  A hard-coded LibraryRef for a a dart:core library.
 final libraryRefForCore = LibraryRef(
   id: _dartCoreLibrary,
@@ -43,6 +70,8 @@ ClassRef classRefFor(String libraryId, String? name) => ClassRef(
 
 /// Meta data for a remote Dart class in Chrome.
 class ClassMetaData {
+  static final _logger = Logger('ClassMetadata');
+
   /// The name of the JS constructor for the object.
   ///
   /// This may be a constructor for a Dart, but it's still a JS name. For
@@ -67,6 +96,7 @@ class ClassMetaData {
     Object? length,
     bool isFunction = false,
     bool isRecord = false,
+    bool isNativeError = false,
   }) {
     return ClassMetaData._(
       jsName as String?,
@@ -75,6 +105,7 @@ class ClassMetaData {
       int.tryParse('$length'),
       isFunction,
       isRecord,
+      isNativeError,
     );
   }
 
@@ -85,6 +116,7 @@ class ClassMetaData {
     this.length,
     this.isFunction,
     this.isRecord,
+    this.isNativeError,
   );
 
   /// Returns the ID of the class.
@@ -100,18 +132,22 @@ class ClassMetaData {
     try {
       final evalExpression = '''
       function(arg) {
-        const sdkUtils = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk').dart;
-        const classObject = sdkUtils.getReifiedType(arg);
-        const isFunction = classObject instanceof sdkUtils.AbstractFunctionType;
-        const isRecord = classObject instanceof sdkUtils.RecordType;
+        const sdk = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk');
+        const dart = sdk.dart;
+        const interceptors = sdk._interceptors;
+        const classObject = dart.getReifiedType(arg);
+        const isFunction = classObject instanceof dart.AbstractFunctionType;
+        const isRecord = classObject instanceof dart.RecordType;
+        const isNativeError = dart.is(arg, interceptors.NativeError);
         const result = {};
         var name = isFunction ? 'Function' : classObject.name;
 
         result['name'] = name;
-        result['libraryId'] = sdkUtils.getLibraryUri(classObject);
-        result['dartName'] = sdkUtils.typeName(classObject);
+        result['libraryId'] = dart.getLibraryUri(classObject);
+        result['dartName'] = dart.typeName(classObject);
         result['isFunction'] = isFunction;
         result['isRecord'] = isRecord;
+        result['isNativeError'] = isNativeError;
         result['length'] = arg['length'];
 
         if (isRecord) {
@@ -136,9 +172,12 @@ class ClassMetaData {
         dartName: metadata['dartName'],
         isFunction: metadata['isFunction'],
         isRecord: metadata['isRecord'],
+        isNativeError: metadata['isNativeError'],
         length: metadata['length'],
       );
-    } on ChromeDebugException {
+    } on ChromeDebugException catch (e, s) {
+      _logger.fine(
+          'Could not create class metadata for ${remoteObject.json}', e, s);
       return null;
     }
   }
@@ -162,4 +201,8 @@ class ClassMetaData {
 
   /// True if this class refers to a record type.
   bool isRecord;
+
+  /// True is this class refers to a native JS type.
+  /// i.e. inherits from NativeError.
+  bool isNativeError;
 }