some work on how we print exceptions (#929)

* some work on how we print exceptions * use config specific imports * review comments
flutter · Aug 22, 2019 · 2fe32ff · 2fe32ff
1 parent 2b0da23
commit 2fe32ff
Show file tree

Hide file tree

Showing 7 changed files with 142 additions and 65 deletions.
diff --git a/packages/devtools/lib/src/config_specific/allowed_error.dart b/packages/devtools/lib/src/config_specific/allowed_error.dart
@@ -0,0 +1,6 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+export 'allowed_error_default.dart'
+    if (dart.library.html) 'allowed_error_html.dart';
diff --git a/packages/devtools/lib/src/config_specific/allowed_error_default.dart b/packages/devtools/lib/src/config_specific/allowed_error_default.dart
@@ -0,0 +1,15 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/// Catch and print errors from the given future. These errors are part of
+/// normal operation for an app, and don't need to be reported to analytics
+/// (i.e., they're not DevTools crashes).
+Future<T> allowedError<T>(Future<T> future) {
+  return future.catchError((Object error) {
+    final errorLines = error.toString().split('\n');
+    print('[${error.runtimeType}] ${errorLines.first}');
+    print(errorLines.skip(1).join('\n'));
+    print('');
+  });
+}
diff --git a/packages/devtools/lib/src/config_specific/allowed_error_html.dart b/packages/devtools/lib/src/config_specific/allowed_error_html.dart
@@ -0,0 +1,17 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:html';
+
+/// Catch and print errors from the given future. These errors are part of
+/// normal operation for an app, and don't need to be reported to analytics
+/// (i.e., they're not DevTools crashes).
+Future<T> allowedError<T>(Future<T> future) {
+  return future.catchError((Object error) {
+    final errorLines = error.toString().split('\n');
+    window.console.groupCollapsed('[${error.runtimeType}] ${errorLines.first}');
+    window.console.log(errorLines.skip(1).join('\n'));
+    window.console.groupEnd();
+  });
+}
diff --git a/packages/devtools/lib/src/main.dart b/packages/devtools/lib/src/main.dart
@@ -36,17 +36,23 @@ const flutterWebLibraryUri = 'package:flutter_web/src/widgets/binding.dart';
 class PerfToolFramework extends Framework {
   PerfToolFramework() {
     html.window.onError.listen(_gAReportExceptions);
+
     initGlobalUI();
     initTestingModel();
   }
 
   void _gAReportExceptions(html.Event e) {
     final html.ErrorEvent errorEvent = e as html.ErrorEvent;
-    ga.error(
-        '${errorEvent.message}\n'
+
+    final message = '${errorEvent.message}\n'
         '${errorEvent.filename}@${errorEvent.lineno}:${errorEvent.colno}\n'
-        '${errorEvent.error}',
-        true);
+        '${errorEvent.error}';
+
+    // Report exceptions with DevTools to GA.
+    ga.error(message, true);
+
+    // Also write them to the console to aid debugging.
+    print(message);
   }
 
   StatusItem isolateSelectStatus;
@@ -246,7 +252,7 @@ class PerfToolFramework extends Framework {
     // is initialed, and react to those events in the UI. Going forward, we'll
     // want to instead have flutter_tools fire hot reload events, and react to
     // them in the UI. That will mean that our UI will update appropriately
-    // even when other clients (the CLI, and IDE) initial the hot reload.
+    // even when other clients (the CLI, and IDE) initiate the hot reload.
 
     final ActionButton reloadAction = ActionButton(
       _reloadActionId,

diff --git a/packages/devtools/lib/src/timeline/timeline_service.dart b/packages/devtools/lib/src/timeline/timeline_service.dart
@@ -5,6 +5,7 @@
 import 'package:meta/meta.dart';
 import 'package:vm_service/vm_service.dart';
 
+import '../config_specific/allowed_error.dart';
 import '../globals.dart';
 import '../profiler/cpu_profile_service.dart';
 import '../vm_service_wrapper.dart';
@@ -31,7 +32,8 @@ class TimelineService {
   }
 
   void _handleConnectionStart(VmServiceWrapper service) {
-    serviceManager.service.setFlag('profile_period', '$defaultSamplePeriod');
+    allowedError(serviceManager.service
+        .setFlag('profile_period', '$defaultSamplePeriod'));
     serviceManager.service.onEvent('Timeline').listen((Event event) {
       final List<dynamic> list = event.json['timelineEvents'];
       final List<Map<String, dynamic>> events =
@@ -56,9 +58,9 @@ class TimelineService {
     timelineController.timelineData = TimelineData();
 
     await serviceManager.serviceAvailable.future;
-    await serviceManager.service
-        .setVMTimelineFlags(<String>['GC', 'Dart', 'Embedder']);
-    await serviceManager.service.clearVMTimeline();
+    await allowedError(serviceManager.service
+        .setVMTimelineFlags(<String>['GC', 'Dart', 'Embedder']));
+    await allowedError(serviceManager.service.clearVMTimeline());
 
     final Timeline timeline = await serviceManager.service.getVMTimeline();
     final List<dynamic> list = timeline.json['traceEvents'];
@@ -119,12 +121,12 @@ class TimelineService {
       await startTimeline();
     } else if (shouldBeRunning && !isRunning) {
       timelineController.resume();
-      await serviceManager.service
-          .setVMTimelineFlags(<String>['GC', 'Dart', 'Embedder']);
+      await allowedError(serviceManager.service
+          .setVMTimelineFlags(<String>['GC', 'Dart', 'Embedder']));
     } else if (!shouldBeRunning && isRunning) {
       // TODO(devoncarew): turn off the events
       timelineController.pause();
-      await serviceManager.service.setVMTimelineFlags(<String>[]);
+      await allowedError(serviceManager.service.setVMTimelineFlags(<String>[]));
     }
   }
 }
diff --git a/packages/devtools/lib/src/ui/vm_flag_elements.dart b/packages/devtools/lib/src/ui/vm_flag_elements.dart
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 import 'dart:async';
 
+import '../config_specific/allowed_error.dart';
 import '../framework/framework.dart';
 import '../globals.dart';
 import '../messages.dart';
@@ -44,8 +45,8 @@ class ProfileGranularitySelector {
   String _selectedValue;
 
   Future<void> setGranularity() async {
-    return serviceManager.service
-        .setFlag(profilePeriodFlagName, selector.value);
+    return allowedError(
+        serviceManager.service.setFlag(profilePeriodFlagName, selector.value));
   }
 
   void _handleSelect() async {

diff --git a/packages/devtools/web/main.dart b/packages/devtools/web/main.dart
@@ -4,68 +4,98 @@
 
 import 'dart:async';
 import 'dart:html';
+
 import 'package:devtools/src/framework/framework_core.dart';
 import 'package:devtools/src/main.dart';
 import 'package:devtools/src/ui/analytics.dart' as ga;
 import 'package:devtools/src/ui/analytics_platform.dart' as ga_platform;
 import 'package:platform_detect/platform_detect.dart';
 
 void main() {
-  // Need to catch all Dart exceptions - done via an isolate.
-  runZoned(() {
-    // Initialize the core framework.
-    FrameworkCore.init();
+  // Run in a zone in order to catch all Dart exceptions.
+  runZoned(
+    () {
+      // Initialize the core framework.
+      FrameworkCore.init();
 
-    // Load the web app framework.
-    final PerfToolFramework framework = PerfToolFramework();
+      // Load the web app framework.
+      final PerfToolFramework framework = PerfToolFramework();
 
-    // Show the opt-in dialog for collection analytics?
-    try {
-      if (ga.isGtagsEnabled() &
-          (!window.localStorage.containsKey(ga_platform.devToolsProperty()) ||
-              window.localStorage[ga_platform.devToolsProperty()].isEmpty)) {
-        framework.showAnalyticsDialog();
+      // Show the opt-in dialog for collection analytics?
+      try {
+        if (ga.isGtagsEnabled() &
+            (!window.localStorage.containsKey(ga_platform.devToolsProperty()) ||
+                window.localStorage[ga_platform.devToolsProperty()].isEmpty)) {
+          framework.showAnalyticsDialog();
+        }
+      } catch (e) {
+        // If there are errors setting up analytics, write them to the console
+        // but do not prevent DevTools from loading.
+        window.console.error(e);
       }
-    } catch (e) {
-      // If there are errors setting up analytics, write them to the console
-      // but do not prevent DevTools from loading.
-      window.console.error(e);
-    }
-
-    if (!browser.isChrome) {
-      final browserName =
-          // Edge shows up as IE, so we replace it's name to avoid confusion.
-          browser.isInternetExplorer || browser == Browser.UnknownBrowser
-              ? 'an unsupported browser'
-              : browser.name;
-      framework.disableAppWithError(
-        'ERROR: You are running DevTools on $browserName, '
-            'but DevTools only runs on Chrome.',
-        'Reopen this url in a Chrome browser to use DevTools.',
-      );
-      return;
-    }
 
-    FrameworkCore.initVmService(errorReporter: (String title, dynamic error) {
-      framework.showError(title, error);
-    }).then((bool connected) {
-      if (!connected) {
-        framework.showConnectionDialog();
-        framework.showSnapshotMessage();
-        // Clear the main element so it stops displaying "Loading..."
-        // TODO(jacobr): display a message explaining how to launch a Flutter
-        // application from the command line and connect to it with DevTools.
-        framework.mainElement.clear();
+      if (!browser.isChrome) {
+        final browserName =
+            // Edge shows up as IE, so we replace it's name to avoid confusion.
+            browser.isInternetExplorer || browser == Browser.UnknownBrowser
+                ? 'an unsupported browser'
+                : browser.name;
+        framework.disableAppWithError(
+          'ERROR: You are running DevTools on $browserName, '
+              'but DevTools only runs on Chrome.',
+          'Reopen this url in a Chrome browser to use DevTools.',
+        );
+        return;
       }
-    });
 
-    framework.loadScreenFromLocation();
-  }, onError: (error, stack) {
-    // Report exceptions with DevTools to GA, any user's Flutter app exceptions
-    // are not collected.
-    ga.error('$error\n$stack', true);
-    // Also write them to the console to aid debugging (rather than silently
-    // failing to load).
-    print('$error\n$stack');
-  });
+      FrameworkCore.initVmService(errorReporter: (String title, dynamic error) {
+        framework.showError(title, error);
+      }).then((bool connected) {
+        if (!connected) {
+          framework.showConnectionDialog();
+          framework.showSnapshotMessage();
+          // Clear the main element so it stops displaying "Loading..."
+          // TODO(jacobr): display a message explaining how to launch a Flutter
+          // application from the command line and connect to it with DevTools.
+          framework.mainElement.clear();
+        }
+      });
+
+      framework.loadScreenFromLocation();
+    },
+    zoneSpecification: const ZoneSpecification(
+      handleUncaughtError: _handleUncaughtError,
+    ),
+  );
+}
+
+void _handleUncaughtError(
+  Zone self,
+  ZoneDelegate parent,
+  Zone zone,
+  Object error,
+  StackTrace stackTrace,
+) {
+  // TODO(devoncarew): `stackTrace` always seems to be null.
+
+  // Report exceptions with DevTools to GA; user's Flutter app exceptions are
+  // not collected.
+  ga.error('$error\n${stackTrace ?? ''}'.trim(), true);
+
+  final Console console = window.console;
+
+  // Also write them to the console to aid debugging.
+  final errorLines = error.toString().split('\n');
+  console.groupCollapsed(
+      'DevTools exception: [${error.runtimeType}] ${errorLines.first}');
+  console.log(errorLines.skip(1).join('\n'));
+
+  if (stackTrace != null) {
+    if (errorLines.length > 1) {
+      console.log('\n');
+    }
+    console.log(stackTrace.toString().trim());
+  }
+
+  console.groupEnd();
 }