Cleanup creation of ChromeDebugException (#1690)

* Migrate debugging/inspector.dart to null safety

* Migrate expression evaluator to null safety

* format

* Fix failing tests

* Migrate services/chrome_proxu_service.dart to null safety

* Migrate rest of the services and dwds_vm_client to null safety

* Migrate package:dwds to null safety

* Cleanup creation of ChromeDebugException

Author: Anna Gringauze

dwds/lib/src/debugging/debugger.dart

@@ -12,6 +12,7 @@ import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
     hide StackTrace;
 
 import '../loaders/strategy.dart';
+import '../services/chrome_debug_exception.dart';
 import '../utilities/conversions.dart';
 import '../utilities/dart_uri.dart';
 import '../utilities/domain.dart';
@@ -379,7 +380,7 @@ class Debugger extends Domain {
 
   Future<BoundVariable?> _boundVariable(Property property) async {
     // TODO(annagrin): value might be null in the future for variables
-    // optimized by V8. Return appopriate sentinel values for them.
+    // optimized by V8. Return appropriate sentinel values for them.
     if (property.value != null) {
       final value = property.value!;
       // We return one level of properties from this object. Sub-properties are
@@ -721,7 +722,7 @@ class Debugger extends Domain {
     try {
       return await _remoteDebugger.evaluateOnCallFrame(callFrameId, expression);
     } on ExceptionDetails catch (e) {
-      throwChromeDebugException(
+      throw ChromeDebugException(
         e.json,
         evalContents: expression,
       );
@@ -935,7 +936,7 @@ String _prettifyExtension(String member) {
     isSetter = member.substring(0, spaceIndex) == 'set';
     member = member.substring(spaceIndex + 1, member.length);
   } else if (poundIndex >= 0) {
-    // Here member is a tearoff or local property getter/setter.
+    // Here member is a tear-off or local property getter/setter.
     isSetter = member.substring(pipeIndex + 1, poundIndex) == 'set';
     member = member.replaceRange(pipeIndex + 1, poundIndex + 3, '');
   } else {
@@ -949,7 +950,7 @@ String _prettifyExtension(String member) {
   return isSetter ? '$member=' : member;
 }
 
-/// Unescapes a DDC-escaped JS identifier name.
+/// Un-escapes a DDC-escaped JS identifier name.
 ///
 /// Identifier names that contain illegal JS characters are escaped by DDC to a
 /// decimal representation of the symbol's UTF-16 value.

dwds/lib/src/debugging/modules.dart

@@ -52,7 +52,7 @@ class Modules {
     return _sourceToLibrary[serverPath];
   }
 
-  Future<String?> moduleForlibrary(String libraryUri) async {
+  Future<String?> moduleForLibrary(String libraryUri) async {
     await _moduleMemoizer.runOnce(_initializeMapping);
     return _libraryToModule[libraryUri];
   }

dwds/lib/src/dwds_vm_client.dart

@@ -166,7 +166,7 @@ void _processSendEvent(Map<String, dynamic> event,
             _logger.fine('Debugger ready time: $debuggerStartTime ms');
           } else {
             _logger
-                .finest('Debugger and DevTools startup times alredy recorded.'
+                .finest('Debugger and DevTools startup times already recorded.'
                     ' Ignoring $event.');
           }
         } else {
@@ -264,7 +264,7 @@ Future<void> _disableBreakpointsAndResume(
     // at this point:
     //
     // - `getIsolate()` and check for status:
-    //    the app migth still pause on existing breakpoint.
+    //    the app might still pause on existing breakpoint.
     //
     // - `pause()` and wait for `Debug.paused` event:
     //   chrome does not send the `Debug.Paused `notification

dwds/lib/src/servers/extension_debugger.dart

@@ -304,7 +304,7 @@ class ExtensionDebugger implements RemoteDebugger {
 
   Map<String, dynamic> _validateResult(Map<String, dynamic>? result) {
     if (result == null) {
-      throw ChromeDebugException({'result': null});
+      throw ChromeDebugException({'text': 'null result from Chrome Devtools'});
     }
     if (result.containsKey('exceptionDetails')) {
       throw ChromeDebugException(

dwds/lib/src/services/chrome_debug_exception.dart

@@ -13,11 +13,14 @@ class ChromeDebugException extends ExceptionDetails implements Exception {
 
   /// Optional, the stack where exception happened.
   @override
-  final StackTrace? stackTrace;
+  late final StackTrace? stackTrace;
 
   ChromeDebugException(Map<String, dynamic> exceptionDetails,
-      {this.additionalDetails, this.evalContents, this.stackTrace})
-      : super(exceptionDetails);
+      {this.additionalDetails, this.evalContents})
+      : super(exceptionDetails) {
+    final json = exceptionDetails['stackTrace'];
+    stackTrace = json == null ? null : StackTrace(json);
+  }
 
   @override
   String toString() {

dwds/lib/src/services/chrome_proxy_service.dart

@@ -42,7 +42,7 @@ class ChromeProxyService implements VmServiceInterface {
   /// are dynamic and roughly map to chrome tabs.
   final VM _vm;
 
-  /// Signals when isolate is intialized.
+  /// Signals when isolate is initialized.
   Future<void> get isInitialized => _initializedCompleter.future;
   Completer<void> _initializedCompleter = Completer<void>();
 
@@ -165,12 +165,12 @@ class ChromeProxyService implements VmServiceInterface {
     return service;
   }
 
-  /// Initializes metdata in [Locations], [Modules], and [ExpressionCompiler].
+  /// Initializes metadata in [Locations], [Modules], and [ExpressionCompiler].
   Future<void> _initializeEntrypoint(String entrypoint) async {
     _locations.initialize(entrypoint);
     _modules.initialize(entrypoint);
     _skipLists.initialize();
-    // We do not need to wait for compiler dependencies to be udpated as the
+    // We do not need to wait for compiler dependencies to be updated as the
     // [ExpressionEvaluator] is robust to evaluation requests during updates.
     unawaited(_updateCompilerDependencies(entrypoint));
   }
@@ -211,7 +211,7 @@ class ChromeProxyService implements VmServiceInterface {
     }
     // Waiting for the debugger to be ready before initializing the entrypoint.
     //
-    // Note: moving `await debugger` after the `_initalizeEntryPoint` call
+    // Note: moving `await debugger` after the `_initializeEntryPoint` call
     // causes `getcwd` system calls to fail. Since that system call is used
     // in first `Uri.base` call in the expression compiler service isolate,
     // the expression compiler service will fail to start.

dwds/lib/src/services/expression_evaluator.dart

@@ -88,7 +88,7 @@ class ExpressionEvaluator {
       return _createError(ErrorKind.invalidInput, 'no library uri');
     }
 
-    final module = await _modules.moduleForlibrary(libraryUri);
+    final module = await _modules.moduleForLibrary(libraryUri);
     if (module == null) {
       return _createError(ErrorKind.internal, 'no module for $libraryUri');
     }
@@ -187,7 +187,7 @@ class ExpressionEvaluator {
           ErrorKind.internal, 'no libraryUri for $dartSourcePath');
     }
 
-    final module = await _modules.moduleForlibrary(libraryUri.toString());
+    final module = await _modules.moduleForLibrary(libraryUri.toString());
     if (module == null) {
       return _createError(
           ErrorKind.internal, 'no module for $libraryUri ($dartSourcePath)');
@@ -231,7 +231,7 @@ class ExpressionEvaluator {
   RemoteObject _formatCompilationError(String error) {
     // Frontend currently gives a text message including library name
     // and function name on compilation error. Strip this information
-    // since it shows syntetic names that are only used for temporary
+    // since it shows synthetic names that are only used for temporary
     // debug library during expression evaluation.
     //
     // TODO(annagrin): modify frontend to avoid stripping dummy names
@@ -312,7 +312,7 @@ class ExpressionEvaluator {
   /// this stripping code.
   String _maybeStripTryCatch(String jsCode) {
     // Match the wrapping generated by the expression compiler exactly
-    // so the maching does not succeed naturally after the wrapping is
+    // so the matching does not succeed naturally after the wrapping is
     // removed:
     //
     // Expression compiler's wrapping:

dwds/lib/src/utilities/shared.dart

@@ -92,10 +92,9 @@ void serveHttpRequests(Stream<HttpRequest> requests, Handler handler,
 void handleErrorIfPresent(wip.WipResponse? response, {String? evalContents}) {
   final result = response?.result;
   if (result == null) return;
-  final exceptionDetails = result['exceptionDetails'] as Map<String, dynamic>?;
-  if (exceptionDetails != null) {
-    throwChromeDebugException(
-      exceptionDetails,
+  if (result.containsKey('exceptionDetails')) {
+    throw ChromeDebugException(
+      result['exceptionDetails'] as Map<String, dynamic>,
       evalContents: evalContents,
     );
   }
@@ -106,27 +105,13 @@ void handleErrorIfPresent(wip.WipResponse? response, {String? evalContents}) {
 /// result or the result is null.
 Map<String, dynamic> getResultOrHandleError(wip.WipResponse? response,
     {String? evalContents}) {
-  final result = response?.result;
-  final resultValue = result?['result'];
-  final exceptionDetails = result?['exceptionDetails'];
-  if (resultValue == null || exceptionDetails != null) {
-    throwChromeDebugException(
-      exceptionDetails,
+  handleErrorIfPresent(response, evalContents: evalContents);
+  final result = response?.result?['result'];
+  if (result == null) {
+    throw ChromeDebugException(
+      {'text': 'null result from Chrome Devtools'},
       evalContents: evalContents,
     );
   }
-  return resultValue;
-}
-
-Never throwChromeDebugException(Map<String, dynamic>? exceptionDetails,
-    {String? evalContents, Object? additionalDetails}) {
-  final stackTraceDetails = exceptionDetails?['stackTrace'];
-  final stackTrace =
-      stackTraceDetails == null ? null : wip.StackTrace(stackTraceDetails);
-  throw ChromeDebugException(
-    exceptionDetails ?? {'text': 'null result from Chrome Devtools'},
-    evalContents: evalContents,
-    additionalDetails: additionalDetails,
-    stackTrace: stackTrace,
-  );
+  return result;
 }

dwds/test/fixtures/context.dart

@@ -85,7 +85,7 @@ class TestContext {
   ///
   /// Note: build_runner-based setups ignore this setting and read
   /// this value from the ddc debug metadata and pass it to the
-  /// expression compiler worker initialiation API.
+  /// expression compiler worker initialization API.
   ///
   /// TODO(annagrin): Currently setting sound null safety for frontend
   /// server tests fails due to missing sound SDK JavaScript and maps.

dwds/test/fixtures/fakes.dart

@@ -124,7 +124,7 @@ class FakeModules implements Modules {
   }
 
   @override
-  Future<String> moduleForlibrary(String libraryUri) {
+  Future<String> moduleForLibrary(String libraryUri) {
     throw UnimplementedError();
   }
 }

dwds/test/location_test.dart

@@ -263,7 +263,7 @@ class MockModules implements Modules {
   }
 
   @override
-  Future<String> moduleForlibrary(String libraryUri) {
+  Future<String> moduleForLibrary(String libraryUri) {
     throw UnimplementedError();
   }
 }