firebase · mikehaney24 · Sep 4, 2019 · Sep 3, 2019 · Sep 3, 2019 · Sep 4, 2019
diff --git a/GoogleDataTransport/CHANGELOG.md b/GoogleDataTransport/CHANGELOG.md
@@ -1,3 +1,6 @@
+# v1.2.0
+- Removes all NSAsserts in favor of custom asserts. (#3747)
+
 # v1.1.3
 - Wrap decoding in GDTUploadCoordinator in a try catch. (#3676)
 

diff --git a/GoogleDataTransport/GDTLibrary/GDTAssert.m b/GoogleDataTransport/GDTLibrary/GDTAssert.m
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-#import "GDTLibrary/Private/GDTAssert.h"
+#import "GDTLibrary/Public/GDTAssert.h"
 
-GDTAssertionBlock GDTAssertionBlockToRunInsteadOfNSAssert(void) {
+GDTAssertionBlock GDTAssertionBlockToRunInstead(void) {
   // This class is only compiled in by unit tests, and this should fail quickly in optimized builds.
   Class GDTAssertClass = NSClassFromString(@"GDTAssertHelper");
   if (__builtin_expect(!!GDTAssertClass, 0)) {

diff --git a/GoogleDataTransport/GDTLibrary/GDTEvent.m b/GoogleDataTransport/GDTLibrary/GDTEvent.m
@@ -16,16 +16,19 @@
 
 #import <GoogleDataTransport/GDTEvent.h>
 
+#import <GoogleDataTransport/GDTAssert.h>
 #import <GoogleDataTransport/GDTStoredEvent.h>
 
-#import "GDTLibrary/Private/GDTAssert.h"
 #import "GDTLibrary/Private/GDTEvent_Private.h"
 
 @implementation GDTEvent
 
 - (instancetype)initWithMappingID:(NSString *)mappingID target:(NSInteger)target {
   GDTAssert(mappingID.length > 0, @"Please give a valid mapping ID");
   GDTAssert(target > 0, @"A target cannot be negative or 0");
+  if (mappingID == nil || mappingID.length == 0 || target <= 0) {
+    return nil;
+  }
   self = [super init];
   if (self) {
     _mappingID = mappingID;

diff --git a/GoogleDataTransport/GDTLibrary/GDTPlatform.m b/GoogleDataTransport/GDTLibrary/GDTPlatform.m
@@ -16,6 +16,8 @@
 
 #import <GoogleDataTransport/GDTPlatform.h>
 
+#import <GoogleDataTransport/GDTAssert.h>
+
 const GDTBackgroundIdentifier GDTBackgroundIdentifierInvalid = 0;
 
 NSString *const kGDTApplicationDidEnterBackgroundNotification =
@@ -40,8 +42,8 @@ @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.");
+  GDTFatalAssert(GDTBackgroundIdentifierInvalid == UIBackgroundTaskInvalid,
+                 @"GDTBackgroundIdentifierInvalid and UIBackgroundTaskInvalid should be the same.");
 #endif
   [self sharedApplication];
 }

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/GDTAssert.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/GDTEvent_Private.h"
 #import "GDTLibrary/Private/GDTRegistrar_Private.h"
 #import "GDTLibrary/Private/GDTUploadCoordinator.h"
@@ -74,6 +74,10 @@ - (instancetype)init {
 }
 
 - (void)storeEvent:(GDTEvent *)event {
+  if (event == nil) {
+    return;
+  }
+
   [self createEventDirectoryIfNotExists];
 
   __block GDTBackgroundIdentifier bgID = GDTBackgroundIdentifierInvalid;

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/GDTAssert.h>
 #import <GoogleDataTransport/GDTConsoleLogger.h>
 #import <GoogleDataTransport/GDTEventTransformer.h>
 #import <GoogleDataTransport/GDTLifecycle.h>
 
-#import "GDTLibrary/Private/GDTAssert.h"
 #import "GDTLibrary/Private/GDTStorage.h"
 
 @implementation GDTTransformer

diff --git a/GoogleDataTransport/GDTLibrary/GDTTransport.m b/GoogleDataTransport/GDTLibrary/GDTTransport.m
@@ -17,21 +17,24 @@
 #import <GoogleDataTransport/GDTTransport.h>
 #import "GDTLibrary/Private/GDTTransport_Private.h"
 
+#import <GoogleDataTransport/GDTAssert.h>
 #import <GoogleDataTransport/GDTClock.h>
 #import <GoogleDataTransport/GDTEvent.h>
 
-#import "GDTLibrary/Private/GDTAssert.h"
 #import "GDTLibrary/Private/GDTTransformer.h"
 
 @implementation GDTTransport
 
 - (instancetype)initWithMappingID:(NSString *)mappingID
                      transformers:(nullable NSArray<id<GDTEventTransformer>> *)transformers
                            target:(NSInteger)target {
+  GDTAssert(mappingID.length > 0, @"A mapping ID cannot be nil or empty");
+  GDTAssert(target > 0, @"A target cannot be negative or 0");
+  if (mappingID == nil || mappingID.length == 0 || target <= 0) {
+    return nil;
+  }
   self = [super init];
   if (self) {
-    GDTAssert(mappingID.length > 0, @"A mapping ID cannot be nil or empty");
-    GDTAssert(target > 0, @"A target cannot be negative or 0");
     _mappingID = mappingID;
     _transformers = transformers;
     _target = target;

diff --git a/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m b/GoogleDataTransport/GDTLibrary/GDTUploadCoordinator.m
@@ -16,10 +16,10 @@
 
 #import "GDTLibrary/Private/GDTUploadCoordinator.h"
 
+#import <GoogleDataTransport/GDTAssert.h>
 #import <GoogleDataTransport/GDTClock.h>
 #import <GoogleDataTransport/GDTConsoleLogger.h>
 
-#import "GDTLibrary/Private/GDTAssert.h"
 #import "GDTLibrary/Private/GDTReachability.h"
 #import "GDTLibrary/Private/GDTRegistrar_Private.h"
 #import "GDTLibrary/Private/GDTStorage.h"

diff --git a/GoogleDataTransport/GDTLibrary/Private/GDTAssert.h b/GoogleDataTransport/GDTLibrary/Private/GDTAssert.h
diff --git a/GoogleDataTransport/GDTLibrary/Public/GDTAssert.h b/GoogleDataTransport/GDTLibrary/Public/GDTAssert.h
@@ -0,0 +1,91 @@
+/*
+ * Copyright 2019 Google
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#import <Foundation/Foundation.h>
+
+#import <GoogleDataTransport/GDTConsoleLogger.h>
+
+/** A block type that could be run instead of normal assertion logging. No return type, no params.
+ */
+typedef void (^GDTAssertionBlock)(void);
+
+/** Returns the result of executing a soft-linked method present in unit tests that allows a block
+ * to be run instead of normal assertion logging. This helps ameliorate issues with catching
+ * exceptions that occur on a dispatch_queue.
+ *
+ * @return A block that can be run instead of normal assert printing.
+ */
+FOUNDATION_EXPORT GDTAssertionBlock _Nullable GDTAssertionBlockToRunInstead(void);
+
+#if defined(NS_BLOCK_ASSERTIONS)
+
+#define GDTAssert(condition, ...) \
+  do {                            \
+  } while (0);
+
+#define GDTFatalAssert(condition, ...) \
+  do {                                 \
+  } while (0);
+
+#else  // defined(NS_BLOCK_ASSERTIONS)
+
+/** Asserts using a console log, unless a block was specified to be run instead.
+ *
+ * @param condition The condition you'd expect to be YES.
+ */
+#define GDTAssert(condition, ...)                                                          \
+  do {                                                                                     \
+    if (__builtin_expect(!(condition), 0)) {                                               \
+      GDTAssertionBlock assertionBlock = GDTAssertionBlockToRunInstead();                  \
+      if (assertionBlock) {                                                                \
+        assertionBlock();                                                                  \
+      } else {                                                                             \
+        __PRAGMA_PUSH_NO_EXTRA_ARG_WARNINGS                                                \
+        NSString *__assert_file__ = [NSString stringWithUTF8String:__FILE__];              \
+        __assert_file__ = __assert_file__ ? __assert_file__ : @"<Unknown File>";           \
+        GDTLogError(GDTMCEGeneralError, @"Assertion failed (%@:%d): %s,", __assert_file__, \
+                    __LINE__, ##__VA_ARGS__);                                              \
+        __PRAGMA_POP_NO_EXTRA_ARG_WARNINGS                                                 \
+      }                                                                                    \
+    }                                                                                      \
+  } while (0);
+
+/** Asserts by logging to the console and throwing an exception if NS_BLOCK_ASSERTIONS is not
+ * defined.
+ *
+ * @param condition The condition you'd expect to be YES.
+ */
+#define GDTFatalAssert(condition, ...)                                                  \
+  do {                                                                                  \
+    __PRAGMA_PUSH_NO_EXTRA_ARG_WARNINGS                                                 \
+    if (__builtin_expect(!(condition), 0)) {                                            \
+      NSString *__assert_file__ = [NSString stringWithUTF8String:__FILE__];             \
+      __assert_file__ = __assert_file__ ? __assert_file__ : @"<Unknown File>";          \
+      GDTLogError(GDTMCEFatalAssertion,                                                 \
+                  @"Fatal assertion encountered, please open an issue at "              \
+                   "https://github.com/firebase/firebase-ios-sdk/issues "               \
+                   "(%@:%d): %s,",                                                      \
+                  __assert_file__, __LINE__, ##__VA_ARGS__);                            \
+      [[NSAssertionHandler currentHandler] handleFailureInMethod:_cmd                   \
+                                                          object:self                   \
+                                                            file:__assert_file__        \
+                                                      lineNumber:__LINE__               \
+                                                     description:@"%@", ##__VA_ARGS__]; \
+    }                                                                                   \
+    __PRAGMA_POP_NO_EXTRA_ARG_WARNINGS                                                  \
+  } while (0);
+
+#endif  // defined(NS_BLOCK_ASSERTIONS)
diff --git a/GoogleDataTransport/GDTLibrary/Public/GDTConsoleLogger.h b/GoogleDataTransport/GDTLibrary/Public/GDTConsoleLogger.h
@@ -56,7 +56,12 @@ typedef NS_ENUM(NSInteger, GDTMessageCode) {
   GDTMCETransportBytesError = 1005,
 
   /** For general purpose error messages in a dependency. */
-  GDTMCEGeneralError = 1006
+  GDTMCEGeneralError = 1006,
+
+  /** For fatal errors. Please go to https://github.com/firebase/firebase-ios-sdk/issues and open
+   * an issue if you encounter an error with this code.
+   */
+  GDTMCEFatalAssertion = 1007
 };
 
 /** */

diff --git a/GoogleDataTransport/GDTTests/Integration/Helpers/GDTIntegrationTestUploader.m b/GoogleDataTransport/GDTTests/Integration/Helpers/GDTIntegrationTestUploader.m
@@ -16,6 +16,7 @@
 
 #import "GDTTests/Integration/Helpers/GDTIntegrationTestUploader.h"
 
+#import <GoogleDataTransport/GDTAssert.h>
 #import <GoogleDataTransport/GDTRegistrar.h>
 #import <GoogleDataTransport/GDTStoredEvent.h>
 
@@ -41,7 +42,8 @@ - (instancetype)initWithServerURL:(NSURL *)serverURL {
 }
 
 - (void)uploadPackage:(GDTUploadPackage *)package {
-  NSAssert(!_currentUploadTask, @"An upload shouldn't be initiated with another in progress.");
+  GDTFatalAssert(!_currentUploadTask,
+                 @"An upload shouldn't be initiated with another in progress.");
   NSURL *serverURL = arc4random_uniform(2) ? [_serverURL URLByAppendingPathComponent:@"log"]
                                            : [_serverURL URLByAppendingPathComponent:@"logBatch"];
   NSURLSession *session = [NSURLSession sharedSession];
@@ -54,24 +56,24 @@ - (void)uploadPackage:(GDTUploadPackage *)package {
   // In real usage, you'd create an instance of whatever request proto your server needs.
   for (GDTStoredEvent *event in package.events) {
     NSData *fileData = [NSData dataWithContentsOfURL:event.dataFuture.fileURL];
-    NSAssert(fileData, @"An event file shouldn't be empty");
+    GDTFatalAssert(fileData, @"An event file shouldn't be empty");
     [uploadData appendData:fileData];
   }
-  _currentUploadTask =
-      [session uploadTaskWithRequest:request
-                            fromData:uploadData
-                   completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response,
-                                       NSError *_Nullable error) {
-                     NSLog(@"Batch upload complete.");
-                     // Remove from the prioritizer if there were no errors.
-                     NSAssert(!error, @"There should be no errors uploading events: %@", error);
-                     if (error) {
-                       [package retryDeliveryInTheFuture];
-                     } else {
-                       [package completeDelivery];
-                     }
-                     self->_currentUploadTask = nil;
-                   }];
+  _currentUploadTask = [session
+      uploadTaskWithRequest:request
+                   fromData:uploadData
+          completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response,
+                              NSError *_Nullable error) {
+            NSLog(@"Batch upload complete.");
+            // Remove from the prioritizer if there were no errors.
+            GDTFatalAssert(!error, @"There should be no errors uploading events: %@", error);
+            if (error) {
+              [package retryDeliveryInTheFuture];
+            } else {
+              [package completeDelivery];
+            }
+            self->_currentUploadTask = nil;
+          }];
   [_currentUploadTask resume];
 }
 

diff --git a/GoogleDataTransport/GDTTests/Unit/GDTAssertTest.m b/GoogleDataTransport/GDTTests/Unit/GDTAssertTest.m
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2019 Google
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef NS_BLOCK_ASSERTIONS
+
+#import <XCTest/XCTest.h>
+
+#import <GoogleDataTransport/GDTAssert.h>
+
+@interface GDTAssertNotBlockedTest : XCTestCase
+
+@end
+
+@implementation GDTAssertNotBlockedTest
+
+/** Tests that asserting is innocuous and doesn't throw. */
+- (void)testNonFatallyAssertingDoesntThrow {
+  GDTAssert(NO, @"test assertion");
+}
+
+/** Tests that fatally asserting throws. */
+- (void)testFatallyAssertingThrows {
+  void (^assertionBlock)(void) = ^{
+    GDTFatalAssert(NO, @"test assertion")
+  };
+  XCTAssertThrows(assertionBlock());
+}
+@end