Remove asserts and replace with GDTLogs (#3535)

mikehaney24 · web-flow · commit 5a2e88d6dc69 · 2019-08-08T11:34:52.000-07:00
* Remove asserts and replace with GDTLogs

Should infrequently log to the console in !NDEBUG builds instead of asserting.

* Remove tests that check for assertions.
diff --git a/Firebase/CoreDiagnostics/FIRCDLibrary/FIRCoreDiagnostics.m b/Firebase/CoreDiagnostics/FIRCDLibrary/FIRCoreDiagnostics.m
@@ -17,6 +17,7 @@
 #import <objc/runtime.h>
 #include <sys/utsname.h>
 
+#import <GoogleDataTransport/GDTConsoleLogger.h>
 #import <GoogleDataTransport/GDTEvent.h>
 #import <GoogleDataTransport/GDTTargets.h>
 #import <GoogleDataTransport/GDTTransport.h>
@@ -116,15 +117,17 @@ - (NSData *)transportBytes {
 
   // Encode 1 time to determine the size.
   if (!pb_encode(&sizestream, logs_proto_mobilesdk_ios_ICoreConfiguration_fields, &_config)) {
-    NSCAssert(NO, @"Error in nanopb encoding for size: %s", PB_GET_ERROR(&sizestream));
+    GDTLogError(GDTMCETransportBytesError, @"Error in nanopb encoding for size: %s",
+                PB_GET_ERROR(&sizestream));
   }
 
   // Encode a 2nd time to actually get the bytes from it.
   size_t bufferSize = sizestream.bytes_written;
   CFMutableDataRef dataRef = CFDataCreateMutable(CFAllocatorGetDefault(), bufferSize);
   pb_ostream_t ostream = pb_ostream_from_buffer((void *)CFDataGetBytePtr(dataRef), bufferSize);
   if (!pb_encode(&ostream, logs_proto_mobilesdk_ios_ICoreConfiguration_fields, &_config)) {
-    NSCAssert(NO, @"Error in nanopb encoding for bytes: %s", PB_GET_ERROR(&ostream));
+    GDTLogError(GDTMCETransportBytesError, @"Error in nanopb encoding for bytes: %s",
+                PB_GET_ERROR(&ostream));
   }
   CFDataSetLength(dataRef, ostream.bytes_written);
 
@@ -370,8 +373,9 @@ void FIRPopulateProtoWithCommonInfoFromApp(logs_proto_mobilesdk_ios_ICoreConfigu
   config->pod_name = logs_proto_mobilesdk_ios_ICoreConfiguration_PodName_FIREBASE;
   config->has_pod_name = 1;
 
-  NSCAssert(diagnosticObjects[kFIRCDllAppsCountKey],
-            @"App count is a required value in the data dict.");
+  if (!diagnosticObjects[kFIRCDllAppsCountKey]) {
+    GDTLogError(GDTMCEGeneralError, @"%@", @"App count is a required value in the data dict.");
+  }
   config->app_count = (int32_t)[diagnosticObjects[kFIRCDllAppsCountKey] integerValue];
   config->has_app_count = 1;
 
diff --git a/GoogleDataTransport/GDTLibrary/GDTConsoleLogger.m b/GoogleDataTransport/GDTLibrary/GDTConsoleLogger.m
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#import "GDTLibrary/Private/GDTConsoleLogger.h"
+#import "GDTLibrary/Public/GDTConsoleLogger.h"
 
 /** The console logger prefix. */
 static NSString *kGDTConsoleLogger = @"[GoogleDataTransport]";
diff --git a/GoogleDataTransport/GDTLibrary/GDTPlatform.m b/GoogleDataTransport/GDTLibrary/GDTPlatform.m
@@ -39,6 +39,7 @@ @implementation GDTApplication
 
 + (void)load {
 #if TARGET_OS_IOS || TARGET_OS_TV
+  // If this asserts, please file a bug at https://github.com/firebase/firebase-ios-sdk/issues.
   NSAssert(GDTBackgroundIdentifierInvalid == UIBackgroundTaskInvalid,
            @"GDTBackgroundIdentifierInvalid and UIBackgroundTaskInvalid should be the same.");
 #endif
diff --git a/GoogleDataTransport/GDTLibrary/GDTReachability.m b/GoogleDataTransport/GDTLibrary/GDTReachability.m
@@ -17,6 +17,8 @@
 #import "GDTLibrary/Private/GDTReachability.h"
 #import "GDTLibrary/Private/GDTReachability_Private.h"
 
+#import <GoogleDataTransport/GDTConsoleLogger.h>
+
 #import <netinet/in.h>
 
 /** Sets the _callbackFlag ivar whenever the network changes.
@@ -74,9 +76,13 @@ - (instancetype)init {
     _reachabilityRef = SCNetworkReachabilityCreateWithAddress(
         kCFAllocatorDefault, (const struct sockaddr *)&zeroAddress);
     Boolean success = SCNetworkReachabilitySetDispatchQueue(_reachabilityRef, _reachabilityQueue);
-    NSAssert(success, @"The reachability queue wasn't set.");
+    if (!success) {
+      GDTLogWarning(GDTMCWReachabilityFailed, @"%@", @"The reachability queue wasn't set.");
+    }
     success = SCNetworkReachabilitySetCallback(_reachabilityRef, GDTReachabilityCallback, NULL);
-    NSAssert(success, @"The reachability callback wasn't set.");
+    if (!success) {
+      GDTLogWarning(GDTMCWReachabilityFailed, @"%@", @"The reachability callback wasn't set.");
+    }
 
     // Get the initial set of flags.
     dispatch_async(_reachabilityQueue, ^{
diff --git a/GoogleDataTransport/GDTLibrary/GDTStorage.m b/GoogleDataTransport/GDTLibrary/GDTStorage.m
@@ -17,12 +17,12 @@
 #import "GDTLibrary/Private/GDTStorage.h"
 #import "GDTLibrary/Private/GDTStorage_Private.h"
 
+#import <GoogleDataTransport/GDTConsoleLogger.h>
 #import <GoogleDataTransport/GDTLifecycle.h>
 #import <GoogleDataTransport/GDTPrioritizer.h>
 #import <GoogleDataTransport/GDTStoredEvent.h>
 
 #import "GDTLibrary/Private/GDTAssert.h"
-#import "GDTLibrary/Private/GDTConsoleLogger.h"
 #import "GDTLibrary/Private/GDTEvent_Private.h"
 #import "GDTLibrary/Private/GDTRegistrar_Private.h"
 #import "GDTLibrary/Private/GDTUploadCoordinator.h"
diff --git a/GoogleDataTransport/GDTLibrary/GDTTransformer.m b/GoogleDataTransport/GDTLibrary/GDTTransformer.m
@@ -17,11 +17,11 @@
 #import "GDTLibrary/Private/GDTTransformer.h"
 #import "GDTLibrary/Private/GDTTransformer_Private.h"
 
+#import <GoogleDataTransport/GDTConsoleLogger.h>
 #import <GoogleDataTransport/GDTEventTransformer.h>
 #import <GoogleDataTransport/GDTLifecycle.h>
 
 #import "GDTLibrary/Private/GDTAssert.h"
-#import "GDTLibrary/Private/GDTConsoleLogger.h"
 #import "GDTLibrary/Private/GDTStorage.h"
 
 @implementation GDTTransformer
diff --git a/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m b/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m
@@ -17,9 +17,9 @@
 #import "GDTLibrary/Private/GDTUploadCoordinator.h"
 
 #import <GoogleDataTransport/GDTClock.h>
+#import <GoogleDataTransport/GDTConsoleLogger.h>
 
 #import "GDTLibrary/Private/GDTAssert.h"
-#import "GDTLibrary/Private/GDTConsoleLogger.h"
 #import "GDTLibrary/Private/GDTReachability.h"
 #import "GDTLibrary/Private/GDTRegistrar_Private.h"
 #import "GDTLibrary/Private/GDTStorage.h"
@@ -212,7 +212,10 @@ - (void)packageDelivered:(GDTUploadPackage *)package successful:(BOOL)successful
     NSNumber *targetNumber = @(package.target);
     [self->_targetToInFlightPackages removeObjectForKey:targetNumber];
     id<GDTPrioritizer> prioritizer = self->_registrar.targetToPrioritizer[targetNumber];
-    NSAssert(prioritizer, @"A prioritizer should be registered for this target: %@", targetNumber);
+    if (!prioritizer) {
+      GDTLogError(GDTMCEPrioritizerError, @"A prioritizer should be registered for this target: %@",
+                  targetNumber);
+    }
     if ([prioritizer respondsToSelector:@selector(packageDelivered:successful:)]) {
       [prioritizer packageDelivered:package successful:successful];
     }
diff --git a/GoogleDataTransport/GDTLibrary/GDTUploadPackage.m b/GoogleDataTransport/GDTLibrary/GDTUploadPackage.m
@@ -17,6 +17,7 @@
 #import <GoogleDataTransport/GDTUploadPackage.h>
 
 #import <GoogleDataTransport/GDTClock.h>
+#import <GoogleDataTransport/GDTConsoleLogger.h>
 
 #import "GDTLibrary/Private/GDTStorage_Private.h"
 #import "GDTLibrary/Private/GDTUploadCoordinator.h"
@@ -74,7 +75,10 @@ - (void)setStorage:(GDTStorage *)storage {
 }
 
 - (void)completeDelivery {
-  NSAssert(_isDelivered == NO, @"It's an API violation to call -completeDelivery twice.");
+  if (_isDelivered) {
+    GDTLogError(GDTMCEDeliverTwice, @"%@",
+                @"It's an API violation to call -completeDelivery twice.");
+  }
   _isDelivered = YES;
   if (!_isHandled && _handler &&
       [_handler respondsToSelector:@selector(packageDelivered:successful:)]) {
diff --git a/GoogleDataTransport/GDTLibrary/Public/GDTConsoleLogger.h b/GoogleDataTransport/GDTLibrary/Public/GDTConsoleLogger.h
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#import "GDTLibrary/Private/GDTAssert.h"
+#import <Foundation/Foundation.h>
 
 /** A list of message codes to print in the logger that help to correspond printed messages with
  * code locations.
@@ -34,14 +34,29 @@ typedef NS_ENUM(NSInteger, GDTMessageCode) {
   /** For warning messages concerning a forced event upload. */
   GDTMCWForcedUpload = 3,
 
+  /** For warning messages concerning a failed reachability call. */
+  GDTMCWReachabilityFailed = 4,
+
   /** For error messages concerning transform: not being implemented by an event transformer. */
   GDTMCETransformerDoesntImplementTransform = 1000,
 
   /** For error messages concerning the creation of a directory failing. */
   GDTMCEDirectoryCreationError = 1001,
 
   /** For error messages concerning the writing of a event file. */
-  GDTMCEFileWriteError = 1002
+  GDTMCEFileWriteError = 1002,
+
+  /** For error messages concerning the lack of a prioritizer for a given backend. */
+  GDTMCEPrioritizerError = 1003,
+
+  /** For error messages concerning a package delivery API violation. */
+  GDTMCEDeliverTwice = 1004,
+
+  /** For error messages concerning an error in an implementation of -transportBytes. */
+  GDTMCETransportBytesError = 1005,
+
+  /** For general purpose error messages in a dependency. */
+  GDTMCEGeneralError = 1006
 };
 
 /** */
@@ -61,5 +76,4 @@ FOUNDATION_EXPORT NSString *_Nonnull GDTMessageCodeEnumToString(GDTMessageCode c
 
 // A define to wrap GULLogError with slightly more convenient usage and a failing assert.
 #define GDTLogError(MESSAGE_CODE, MESSAGE_FORMAT, ...) \
-  GDTLog(MESSAGE_CODE, MESSAGE_FORMAT, __VA_ARGS__);   \
-  GDTAssert(NO, MESSAGE_FORMAT, __VA_ARGS__);
+  GDTLog(MESSAGE_CODE, MESSAGE_FORMAT, __VA_ARGS__);
diff --git a/GoogleDataTransport/GDTLibrary/Public/GoogleDataTransport.h b/GoogleDataTransport/GDTLibrary/Public/GoogleDataTransport.h
@@ -15,6 +15,7 @@
  */
 
 #import "GDTClock.h"
+#import "GDTConsoleLogger.h"
 #import "GDTDataFuture.h"
 #import "GDTEvent.h"
 #import "GDTEventDataObject.h"
diff --git a/GoogleDataTransport/GDTTests/Unit/GDTTransformerTest.m b/GoogleDataTransport/GDTTests/Unit/GDTTransformerTest.m
@@ -110,19 +110,4 @@ - (void)testWriteEventWithTransformersThatCreateANewEvent {
   XCTAssertNoThrow([transformer transformEvent:event withTransformers:transformers]);
 }
 
-/** Tests that using a transformer without transform: implemented throws. */
-- (void)testWriteEventWithBadTransformer {
-  GDTTransformer *transformer = [GDTTransformer sharedInstance];
-  GDTEvent *event = [[GDTEvent alloc] initWithMappingID:@"2" target:1];
-  event.dataObject = [[GDTDataObjectTesterSimple alloc] init];
-  NSArray *transformers = @[ [[NSObject alloc] init] ];
-
-  XCTestExpectation *errorExpectation = [self expectationWithDescription:@"transform: is missing"];
-  [GDTAssertHelper setAssertionBlock:^{
-    [errorExpectation fulfill];
-  }];
-  [transformer transformEvent:event withTransformers:transformers];
-  [self waitForExpectations:@[ errorExpectation ] timeout:5.0];
-}
-
 @end
diff --git a/GoogleDataTransport/GDTTests/Unit/GDTUploadPackageTest.m b/GoogleDataTransport/GDTTests/Unit/GDTUploadPackageTest.m
@@ -77,19 +77,6 @@ - (void)testInit {
   XCTAssertNotNil([[GDTUploadPackage alloc] initWithTarget:kGDTTargetTest]);
 }
 
-/** Tests that calling -completeDelivery twice on the same package asserts. */
-- (void)testCompleteDeliveryTwiceAsserts {
-  GDTTestPrioritizer *prioritizer = [[GDTTestPrioritizer alloc] init];
-  GDTTestUploader *uploader = [[GDTTestUploader alloc] init];
-
-  [[GDTRegistrar sharedInstance] registerPrioritizer:prioritizer target:kGDTTargetTest];
-  [[GDTRegistrar sharedInstance] registerUploader:uploader target:kGDTTargetTest];
-
-  GDTUploadPackage *uploadPackage = [[GDTUploadPackage alloc] initWithTarget:kGDTTargetTest];
-  [uploadPackage completeDelivery];
-  XCTAssertThrows([uploadPackage completeDelivery]);
-}
-
 /** Tests copying indicates that the underlying sets of events can't be changed from underneath. */
 - (void)testRegisterUpload {
   GDTUploadPackage *uploadPackage = [[GDTUploadPackage alloc] initWithTarget:kGDTTargetTest];
diff --git a/GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTNanopbHelpers.m b/GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTNanopbHelpers.m
@@ -22,6 +22,8 @@
 #import <AppKit/AppKit.h>
 #endif  // TARGET_OS_IOS || TARGET_OS_TV
 
+#import <GoogleDataTransport/GDTConsoleLogger.h>
+
 #import <nanopb/pb.h>
 #import <nanopb/pb_decode.h>
 #import <nanopb/pb_encode.h>
@@ -48,15 +50,17 @@
   pb_ostream_t sizestream = PB_OSTREAM_SIZING;
   // Encode 1 time to determine the size.
   if (!pb_encode(&sizestream, gdt_cct_BatchedLogRequest_fields, batchedLogRequest)) {
-    NSCAssert(NO, @"Error in nanopb encoding for size: %s", PB_GET_ERROR(&sizestream));
+    GDTLogError(GDTMCEGeneralError, @"Error in nanopb encoding for size: %s",
+                PB_GET_ERROR(&sizestream));
   }
 
   // Encode a 2nd time to actually get the bytes from it.
   size_t bufferSize = sizestream.bytes_written;
   CFMutableDataRef dataRef = CFDataCreateMutable(CFAllocatorGetDefault(), bufferSize);
   pb_ostream_t ostream = pb_ostream_from_buffer((void *)CFDataGetBytePtr(dataRef), bufferSize);
   if (!pb_encode(&ostream, gdt_cct_BatchedLogRequest_fields, batchedLogRequest)) {
-    NSCAssert(NO, @"Error in nanopb encoding for bytes: %s", PB_GET_ERROR(&ostream));
+    GDTLogError(GDTMCEGeneralError, @"Error in nanopb encoding for bytes: %s",
+                PB_GET_ERROR(&ostream));
   }
   CFDataSetLength(dataRef, ostream.bytes_written);
 
@@ -86,7 +90,11 @@ gdt_cct_BatchedLogRequest GDTCCTConstructBatchedLogRequest(
 
 gdt_cct_LogRequest GDTCCTConstructLogRequest(int32_t logSource,
                                              NSSet<GDTStoredEvent *> *_Nonnull logSet) {
-  NSCAssert(logSet.count, @"An empty event set can't be serialized to proto.");
+  if (logSet.count == 0) {
+    GDTLogError(GDTMCEGeneralError, @"%@", @"An empty event set can't be serialized to proto.");
+    gdt_cct_LogRequest logRequest = gdt_cct_LogRequest_init_default;
+    return logRequest;
+  }
   gdt_cct_LogRequest logRequest = gdt_cct_LogRequest_init_default;
   logRequest.log_source = logSource;
   logRequest.has_log_source = 1;
@@ -118,7 +126,11 @@ gdt_cct_LogEvent GDTCCTConstructLogEvent(GDTStoredEvent *event) {
   NSData *extensionBytes = [NSData dataWithContentsOfURL:event.dataFuture.fileURL
                                                  options:0
                                                    error:&error];
-  NSCAssert(error == nil, @"There was an error reading extension bytes from disk: %@", error);
+  if (error) {
+    GDTLogError(GDTMCEGeneralError, @"There was an error reading extension bytes from disk: %@",
+                error);
+    return logEvent;
+  }
   logEvent.source_extension = GDTCCTEncodeData(extensionBytes);  // read bytes from the file.
   return logEvent;
 }
diff --git a/GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTUploader.m b/GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTUploader.m
@@ -16,6 +16,7 @@
 
 #import "GDTCCTLibrary/Private/GDTCCTUploader.h"
 
+#import <GoogleDataTransport/GDTConsoleLogger.h>
 #import <GoogleDataTransport/GDTPlatform.h>
 #import <GoogleDataTransport/GDTRegistrar.h>
 
@@ -86,15 +87,20 @@ - (NSURL *)defaultServerURL {
 
 - (void)uploadPackage:(GDTUploadPackage *)package {
   dispatch_async(_uploaderQueue, ^{
-    NSAssert(!self->_currentTask, @"An upload shouldn't be initiated with another in progress.");
-    NSAssert(!self->_currentUploadPackage, @"An upload shouldn't be initiated with a new package.");
+    if (self->_currentTask || self->_currentUploadPackage) {
+      GDTLogWarning(GDTMCWUploadFailed, @"%@",
+                    @"An upload shouldn't be initiated with another in progress.");
+      return;
+    }
     NSURL *serverURL = self.serverURL ? self.serverURL : [self defaultServerURL];
     NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:serverURL];
     request.HTTPMethod = @"POST";
 
     id completionHandler =
         ^(NSData *_Nullable data, NSURLResponse *_Nullable response, NSError *_Nullable error) {
-          NSAssert(!error, @"There should be no errors uploading events: %@", error);
+          if (error) {
+            GDTLogWarning(GDTMCWUploadFailed, @"There was an error uploading events: %@", error);
+          }
           NSError *decodingError;
           gdt_cct_LogResponse logResponse = GDTCCTDecodeLogResponse(data, &decodingError);
           if (!decodingError && logResponse.has_next_request_wait_millis) {
