Add custom keys/values when recording non-fatals

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java

@@ -50,7 +50,9 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.TreeSet;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
@@ -199,15 +201,16 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal()
     final Thread thread = Thread.currentThread();
     final Exception nonFatal = new RuntimeException("Non-fatal");
     final CrashlyticsController controller = createController();
+    final Map<String, String> extraInfo = new HashMap<>();
 
     when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
         .thenReturn(new TreeSet<>(Collections.singleton(sessionId)));
 
-    controller.writeNonFatalException(thread, nonFatal);
+    controller.writeNonFatalException(thread, nonFatal, extraInfo);
     controller.doCloseSessions(testSettingsProvider);
 
     verify(mockSessionReportingCoordinator)
-        .persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong());
+        .persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong(), eq(extraInfo));
   }
 
   public void testFatalException_callsSessionReportingCoordinatorPersistFatal() throws Exception {
@@ -347,7 +350,17 @@ public void testLoggedExceptionsAfterCrashOk() {
         testSettingsProvider, Thread.currentThread(), new RuntimeException());
 
     // This should not throw.
-    controller.writeNonFatalException(Thread.currentThread(), new RuntimeException());
+    controller.writeNonFatalException(Thread.currentThread(), new RuntimeException(), null);
+
+    Map<String, String> extraInfo = new HashMap<>();
+    extraInfo.put("someKey", "someValue");
+    extraInfo.put("someOtherKey", "someOtherValue");
+
+    // This should also not throw.
+    controller.writeNonFatalException(
+            Thread.currentThread(),
+            new RuntimeException(),
+            extraInfo);
   }
 
   /**

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java

@@ -123,7 +123,7 @@ public void testNonFatalEvent_persistsNormalPriorityEventWithoutAllThreadsForSes
     mockEventInteractions();
 
     reportingCoordinator.onBeginSession(sessionId, timestamp);
-    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, null);
 
     final boolean expectedAllThreads = false;
     final boolean expectedHighPriority = false;
@@ -146,7 +146,7 @@ public void testNonFatalEvent_addsLogsToEvent() {
     when(logFileManager.getLogString()).thenReturn(testLog);
 
     reportingCoordinator.onBeginSession(sessionId, timestamp);
-    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, null);
 
     verify(mockEventBuilder)
         .setLog(CrashlyticsReport.Session.Event.Log.builder().setContent(testLog).build());
@@ -165,7 +165,7 @@ public void testNonFatalEvent_addsNoLogsToEventWhenNoneAvailable() {
     when(logFileManager.getLogString()).thenReturn(null);
 
     reportingCoordinator.onBeginSession(sessionId, timestamp);
-    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, null);
 
     verify(mockEventBuilder, never()).setLog(any(CrashlyticsReport.Session.Event.Log.class));
     verify(mockEventBuilder).build();
@@ -218,36 +218,126 @@ public void testNonFatalEvent_addsSortedKeysToEvent() {
 
     final String sessionId = "testSessionId";
 
-    final String testKey1 = "testKey1";
-    final String testValue1 = "testValue1";
-    final String testKey2 = "testKey2";
-    final String testValue2 = "testValue2";
+    final String internalKey1 = "internalKey1";
+    final String internalValue1 = "internalValue1";
+    final String internalKey2 = "internalKey2";
+    final String internalValue2 = "internalValue2";
 
-    final Map<String, String> attributes = new HashMap<>();
-    attributes.put(testKey1, testValue1);
-    attributes.put(testKey2, testValue2);
+    final String customKey1 = "customKey1";
+    final String customValue1 = "customValue1";
+    final String customKey2 = "customKey2";
+    final String customValue2 = "customValue2";
+
+    final Map<String, String> internalKeys = new HashMap<>();
+    internalKeys.put(internalKey1, internalValue1);
+    internalKeys.put(internalKey2, internalValue2);
+
+    final Map<String, String> customAttributes = new HashMap<>();
+    customAttributes.put(customKey1, customValue1);
+    customAttributes.put(customKey2, customValue2);
+
+    final CustomAttribute internalAttribute1 =
+            CustomAttribute.builder().setKey(internalKey1).setValue(internalValue1).build();
+    final CustomAttribute internalAttribute2 =
+            CustomAttribute.builder().setKey(internalKey2).setValue(internalValue2).build();
 
     final CustomAttribute customAttribute1 =
-        CustomAttribute.builder().setKey(testKey1).setValue(testValue1).build();
+            CustomAttribute.builder().setKey(customKey1).setValue(customValue1).build();
     final CustomAttribute customAttribute2 =
-        CustomAttribute.builder().setKey(testKey2).setValue(testValue2).build();
+            CustomAttribute.builder().setKey(customKey2).setValue(customValue2).build();
+
+    final List<CustomAttribute> expectedInternalKeys = new ArrayList<>();
+    expectedInternalKeys.add(internalAttribute1);
+    expectedInternalKeys.add(internalAttribute2);
 
     final List<CustomAttribute> expectedCustomAttributes = new ArrayList<>();
     expectedCustomAttributes.add(customAttribute1);
     expectedCustomAttributes.add(customAttribute2);
 
-    when(reportMetadata.getCustomKeys()).thenReturn(attributes);
-    when(reportMetadata.getInternalKeys()).thenReturn(attributes);
+    when(reportMetadata.getCustomKeys()).thenReturn(customAttributes);
+    when(reportMetadata.getInternalKeys()).thenReturn(internalKeys);
 
     reportingCoordinator.onBeginSession(sessionId, timestamp);
-    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, null);
 
-    verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
-    verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
-    verify(mockEventAppBuilder).build();
-    verify(mockEventBuilder).setApp(mockEventApp);
-    verify(mockEventBuilder).build();
-    verify(logFileManager, never()).clearLog();
+    verifySortedKeysAddedToEvent(expectedCustomAttributes, expectedInternalKeys);
+  }
+
+  @Test
+  public void testNonFatalEvent_addsSortedKeysAndExtraInfoToEvent() {
+    final long timestamp = System.currentTimeMillis();
+
+    mockEventInteractions();
+
+    final String sessionId = "testSessionId";
+
+    final String internalKey1 = "internalKey1";
+    final String internalValue1 = "internalValue1";
+    final String internalKey2 = "internalKey2";
+    final String internalValue2 = "internalValue2";
+
+    final String customKey1 = "customKey1";
+    final String customValue1 = "customValue1";
+    final String customKey2 = "customKey2";
+    final String customValue2 = "customValue2";
+    final String customOverrideKey = "customOverrideKey";
+    final String customOverrideValueOriginal = "customOverrideValueOriginal";
+    final String customOverrideValueOverridden = "customOverrideValueOverridden";
+
+    final String extraInfoKey1 = "extraInfoKey1";
+    final String extraInfoValue1 = "extraInfoValue1";
+    final String extraInfoKey2 = "extraInfoKey2";
+    final String extraInfoValue2 = "extraInfoValue2";
+
+    final Map<String, String> internalKeys = new HashMap<>();
+    internalKeys.put(internalKey1, internalValue1);
+    internalKeys.put(internalKey2, internalValue2);
+
+    final Map<String, String> customAttributes = new HashMap<>();
+    customAttributes.put(customKey1, customValue1);
+    customAttributes.put(customKey2, customValue2);
+    customAttributes.put(customOverrideKey, customOverrideValueOriginal);
+
+    final Map<String, String> extraInfo = new HashMap<>();
+    extraInfo.put(extraInfoKey1, extraInfoValue1);
+    extraInfo.put(extraInfoKey2, extraInfoValue2);
+    extraInfo.put(customOverrideKey, customOverrideValueOverridden);
+
+    final CustomAttribute internalAttribute1 =
+            CustomAttribute.builder().setKey(internalKey1).setValue(internalValue1).build();
+    final CustomAttribute internalAttribute2 =
+            CustomAttribute.builder().setKey(internalKey2).setValue(internalValue2).build();
+
+    final CustomAttribute customAttribute1 =
+            CustomAttribute.builder().setKey(customKey1).setValue(customValue1).build();
+    final CustomAttribute customAttribute2 =
+            CustomAttribute.builder().setKey(customKey2).setValue(customValue2).build();
+    final CustomAttribute customAttributeOverride =
+            CustomAttribute.builder().setKey(customOverrideKey).setValue(customOverrideValueOverridden).build();
+
+    final CustomAttribute extraInfoAttribute1 =
+            CustomAttribute.builder().setKey(extraInfoKey1).setValue(extraInfoValue1).build();
+    final CustomAttribute extraInfoAttribute2 =
+            CustomAttribute.builder().setKey(extraInfoKey2).setValue(extraInfoValue2).build();
+
+    final List<CustomAttribute> expectedInternalKeys = new ArrayList<>();
+    expectedInternalKeys.add(internalAttribute1);
+    expectedInternalKeys.add(internalAttribute2);
+
+    final List<CustomAttribute> expectedCustomAttributes = new ArrayList<>();
+    expectedCustomAttributes.add(customAttribute1);
+    expectedCustomAttributes.add(customAttribute2);
+    expectedCustomAttributes.add(customAttributeOverride);
+    expectedCustomAttributes.add(extraInfoAttribute1);
+    expectedCustomAttributes.add(extraInfoAttribute2);
+
+    when(reportMetadata.getCustomKeys()).thenReturn(customAttributes);
+    when(reportMetadata.getInternalKeys()).thenReturn(internalKeys);
+
+    reportingCoordinator.onBeginSession(sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, extraInfo);
+
+    verifySortedKeysAddedToEvent(expectedCustomAttributes, expectedInternalKeys);
   }
 
   @Test
@@ -263,7 +353,7 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() {
     when(reportMetadata.getCustomKeys()).thenReturn(attributes);
 
     reportingCoordinator.onBeginSession(sessionId, timestamp);
-    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, null);
 
     verify(mockEventAppBuilder, never()).setCustomAttributes(any());
     verify(mockEventAppBuilder, never()).build();
@@ -284,7 +374,7 @@ public void testNonFatalEvent_addRolloutsEvent() {
     when(reportMetadata.getRolloutsState()).thenReturn(rolloutsState);
 
     reportingCoordinator.onBeginSession(sessionId, timestamp);
-    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
+    reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp, null);
 
     verify(mockEventAppBuilder, never()).setCustomAttributes(any());
     verify(mockEventAppBuilder, never()).build();
@@ -542,6 +632,17 @@ private void mockEventInteractions() {
         .thenReturn(mockEvent);
   }
 
+  private void verifySortedKeysAddedToEvent(
+          List<CustomAttribute> expectedCustomAttributes,
+          List<CustomAttribute> expectedInternalKeys) {
+    verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
+    verify(mockEventAppBuilder).setInternalKeys(expectedInternalKeys);
+    verify(mockEventAppBuilder).build();
+    verify(mockEventBuilder).setApp(mockEventApp);
+    verify(mockEventBuilder).build();
+    verify(logFileManager, never()).clearLog();
+  }
+
   private static CrashlyticsReport mockReport(String sessionId) {
     final CrashlyticsReport mockReport = mock(CrashlyticsReport.class);
     final CrashlyticsReport.Session mockSession = mock(CrashlyticsReport.Session.class);

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java

@@ -45,6 +45,7 @@
 import com.google.firebase.remoteconfig.interop.FirebaseRemoteConfigInterop;
 import com.google.firebase.sessions.api.FirebaseSessionsDependencies;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 
@@ -224,11 +225,23 @@ public static FirebaseCrashlytics getInstance() {
    * @param throwable a {@link Throwable} to be recorded as a non-fatal event.
    */
   public void recordException(@NonNull Throwable throwable) {
+    recordException(throwable, null);
+  }
+
+  /**
+   * Records a non-fatal report to send to Crashlytics.
+   *
+   * @param throwable a {@link Throwable} to be recorded as a non-fatal event.
+   * @param extraInfo a {@link CustomKeysAndValues} to include extra information about
+   *                  the non-fatal event.  Can be null.
+   */
+  public void recordException(@NonNull Throwable throwable, @Nullable CustomKeysAndValues extraInfo) {
     if (throwable == null) { // Users could call this with null despite the annotation.
       Logger.getLogger().w("A null value was passed to recordException. Ignoring.");
       return;
     }
-    core.logException(throwable);
+    Map<String, String> extraInfoKeysAndValues = (extraInfo != null) ? extraInfo.keysAndValues : null;
+    core.logException(throwable, extraInfoKeysAndValues);
   }
 
   /**

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

@@ -424,7 +424,7 @@ public Void call() throws Exception {
   }
 
   /** Log a caught exception - write out Throwable as event section of protobuf */
