Scoping out report callback API (#7503)

Author: samedson

Crashlytics/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [added] Added a new API checkAndUpdateUnsentReportsWithCompletion for updating the crash report from the previous run of the app if, for example, the developer wants to implement a feedback dialog to ask end-users for more information. Unsent Crashlytics Reports have familiar methods like setting custom keys and logs.
+
 # v7.6.0
 - [fixed] Fixed an issue where some developers experienced a race condition involving binary image operations (#7459).
 

Crashlytics/Crashlytics/Components/FIRCLSUserLogging.h

@@ -101,6 +101,11 @@ void FIRCLSUserLoggingWriteAndCheckABFiles(FIRCLSUserLoggingABStorage* storage,
 NSArray* FIRCLSUserLoggingStoredKeyValues(const char* path);
 
 OBJC_EXTERN void FIRCLSLog(NSString* format, ...) NS_FORMAT_FUNCTION(1, 2);
+OBJC_EXTERN void FIRCLSLogToStorage(FIRCLSUserLoggingABStorage* storage,
+                                    const char** activePath,
+                                    NSString* format,
+                                    ...) NS_FORMAT_FUNCTION(3, 4);
+
 #endif
 
 __END_DECLS

Crashlytics/Crashlytics/Components/FIRCLSUserLogging.m

@@ -42,7 +42,9 @@ static void FIRCLSUserLoggingWriteKeysAndValues(NSDictionary *keysAndValues,
 static void FIRCLSUserLoggingCheckAndSwapABFiles(FIRCLSUserLoggingABStorage *storage,
                                                  const char **activePath,
                                                  off_t fileSize);
-void FIRCLSLogInternal(NSString *message);
+void FIRCLSLogInternal(FIRCLSUserLoggingABStorage *storage,
+                       const char **activePath,
+                       NSString *message);
 
 #pragma mark - Setup
 void FIRCLSUserLoggingInit(FIRCLSUserLoggingReadOnlyContext *roContext,
@@ -397,7 +399,26 @@ void FIRCLSLog(NSString *format, ...) {
   NSString *msg = [[NSString alloc] initWithFormat:format arguments:args];
   va_end(args);
 
-  FIRCLSLogInternal(msg);
+  FIRCLSUserLoggingABStorage *currentStorage = &_firclsContext.readonly->logging.logStorage;
+  const char **activePath = &_firclsContext.writable->logging.activeUserLogPath;
+  FIRCLSLogInternal(currentStorage, activePath, msg);
+}
+
+void FIRCLSLogToStorage(FIRCLSUserLoggingABStorage *storage,
+                        const char **activePath,
+                        NSString *format,
+                        ...) {
+  // If the format is nil do nothing just like NSLog.
+  if (!format) {
+    return;
+  }
+
+  va_list args;
+  va_start(args, format);
+  NSString *msg = [[NSString alloc] initWithFormat:format arguments:args];
+  va_end(args);
+
+  FIRCLSLogInternal(storage, activePath, msg);
 }
 
 #pragma mark - Properties
@@ -525,7 +546,9 @@ void FIRCLSLogInternalWrite(FIRCLSFile *file, NSString *message, uint64_t time)
   FIRCLSFileWriteSectionEnd(file);
 }
 
-void FIRCLSLogInternal(NSString *message) {
+void FIRCLSLogInternal(FIRCLSUserLoggingABStorage *storage,
+                       const char **activePath,
+                       NSString *message) {
   if (!message) {
     return;
   }
@@ -539,7 +562,7 @@ void FIRCLSLogInternal(NSString *message) {
   struct timeval te;
 
   NSUInteger messageLength = [message length];
-  int maxLogSize = _firclsContext.readonly->logging.logStorage.maxSize;
+  int maxLogSize = storage->maxSize;
 
   if (messageLength > maxLogSize) {
     FIRCLSWarningLog(
@@ -556,9 +579,7 @@ void FIRCLSLogInternal(NSString *message) {
 
   const uint64_t time = te.tv_sec * 1000LL + te.tv_usec / 1000;
 
-  FIRCLSUserLoggingWriteAndCheckABFiles(&_firclsContext.readonly->logging.logStorage,
-                                        &_firclsContext.writable->logging.activeUserLogPath,
-                                        ^(FIRCLSFile *file) {
-                                          FIRCLSLogInternalWrite(file, message, time);
-                                        });
+  FIRCLSUserLoggingWriteAndCheckABFiles(storage, activePath, ^(FIRCLSFile *file) {
+    FIRCLSLogInternalWrite(file, message, time);
+  });
 }

Crashlytics/Crashlytics/Controllers/FIRCLSExistingReportManager.h

@@ -19,25 +19,59 @@ NS_ASSUME_NONNULL_BEGIN
 @class FIRCLSManagerData;
 @class FIRCLSReportUploader;
 @class FIRCLSDataCollectionToken;
+@class FIRCrashlyticsReport;
 
 @interface FIRCLSExistingReportManager : NSObject
 
+/**
+ * Returns the number of unsent reports on the device, ignoring empty reports in
+ * the active folder, and ignoring any reports in "processing" or "prepared".
+ *
+ * In the past, this would count reports in the processed or prepared
+ * folders. This has been changed because reports in those paths have already
+ * been cleared for upload, so there isn't any point in asking for permission
+ * or possibly spamming end-users if a report gets stuck.
+ *
+ * The tricky part is, customers will NOT be alerted in checkForUnsentReports
+ * for reports in these paths, but when they choose sendUnsentReports / enable data
+ * collection, reports in those directories will be re-managed. This should be ok and
+ * just an edge case because reports should only be in processing or prepared for a split second as
+ * they do on-device symbolication and get converted into a GDTEvent. After a report is handed off
+ * to GoogleDataTransport, it is uploaded regardless of Crashlytics data collection.
+ */
+@property(nonatomic, readonly) NSUInteger unsentReportsCount;
+
+/**
+ * This value needs to stay in sync with numUnsentReports, so if there is > 0 numUnsentReports,
+ * newestUnsentReport needs to return a value. Otherwise it needs to return null.
+ *
+ * FIRCLSContext needs to be initialized before the FIRCrashlyticsReport is instantiated.
+ */
+@property(nonatomic, readonly) FIRCrashlyticsReport *_Nullable newestUnsentReport;
+
 - (instancetype)initWithManagerData:(FIRCLSManagerData *)managerData
                      reportUploader:(FIRCLSReportUploader *)reportUploader;
 
 - (instancetype)init NS_UNAVAILABLE;
 + (instancetype)new NS_UNAVAILABLE;
 
-- (int)unsentReportsCountWithPreexisting:(NSArray<NSString *> *)paths;
+/**
+ * This is important to call once, early in startup, before the
+ * new report for this run of the app has been created. Any
+ * reports in ExistingReportManager will be uploaded or deleted
+ * and we don't want to do that for the current run of the app.
+ */
+- (void)collectExistingReports;
 
-- (void)deleteUnsentReportsWithPreexisting:(NSArray *)preexistingReportPaths;
+/**
+ * This is the side-effect of calling deleteUnsentReports, or collect_reports setting
+ * being false.
+ */
+- (void)deleteUnsentReports;
 
-- (void)processExistingReportPaths:(NSArray *)reportPaths
-               dataCollectionToken:(FIRCLSDataCollectionToken *)dataCollectionToken
+- (void)sendUnsentReportsWithToken:(FIRCLSDataCollectionToken *)dataCollectionToken
                           asUrgent:(BOOL)urgent;
 
-- (void)handleContentsInOtherReportingDirectoriesWithToken:(FIRCLSDataCollectionToken *)token;
-
 @end
 
 NS_ASSUME_NONNULL_END

Crashlytics/Crashlytics/Controllers/FIRCLSExistingReportManager.m

@@ -17,16 +17,23 @@
 #import "Crashlytics/Crashlytics/Controllers/FIRCLSManagerData.h"
 #import "Crashlytics/Crashlytics/Controllers/FIRCLSReportUploader.h"
 #import "Crashlytics/Crashlytics/DataCollection/FIRCLSDataCollectionToken.h"
-#import "Crashlytics/Crashlytics/Helpers/FIRCLSLogger.h"
 #import "Crashlytics/Crashlytics/Models/FIRCLSFileManager.h"
 #import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h"
+#import "Crashlytics/Crashlytics/Private/FIRCrashlyticsReport_Private.h"
+#import "Crashlytics/Crashlytics/Public/FirebaseCrashlytics/FIRCrashlyticsReport.h"
 
 @interface FIRCLSExistingReportManager ()
 
 @property(nonatomic, strong) FIRCLSFileManager *fileManager;
 @property(nonatomic, strong) NSOperationQueue *operationQueue;
 @property(nonatomic, strong) FIRCLSReportUploader *reportUploader;
 
+// This list of active reports excludes the brand new active report that will be created this run of
+// the app.
+@property(nonatomic, strong) NSArray *existingUnemptyActiveReportPaths;
+@property(nonatomic, strong) NSArray *processingReportPaths;
+@property(nonatomic, strong) NSArray *preparedReportPaths;
+
 @end
 
 @implementation FIRCLSExistingReportManager
@@ -45,51 +52,106 @@ - (instancetype)initWithManagerData:(FIRCLSManagerData *)managerData
   return self;
 }
 
-/**
- * Returns the number of unsent reports on the device, including the ones passed in.
- */
-- (int)unsentReportsCountWithPreexisting:(NSArray<NSString *> *)paths {
-  int count = [self countSubmittableAndDeleteUnsubmittableReportPaths:paths];
+NSInteger compareOlder(FIRCLSInternalReport *reportA,
+                       FIRCLSInternalReport *reportB,
+                       void *context) {
+  return [reportA.dateCreated compare:reportB.dateCreated];
+}
+
+- (void)collectExistingReports {
+  self.existingUnemptyActiveReportPaths =
+      [self getUnemptyExistingActiveReportsAndDeleteEmpty:self.fileManager.activePathContents];
+  self.processingReportPaths = self.fileManager.processingPathContents;
+  self.preparedReportPaths = self.fileManager.preparedPathContents;
+}
+
+- (FIRCrashlyticsReport *)newestUnsentReport {
+  if (self.unsentReportsCount <= 0) {
+    return nil;
+  }
+
+  NSMutableArray<NSString *> *allReportPaths =
+      [NSMutableArray arrayWithArray:self.existingUnemptyActiveReportPaths];
 
-  count += self.fileManager.processingPathContents.count;
-  count += self.fileManager.preparedPathContents.count;
-  return count;
+  NSMutableArray<FIRCLSInternalReport *> *validReports = [NSMutableArray array];
+  for (NSString *path in allReportPaths) {
+    FIRCLSInternalReport *_Nullable report = [FIRCLSInternalReport reportWithPath:path];
+    if (!report) {
+      continue;
+    }
+    [validReports addObject:report];
+  }
+
+  [validReports sortUsingFunction:compareOlder context:nil];
+
+  FIRCLSInternalReport *_Nullable internalReport = [validReports lastObject];
+  return [[FIRCrashlyticsReport alloc] initWithInternalReport:internalReport];
+}
+
+- (NSUInteger)unsentReportsCount {
+  // There are nuances about why we only count active reports.
+  // See the header comment for more information.
+  return self.existingUnemptyActiveReportPaths.count;
 }
 
-- (int)countSubmittableAndDeleteUnsubmittableReportPaths:(NSArray *)reportPaths {
-  int count = 0;
+- (NSArray *)getUnemptyExistingActiveReportsAndDeleteEmpty:(NSArray *)reportPaths {
+  NSMutableArray *unemptyReports = [NSMutableArray array];
   for (NSString *path in reportPaths) {
     FIRCLSInternalReport *report = [FIRCLSInternalReport reportWithPath:path];
-    if ([report needsToBeSubmitted]) {
-      count++;
+    if ([report hasAnyEvents]) {
+      [unemptyReports addObject:path];
     } else {
       [self.operationQueue addOperationWithBlock:^{
-        [self->_fileManager removeItemAtPath:path];
+        [self.fileManager removeItemAtPath:path];
       }];
     }
   }
-  return count;
+  return unemptyReports;
 }
 
-- (void)processExistingReportPaths:(NSArray *)reportPaths
-               dataCollectionToken:(FIRCLSDataCollectionToken *)dataCollectionToken
+- (void)sendUnsentReportsWithToken:(FIRCLSDataCollectionToken *)dataCollectionToken
                           asUrgent:(BOOL)urgent {
-  for (NSString *path in reportPaths) {
+  for (NSString *path in self.existingUnemptyActiveReportPaths) {
     [self processExistingActiveReportPath:path
                       dataCollectionToken:dataCollectionToken
                                  asUrgent:urgent];
   }
+
+  // deal with stuff in processing more carefully - do not process again
+  [self.operationQueue addOperationWithBlock:^{
+    for (NSString *path in self.processingReportPaths) {
+      FIRCLSInternalReport *report = [FIRCLSInternalReport reportWithPath:path];
+      [self.reportUploader prepareAndSubmitReport:report
+                              dataCollectionToken:dataCollectionToken
+                                         asUrgent:NO
+                                   withProcessing:NO];
+    }
+  }];
+
+  // Because this could happen quite a bit after the inital set of files was
+  // captured, some could be completed (deleted). So, just double-check to make sure
+  // the file still exists.
+  [self.operationQueue addOperationWithBlock:^{
+    for (NSString *path in self.preparedReportPaths) {
+      if (![[self.fileManager underlyingFileManager] fileExistsAtPath:path]) {
+        continue;
+      }
+      [self.reportUploader uploadPackagedReportAtPath:path
+                                  dataCollectionToken:dataCollectionToken
+                                             asUrgent:NO];
+    }
+  }];
 }
 
 - (void)processExistingActiveReportPath:(NSString *)path
                     dataCollectionToken:(FIRCLSDataCollectionToken *)dataCollectionToken
                                asUrgent:(BOOL)urgent {
   FIRCLSInternalReport *report = [FIRCLSInternalReport reportWithPath:path];
 
-  // TODO: needsToBeSubmitted should really be called on the background queue.
-  if (![report needsToBeSubmitted]) {
+  // TODO: hasAnyEvents should really be called on the background queue.
+  if (![report hasAnyEvents]) {
     [self.operationQueue addOperationWithBlock:^{
-      [self->_fileManager removeItemAtPath:path];
+      [self.fileManager removeItemAtPath:path];
     }];
 
     return;
@@ -104,11 +166,6 @@ - (void)processExistingActiveReportPath:(NSString *)path
     return;
   }
 
-  [self submitReport:report dataCollectionToken:dataCollectionToken];
-}
-
-- (void)submitReport:(FIRCLSInternalReport *)report
-    dataCollectionToken:(FIRCLSDataCollectionToken *)dataCollectionToken {
   [self.operationQueue addOperationWithBlock:^{
     [self.reportUploader prepareAndSubmitReport:report
                             dataCollectionToken:dataCollectionToken
@@ -117,61 +174,17 @@ - (void)submitReport:(FIRCLSInternalReport *)report
   }];
 }
 
-// This is the side-effect of calling deleteUnsentReports, or collect_reports setting
-// being false
-- (void)deleteUnsentReportsWithPreexisting:(NSArray *)preexistingReportPaths {
-  [self removeExistingReportPaths:preexistingReportPaths];
-  [self removeExistingReportPaths:self.fileManager.processingPathContents];
-  [self removeExistingReportPaths:self.fileManager.preparedPathContents];
-}
+- (void)deleteUnsentReports {
+  NSArray<NSString *> *reportPaths = @[];
+  reportPaths = [reportPaths arrayByAddingObjectsFromArray:self.existingUnemptyActiveReportPaths];
+  reportPaths = [reportPaths arrayByAddingObjectsFromArray:self.processingReportPaths];
+  reportPaths = [reportPaths arrayByAddingObjectsFromArray:self.preparedReportPaths];
 
-- (void)removeExistingReportPaths:(NSArray *)reportPaths {
   [self.operationQueue addOperationWithBlock:^{
     for (NSString *path in reportPaths) {
       [self.fileManager removeItemAtPath:path];
     }
   }];
 }
 
-- (void)handleContentsInOtherReportingDirectoriesWithToken:(FIRCLSDataCollectionToken *)token {
-  [self handleExistingFilesInProcessingWithToken:token];
-  [self handleExistingFilesInPreparedWithToken:token];
-}
-
-- (void)handleExistingFilesInProcessingWithToken:(FIRCLSDataCollectionToken *)token {
-  NSArray *processingPaths = _fileManager.processingPathContents;
-
-  // deal with stuff in processing more carefully - do not process again
-  [self.operationQueue addOperationWithBlock:^{
-    for (NSString *path in processingPaths) {
-      FIRCLSInternalReport *report = [FIRCLSInternalReport reportWithPath:path];
-      [self.reportUploader prepareAndSubmitReport:report
-                              dataCollectionToken:token
-                                         asUrgent:NO
-                                   withProcessing:NO];
-    }
-  }];
-}
-
-- (void)handleExistingFilesInPreparedWithToken:(FIRCLSDataCollectionToken *)token {
-  NSArray *preparedPaths = self.fileManager.preparedPathContents;
-  [self.operationQueue addOperationWithBlock:^{
-    [self uploadPreexistingFiles:preparedPaths withToken:token];
-  }];
-}
-
-- (void)uploadPreexistingFiles:(NSArray *)files withToken:(FIRCLSDataCollectionToken *)token {
-  // Because this could happen quite a bit after the inital set of files was
-  // captured, some could be completed (deleted). So, just double-check to make sure
-  // the file still exists.
-
-  for (NSString *path in files) {
-    if (![[_fileManager underlyingFileManager] fileExistsAtPath:path]) {
-      continue;
-    }
-
-    [self.reportUploader uploadPackagedReportAtPath:path dataCollectionToken:token asUrgent:NO];
-  }
-}
-
 @end

Crashlytics/Crashlytics/Controllers/FIRCLSReportManager.h

@@ -36,7 +36,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (FBLPromise<NSNumber *> *)startWithProfilingMark:(FIRCLSProfileMark)mark;
 
-- (FBLPromise<NSNumber *> *)checkForUnsentReports;
+- (FBLPromise<FIRCrashlyticsReport *> *)checkForUnsentReports;
 - (FBLPromise *)sendUnsentReports;
 - (FBLPromise *)deleteUnsentReports;