[firebase_crashlytics] Fix Printing, README, Changelog & log FlutterE…

…rrorDetails.informationCollector (firebase#1912)

packages/firebase_crashlytics/CHANGELOG.md

@@ -1,6 +1,17 @@
+## 0.1.0+1
+
+* Added additional exception information from the Flutter framework to the reports.
+* Refactored debug printing of exceptions to be human-readable.
+* Passing `null` stack traces is now supported.
+* Added the "Error reported to Crashlytics." print statement that was previously missing.
+* Updated `README.md` to include both the breaking change from `0.1.0` and the newly added
+  `recordError` function in the setup section.
+* Adjusted `README.md` formatting.
+* Fixed `recordFlutterError` method name in the `0.1.0` changelog entry.
+
 ## 0.1.0
 
-* **Breaking Change** Renamed `onError` to `reportFlutterError`.
+* **Breaking Change** Renamed `onError` to `recordFlutterError`.
 * Added `recordError` method for errors caught using `runZoned`'s `onError`.
 
 ## 0.0.4+12

packages/firebase_crashlytics/README.md

@@ -11,13 +11,14 @@ For Flutter plugins for other Firebase products, see [FlutterFire.md](https://gi
 ## Usage
 
 ### Import the firebase_crashlytics plugin
-To use the firebase_crashlytics plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/firebase_crashlytics#pub-pkg-tab-installing).
+
+To use the `firebase_crashlytics` plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/firebase_crashlytics#pub-pkg-tab-installing).
 
 ### Android integration
 
-Enable the Google services by configuring the Gradle scripts as such.
+Enable the Google services by configuring the Gradle scripts as such:
 
-1. Add Fabric repository to the `[project]/android/build.gradle` file.
+1. Add the Fabric repository to the `[project]/android/build.gradle` file.
 ```
 repositories {
   google()
@@ -29,7 +30,7 @@ repositories {
 }
 ```
 
-2. Add the classpaths to the `[project]/android/build.gradle` file.
+2. Add the following classpaths to the `[project]/android/build.gradle` file.
 ```gradle
 dependencies {
   // Example existing classpath
@@ -41,33 +42,33 @@ dependencies {
 }
 ```
 
-2. Add the apply plugins to the `[project]/android/app/build.gradle` file.
+2. Apply the following plugins in the `[project]/android/app/build.gradle` file.
 ```gradle
 // ADD THIS AT THE BOTTOM
 apply plugin: 'io.fabric'
 apply plugin: 'com.google.gms.google-services'
 ```
 
-*Note:* If this section is not completed you will get an error like this:
+*Note:* If this section is not completed, you will get an error like this:
 ```
 java.lang.IllegalStateException:
 Default FirebaseApp is not initialized in this process [package name].
 Make sure to call FirebaseApp.initializeApp(Context) first.
 ```
 
 *Note:* When you are debugging on Android, use a device or AVD with Google Play services.
-Otherwise you will not be able to use Firebase Crashlytics.
+Otherwise, you will not be able to use Firebase Crashlytics.
 
 ### iOS Integration
 
-Add the Crashlytics run scripts
+Add the Crashlytics run scripts:
 
-1. From Xcode select Runner from the project navigation.
-1. Select the Build Phases tab.
-1. Click + Add a new build phase, and select New Run Script Phase.
+1. From Xcode select `Runner` from the project navigation.
+1. Select the `Build Phases` tab.
+1. Click `+ Add a new build phase`, and select `New Run Script Phase`.
 1. Add `${PODS_ROOT}/Fabric/run` to the `Type a script...` text box.
-1. If on Xcode 10 Add your app's built Info.plist location to the Build Phase's Input Files field.
-Eg: `$(BUILT_PRODUCTS_DIR)/$(INFOPLIST_PATH)`
+1. If you are using Xcode 10, add the location of `Info.plist`, built by your app, to the `Build Phase's Input Files` field.  
+   E.g.: `$(BUILT_PRODUCTS_DIR)/$(INFOPLIST_PATH)`
 
 ### Use the plugin
 
@@ -85,20 +86,32 @@ void main() {
   // development.
   Crashlytics.instance.enableInDevMode = true;
 
-  // Pass all uncaught errors to Crashlytics.
-  FlutterError.onError = (FlutterErrorDetails details) {
-    Crashlytics.instance.onError(details);
-  };
+  // Pass all uncaught errors from the framework to Crashlytics.
+  FlutterError.onError = Crashlytics.instance.recordFlutterError;
+  
   runApp(MyApp());
 }
 ```
 
+Overriding `FlutterError.onError` with `Crashlytics.instance.recordFlutterError`  will automatically catch all 
+errors that are thrown from within the Flutter framework.  
+If you want to catch errors that occur in `runZoned`, 
+you can supply `Crashlytics.instance.recordError` to the `onError` parameter:
+```dart
+runZoned<Future<void>>(() async {
+    // ...
+  }, onError: Crashlytics.instance.recordError);
+``` 
+
 ## Result
 
 If an error is caught, you should see the following messages in your logs:
 ```
-flutter: Error caught by Crashlytics plugin:
-...
+flutter: Flutter error caught by Crashlytics plugin:
+// OR if you use recordError for runZoned:
+flutter: Error caught by Crashlytics plugin <recordError>:
+// Exception, context, information, and stack trace in debug mode
+// OR if not in debug mode:
 flutter: Error reported to Crashlytics.
 ```
 
@@ -107,7 +120,7 @@ flutter: Error reported to Crashlytics.
 ## Example
 
 See the [example application](https://github.com/flutter/plugins/tree/master/packages/firebase_crashlytics/example) source
-for a complete sample app using the Firebase Crashlytics.
+for a complete sample app using `firebase_crashlytics`.
 
 ## Issues and feedback
 

packages/firebase_crashlytics/android/src/main/java/io/flutter/plugins/firebase/crashlytics/firebasecrashlytics/FirebaseCrashlyticsPlugin.java

@@ -70,6 +70,15 @@ public void onMethodCall(MethodCall call, Result result) {
       exception.setStackTrace(elements.toArray(new StackTraceElement[elements.size()]));
 
       Crashlytics.setString("exception", (String) call.argument("exception"));
+
+      // Set a "reason" (to match iOS) to show where the exception was thrown.
+      final String context = call.argument("context");
+      if (context != null) Crashlytics.setString("reason", "thrown " + context);
+
+      // Log information.
+      final String information = call.argument("information");
+      if (information != null && !information.isEmpty()) Crashlytics.log(information);
+
       Crashlytics.logException(exception);
       result.success("Error reported to Crashlytics.");
     } else if (call.method.equals("Crashlytics#isDebuggable")) {

packages/firebase_crashlytics/example/test_driver/crashlytics.dart

@@ -24,11 +24,13 @@ void main() {
     crashlytics.setDouble('testDouble', 42.0);
     crashlytics.setString('testString', 'bar');
     Crashlytics.instance.log('testing');
-    await crashlytics.recordFlutterError(
-      FlutterErrorDetails(
+    await crashlytics.recordFlutterError(FlutterErrorDetails(
         exception: 'testing',
         stack: StackTrace.fromString(''),
-      ),
-    );
+        context: DiagnosticsNode.message('during testing'),
+        informationCollector: () => <DiagnosticsNode>[
+              DiagnosticsNode.message('testing'),
+              DiagnosticsNode.message('information'),
+            ]));
   });
 }

packages/firebase_crashlytics/ios/Classes/FirebaseCrashlyticsPlugin.m

@@ -59,15 +59,28 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result
       }
     }
 
+    // Add additional information from the Flutter framework to the exception reported in
+    // Crashlytics. Using CLSLog instead of CLS_LOG to try to avoid the automatic inclusion of the
+    // line number. It also ensures that the log is only written to Crashlytics and not also to the
+    // offline log as explained here:
+    // https://support.crashlytics.com/knowledgebase/articles/92519-how-do-i-use-logging
+    // Although, that would only happen in debug mode, which this method call is never called in.
+    NSString *information = call.arguments[@"information"];
+    if ([information length] != 0) {
+      CLSLog(information);
+    }
+
     // Report crash.
     NSArray *errorElements = call.arguments[@"stackTraceElements"];
     NSMutableArray *frames = [NSMutableArray array];
     for (NSDictionary *errorElement in errorElements) {
       [frames addObject:[self generateFrame:errorElement]];
     }
-    [[Crashlytics sharedInstance] recordCustomExceptionName:call.arguments[@"exception"]
-                                                     reason:call.arguments[@"context"]
-                                                 frameArray:frames];
+    [[Crashlytics sharedInstance]
+        recordCustomExceptionName:call.arguments[@"exception"]
+                           reason:[NSString
+                                      stringWithFormat:@"thrown %s", call.arguments[@"context"]]
+                       frameArray:frames];
     result(@"Error reported to Crashlytics.");
   } else if ([@"Crashlytics#isDebuggable" isEqualToString:call.method]) {
     result([NSNumber numberWithBool:[Crashlytics sharedInstance].debugMode]);

packages/firebase_crashlytics/lib/src/firebase_crashlytics.dart

@@ -30,7 +30,10 @@ class Crashlytics {
     print('Flutter error caught by Crashlytics plugin:');
 
     _recordError(details.exceptionAsString(), details.stack,
-        context: details.context);
+        context: details.context,
+        information: details.informationCollector == null
+            ? null
+            : details.informationCollector());
   }
 
   /// Submits a report of a non-fatal error.
@@ -165,6 +168,12 @@ class Crashlytics {
           'line': lineNumber,
         };
 
+        // The next section would throw an exception in some cases if there was no stop here.
+        if (lineParts.length < 3) {
+          elements.add(element);
+          continue;
+        }
+
         if (lineParts[2].contains(".")) {
           final String className =
               lineParts[2].substring(0, lineParts[2].indexOf(".")).trim();
@@ -185,29 +194,65 @@ class Crashlytics {
     return elements;
   }
 
+  // On top of the default exception components, [information] can be passed as well.
+  // This allows the developer to get a better understanding of exceptions thrown
+  // by the Flutter framework. [FlutterErrorDetails] often explain why an exception
+  // occurred and give useful background information in [FlutterErrorDetails.informationCollector].
+  // Crashlytics will log this information in addition to the stack trace.
+  // If [information] is `null` or empty, it will be ignored.
   Future<void> _recordError(dynamic exception, StackTrace stack,
-      {dynamic context}) async {
+      {dynamic context, Iterable<DiagnosticsNode> information}) async {
     bool inDebugMode = false;
     if (!enableInDevMode) {
       assert(inDebugMode = true);
     }
 
+    final String _information = (information == null || information.isEmpty)
+        ? ''
+        : (StringBuffer()..writeAll(information, '\n')).toString();
+
     if (inDebugMode && !enableInDevMode) {
-      print(Trace.format(stack));
+      // If available, give context to the exception.
+      if (context != null)
+        print('The following exception was thrown $context:');
+
+      // Need to print the exception to explain why the exception was thrown.
+      print(exception);
+
+      // Print information provided by the Flutter framework about the exception.
+      if (_information.isNotEmpty) print('\n$_information');
+
+      // Not using Trace.format here to stick to the default stack trace format
+      // that Flutter developers are used to seeing.
+      if (stack != null) print('\n$stack');
     } else {
-      // Report error
+      // The stack trace can be null. To avoid the following exception:
+      // Invalid argument(s): Cannot create a Trace from null.
+      // To avoid that exception, we can check for null and provide an empty stack trace.
+      stack ??= StackTrace.fromString('');
+
+      // Report error.
       final List<String> stackTraceLines =
           Trace.format(stack).trimRight().split('\n');
       final List<Map<String, String>> stackTraceElements =
           getStackTraceElements(stackTraceLines);
-      await channel
-          .invokeMethod<dynamic>('Crashlytics#onError', <String, dynamic>{
+
+      // The context is a string that "should be in a form that will make sense in
+      // English when following the word 'thrown'" according to the documentation for
+      // [FlutterErrorDetails.context]. It is displayed to the user on Crashlytics
+      // as the "reason", which is forced by iOS, with the "thrown" prefix added.
+      final String result = await channel
+          .invokeMethod<String>('Crashlytics#onError', <String, dynamic>{
         'exception': "${exception.toString()}",
         'context': '$context',
+        'information': _information,
         'stackTraceElements': stackTraceElements,
         'logs': _logs.toList(),
         'keys': _prepareKeys(),
       });
+
+      // Print result.
+      print(result);
     }
   }
 }

packages/firebase_crashlytics/pubspec.yaml

@@ -2,7 +2,7 @@ name: firebase_crashlytics
 description:
   Flutter plugin for Firebase Crashlytics. It reports uncaught errors to the
   Firebase console.
-version: 0.1.0
+version: 0.1.0+1
 author: Flutter Team <[email protected]>
 homepage: https://github.com/flutter/plugins/tree/master/packages/firebase_crashlytics
 

packages/firebase_crashlytics/test/firebase_crashlytics_test.dart

@@ -16,6 +16,8 @@ void main() {
           .setMockMethodCallHandler((MethodCall methodCall) async {
         log.add(methodCall);
         switch (methodCall.method) {
+          case 'Crashlytics#onError':
+            return 'Error reported to Crashlytics.';
           case 'Crashlytics#isDebuggable':
             return true;
           case 'Crashlytics#setUserEmail':
@@ -38,6 +40,10 @@ void main() {
         exception: 'foo exception',
         stack: StackTrace.current,
         library: 'foo library',
+        informationCollector: () => <DiagnosticsNode>[
+          DiagnosticsNode.message('test message'),
+          DiagnosticsNode.message('second message'),
+        ],
         context: ErrorDescription('foo context'),
       );
       crashlytics.enableInDevMode = true;
@@ -50,6 +56,7 @@ void main() {
       expect(log[0].method, 'Crashlytics#onError');
       expect(log[0].arguments['exception'], 'foo exception');
       expect(log[0].arguments['context'], 'foo context');
+      expect(log[0].arguments['information'], 'test message\nsecond message');
       expect(log[0].arguments['logs'], isNotEmpty);
       expect(log[0].arguments['logs'], contains('foo'));
       expect(log[0].arguments['keys'][0]['key'], 'testBool');