Wrap a zone exception handler around our Chrome event handlers (#732)

dwds/lib/src/debugging/debugger.dart

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);

dwds/test/debugger_test.dart

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')));
+    });
+  });
 }

dwds/test/fixtures/fakes.dart

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;