Skip to content

Cleanup creation of ChromeDebugException #1690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/modules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
4 changes: 2 additions & 2 deletions dwds/lib/src/dwds_vm_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/servers/extension_debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
9 changes: 6 additions & 3 deletions dwds/lib/src/services/chrome_debug_exception.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 4 additions & 4 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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>();

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions dwds/lib/src/services/expression_evaluator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down Expand Up @@ -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)');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
33 changes: 9 additions & 24 deletions dwds/lib/src/utilities/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Expand All @@ -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;
}
2 changes: 1 addition & 1 deletion dwds/test/fixtures/context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion dwds/test/fixtures/fakes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class FakeModules implements Modules {
}

@override
Future<String> moduleForlibrary(String libraryUri) {
Future<String> moduleForLibrary(String libraryUri) {
throw UnimplementedError();
}
}
Expand Down
2 changes: 1 addition & 1 deletion dwds/test/location_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ class MockModules implements Modules {
}

@override
Future<String> moduleForlibrary(String libraryUri) {
Future<String> moduleForLibrary(String libraryUri) {
throw UnimplementedError();
}
}