Wrap a zone exception handler around our Chrome event handlers (#732)
diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart
index 4977c94..f88295f 100644
--- a/dwds/lib/src/debugging/debugger.dart
+++ b/dwds/lib/src/debugging/debugger.dart
@@ -5,7 +5,8 @@
import 'dart:async';
import 'package:logging/logging.dart';
-import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
+import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
+ hide StackTrace;
import '../../asset_handler.dart';
import '../../dwds.dart' show LogWriter;
@@ -20,8 +21,6 @@
import 'remote_debugger.dart';
import 'sources.dart';
-final _logger = Logger('Debugger');
-
/// Converts from ExceptionPauseMode strings to [PauseState] enums.
///
/// Values defined in:
@@ -33,6 +32,8 @@
};
class Debugger extends Domain {
+ static final logger = Logger('Debugger');
+
final AssetHandler _assetHandler;
final LogWriter _logWriter;
final RemoteDebugger _remoteDebugger;
@@ -173,9 +174,13 @@
// We must add a listener before enabling the debugger otherwise we will
// miss events.
// Allow a null debugger/connection for unit tests.
- _remoteDebugger?.onScriptParsed?.listen(sources.scriptParsed);
- _remoteDebugger?.onPaused?.listen(_pauseHandler);
- _remoteDebugger?.onResumed?.listen(_resumeHandler);
+ runZoned(() {
+ _remoteDebugger?.onScriptParsed?.listen(sources.scriptParsed);
+ _remoteDebugger?.onPaused?.listen(_pauseHandler);
+ _remoteDebugger?.onResumed?.listen(_resumeHandler);
+ }, onError: (e, StackTrace s) {
+ logger.warning('Error handling Chrome event', e, s);
+ });
handleErrorIfPresent(await _remoteDebugger?.sendCommand('Page.enable'));
handleErrorIfPresent(await _remoteDebugger?.enable() as WipResponse);
@@ -445,7 +450,7 @@
_streamNotify('Debug', event);
} on ChromeDebugException catch (e, s) {
if (e.exception.description.contains('require is not defined')) {
- _logger.warning(
+ logger.warning(
'Error handling pause event, app does not appear to be loaded',
e,
s);
diff --git a/dwds/test/debugger_test.dart b/dwds/test/debugger_test.dart
index 71843bf..b0ac91e 100644
--- a/dwds/test/debugger_test.dart
+++ b/dwds/test/debugger_test.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
@TestOn('vm')
+import 'dart:async';
import 'package:dwds/dwds.dart' show ModuleStrategy;
import 'package:dwds/src/debugging/debugger.dart';
import 'package:dwds/src/debugging/inspector.dart';
@@ -11,6 +12,8 @@
import 'package:dwds/src/utilities/shared.dart';
import 'package:source_maps/parser.dart';
import 'package:test/test.dart';
+import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
+ show DebuggerPausedEvent;
import 'fixtures/context.dart';
import 'fixtures/debugger_data.dart';
@@ -20,11 +23,14 @@
AppInspector inspector;
Debugger debugger;
FakeWebkitDebugger webkitDebugger;
+StreamController<DebuggerPausedEvent> pausedController;
void main() async {
setUpAll(() async {
globalModuleStrategy = ModuleStrategy.requireJS;
webkitDebugger = FakeWebkitDebugger();
+ pausedController = StreamController<DebuggerPausedEvent>();
+ webkitDebugger.onPaused = pausedController.stream;
debugger = await Debugger.create(
null,
webkitDebugger,
@@ -65,4 +71,21 @@
var frame1Variables = firstFrame.vars.map((each) => each.name).toList();
expect(frame1Variables, ['a', 'b']);
});
+
+ group('errors', () {
+ setUp(() {
+ // We need to provide an Isolate so that the code doesn't bail out on a null
+ // check before it has a chance to throw.
+ inspector = FakeInspector(fakeIsolate: simpleIsolate);
+ });
+
+ test('errors in the zone are caught and logged', () async {
+ // Add a DebuggerPausedEvent with a null parameter to provoke an error.
+ pausedController.sink.add(DebuggerPausedEvent(null));
+ expect(
+ Debugger.logger.onRecord,
+ emitsThrough(predicate(
+ (log) => log.message == 'Error handling Chrome event')));
+ });
+ });
}
diff --git a/dwds/test/fixtures/fakes.dart b/dwds/test/fixtures/fakes.dart
index 2b5b915..3dfffc7 100644
--- a/dwds/test/fixtures/fakes.dart
+++ b/dwds/test/fixtures/fakes.dart
@@ -20,8 +20,26 @@
import 'debugger_data.dart';
+/// Constructs a trivial Isolate we can use when we need to provide one but
+/// don't want go through initialization.
+Isolate get simpleIsolate => Isolate(
+ id: '1',
+ number: '1',
+ name: 'fake',
+ libraries: [],
+ exceptionPauseMode: 'abc',
+ breakpoints: [],
+ pauseOnExit: false,
+ pauseEvent: null,
+ startTime: 0,
+ livePorts: 0,
+ runnable: false,
+ );
+
class FakeInspector extends Domain implements AppInspector {
- FakeInspector() : super.forInspector();
+ FakeInspector({this.fakeIsolate}) : super.forInspector();
+
+ Isolate fakeIsolate;
@override
Object noSuchMethod(Invocation invocation) {
@@ -45,9 +63,9 @@
@override
Future<ScriptRef> scriptWithId(String scriptId) => null;
@override
- Isolate checkIsolate(String isolateId) => null;
+ Isolate checkIsolate(String isolateId) => fakeIsolate;
@override
- Isolate get isolate => null;
+ Isolate get isolate => fakeIsolate;
@override
IsolateRef get isolateRef => null;
@override
@@ -119,7 +137,7 @@
Stream<GlobalObjectClearedEvent> get onGlobalObjectCleared => null;
@override
- Stream<DebuggerPausedEvent> get onPaused => null;
+ Stream<DebuggerPausedEvent> onPaused;
@override
Stream<DebuggerResumedEvent> get onResumed => null;