-  void writeNonFatalException(@NonNull final Thread thread, @NonNull final Throwable ex) {
+  void writeNonFatalException(@NonNull final Thread thread, @NonNull final Throwable ex, Map<String, String> extraInfo) {
     // Capture and close over the current time, so that we get the exact call time,
     // rather than the time at which the task executes.
     final long timestampMillis = System.currentTimeMillis();
@@ -442,7 +442,7 @@ public void run() {
                 return;
               }
               reportingCoordinator.persistNonFatalEvent(
-                  ex, thread, currentSessionId, timestampSeconds);
+                  ex, thread, currentSessionId, timestampSeconds, extraInfo);
             }
           }
         });

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java

@@ -321,7 +321,19 @@ public static String getVersion() {
    * safe to invoke this method from the main thread.
    */
   public void logException(@NonNull Throwable throwable) {
-    controller.writeNonFatalException(Thread.currentThread(), throwable);
+    logException(throwable, null);
+  }
+
+  /**
+   * Logs a non-fatal Throwable on the Crashlytics servers. Crashlytics will analyze the Throwable
+   * and create a new issue or add it to an existing issue, as appropriate.
+   *
+   * <p>To ensure accurate reporting, this method must be invoked from the thread on which the
+   * Throwable was thrown. The Throwable will always be processed on a background thread, so it is
+   * safe to invoke this method from the main thread.
+   */
+  public void logException(@NonNull Throwable throwable, Map<String, String> extraInfo) {
+    controller.writeNonFatalException(Thread.currentThread(), throwable, extraInfo);
   }
 
   /**