diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index dea20660a..a848f9794 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,3 +1,10 @@ +## 11.2.3-dev + +- Fix race causing intermittent `Aww, snap` errors on starting debugger + with multiple breakpoints in source. +- Fix needing chrome to be focus in order to wait for the isolate to + exit on hot restart. + ## 11.2.2 - Depend on `dds` version `2.1.1`. diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index b230d613b..8194f0568 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -9,6 +9,7 @@ import 'dart:math' as math; import 'package:logging/logging.dart'; import 'package:meta/meta.dart'; +import 'package:pool/pool.dart'; import 'package:vm_service/vm_service.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' hide StackTrace; @@ -187,6 +188,7 @@ class Debugger extends Domain { runZonedGuarded(() { _remoteDebugger?.onPaused?.listen(_pauseHandler); _remoteDebugger?.onResumed?.listen(_resumeHandler); + _remoteDebugger?.onTargetCrashed?.listen(_crashHandler); }, (e, StackTrace s) { logger.warning('Error handling Chrome event', e, s); }); @@ -580,6 +582,23 @@ class Debugger extends Domain { _streamNotify('Debug', event); } + /// Handles targetCrashed events coming from the Chrome connection. + Future _crashHandler(TargetCrashedEvent _) async { + // We can receive a resume event in the middle of a reload which will result + // in a null isolate. + var isolate = inspector?.isolate; + if (isolate == null) return; + + stackComputer = null; + var event = Event( + kind: EventKind.kIsolateExit, + timestamp: DateTime.now().millisecondsSinceEpoch, + isolate: inspector.isolateRef); + isolate.pauseEvent = event; + _streamNotify('Isolate', event); + logger.severe('Target crashed!'); + } + /// Evaluate [expression] by calling Chrome's Runtime.evaluate Future evaluate(String expression) async { try { @@ -655,6 +674,8 @@ class _Breakpoints extends Domain { final _bpByDartId = >{}; + final _pool = Pool(1); + final Locations locations; final RemoteDebugger remoteDebugger; @@ -722,12 +743,16 @@ class _Breakpoints extends Domain { // The module can be loaded from a nested path and contain an ETAG suffix. var urlRegex = '.*${location.jsLocation.module}.*'; - var response = await remoteDebugger - .sendCommand('Debugger.setBreakpointByUrl', params: { - 'urlRegex': urlRegex, - 'lineNumber': location.jsLocation.line - 1, + // Prevent `Aww, snap!` errors when setting multiple breakpoints + // simultaneously by serializing the requests. + return _pool.withResource(() async { + var response = await remoteDebugger + .sendCommand('Debugger.setBreakpointByUrl', params: { + 'urlRegex': urlRegex, + 'lineNumber': location.jsLocation.line - 1, + }); + return response.result['breakpointId'] as String; }); - return response.result['breakpointId'] as String; } /// Records the internal Dart <=> JS breakpoint id mapping and adds the diff --git a/dwds/lib/src/debugging/remote_debugger.dart b/dwds/lib/src/debugging/remote_debugger.dart index 59671293f..6520c8e27 100644 --- a/dwds/lib/src/debugging/remote_debugger.dart +++ b/dwds/lib/src/debugging/remote_debugger.dart @@ -6,6 +6,10 @@ import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; +class TargetCrashedEvent extends WipEvent { + TargetCrashedEvent(Map json) : super(json); +} + /// A generic debugger used in remote debugging. abstract class RemoteDebugger { Stream get onConsoleAPICalled; @@ -20,6 +24,8 @@ abstract class RemoteDebugger { Stream get onScriptParsed; + Stream get onTargetCrashed; + Map get scripts; Stream get onClose; diff --git a/dwds/lib/src/debugging/webkit_debugger.dart b/dwds/lib/src/debugging/webkit_debugger.dart index 7e981cda7..e9b474b04 100644 --- a/dwds/lib/src/debugging/webkit_debugger.dart +++ b/dwds/lib/src/debugging/webkit_debugger.dart @@ -112,6 +112,11 @@ class WebkitDebugger implements RemoteDebugger { @override Stream get onScriptParsed => _wipDebugger.onScriptParsed; + @override + Stream get onTargetCrashed => _wipDebugger.eventStream( + 'Inspector.targetCrashed', + (WipEvent event) => TargetCrashedEvent(event.json)); + @override Map get scripts => _wipDebugger.scripts; diff --git a/dwds/lib/src/dwds_vm_client.dart b/dwds/lib/src/dwds_vm_client.dart index 558f8e90b..82b777369 100644 --- a/dwds/lib/src/dwds_vm_client.dart +++ b/dwds/lib/src/dwds_vm_client.dart @@ -194,22 +194,28 @@ Future _disableBreakpointsAndResume( if (vm.isolates.isEmpty) throw StateError('No active isolate to resume.'); var isolateRef = vm.isolates.first; - // Pause the app to prevent it from hitting a breakpoint - // during hot restart and stalling hot restart execution. - // Then wait for the app to pause or to hit a breakpoint. - var debug = chromeProxyService.onEvent('Debug').firstWhere((event) => - event.kind == EventKind.kPauseInterrupted || - event.kind == EventKind.kPauseBreakpoint); - - await client.pause(isolateRef.id); - - var isolate = await client.getIsolate(isolateRef.id); - if (isolate.pauseEvent.kind != EventKind.kPauseInterrupted && - isolate.pauseEvent.kind != EventKind.kPauseBreakpoint) { - await debug; - } - await chromeProxyService.disableBreakpoints(); - await client.resume(isolateRef.id); + try { + // Any checks for paused status result in race conditions or hangs + // at this point: + // + // - `getIsolate()` and check for status: + // the app migth still pause on existing breakpoint. + // + // - `pause()` and wait for `Debug.paused` event: + // chrome does not send the `Debug.Paused `notification + // without shifting focus to chrome. + // + // Instead, just try resuming and + // ignore failures indicating that the app is already running: + // + // WipError -32000 Can only perform operation while paused. + await client.resume(isolateRef.id); + } on RPCError catch (e, s) { + if (!e.message.contains('Can only perform operation while paused')) { + _logger.severe('Hot restart failed to resume exiting isolate', e, s); + rethrow; + } + } _logger.info('Successfully disabled breakpoints and resumed the isolate'); } diff --git a/dwds/lib/src/handlers/dev_handler.dart b/dwds/lib/src/handlers/dev_handler.dart index 2a036ee18..9dc66c436 100644 --- a/dwds/lib/src/handlers/dev_handler.dart +++ b/dwds/lib/src/handlers/dev_handler.dart @@ -142,6 +142,9 @@ class DevHandler { tabConnection.onReceive.listen((message) { _log(' wip', '<== $message'); }); + tabConnection.onNotification.listen((message) { + _log(' wip', '<== $message'); + }); } var contextIds = tabConnection.runtime.onExecutionContextCreated .map((context) => context.id) diff --git a/dwds/lib/src/servers/extension_debugger.dart b/dwds/lib/src/servers/extension_debugger.dart index 7bb069e5e..941fca290 100644 --- a/dwds/lib/src/servers/extension_debugger.dart +++ b/dwds/lib/src/servers/extension_debugger.dart @@ -279,6 +279,11 @@ class ExtensionDebugger implements RemoteDebugger { 'Debugger.scriptParsed', (WipEvent event) => ScriptParsedEvent(event.json)); + @override + Stream get onTargetCrashed => eventStream( + 'Inspector.targetCrashed', + (WipEvent event) => TargetCrashedEvent(event.json)); + @override Map get scripts => UnmodifiableMapView(_scripts); diff --git a/dwds/lib/src/version.dart b/dwds/lib/src/version.dart index 97e914528..0bfc9c790 100644 --- a/dwds/lib/src/version.dart +++ b/dwds/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '11.2.2'; +const packageVersion = '11.2.3-dev'; diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index 707931074..3c941bf49 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -1,6 +1,6 @@ name: dwds # Every time this changes you need to run `pub run build_runner build`. -version: 11.2.2 +version: 11.2.3-dev homepage: https://github.com/dart-lang/webdev/tree/master/dwds description: >- A service that proxies between the Chrome debug protocol and the Dart VM diff --git a/dwds/test/fixtures/fakes.dart b/dwds/test/fixtures/fakes.dart index cc9ab3da4..36227f6fc 100644 --- a/dwds/test/fixtures/fakes.dart +++ b/dwds/test/fixtures/fakes.dart @@ -11,6 +11,7 @@ import 'package:dwds/src/debugging/execution_context.dart'; import 'package:dwds/src/debugging/inspector.dart'; import 'package:dwds/src/debugging/instance.dart'; import 'package:dwds/src/debugging/modules.dart'; +import 'package:dwds/src/debugging/remote_debugger.dart'; import 'package:dwds/src/debugging/webkit_debugger.dart'; import 'package:dwds/src/loaders/strategy.dart'; import 'package:dwds/src/utilities/domain.dart'; @@ -174,6 +175,9 @@ class FakeWebkitDebugger implements WebkitDebugger { @override Stream get onScriptParsed => null; + @override + Stream get onTargetCrashed => null; + @override Future pause() => null;