Merge pull request #143 from zadr/z/exception-refinement

Add ability to turn off logging after encountering an ObjC exception

CoreAardvark.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name     = 'CoreAardvark'
-  s.version  = '3.0.2'
+  s.version  = '3.1.0'
   s.license  = 'Apache License, Version 2.0'
   s.summary  = 'Aardvark is a library that makes it dead simple to create actionable bug reports. Usable by extensions.'
   s.homepage = 'https://github.com/square/Aardvark'

Sources/CoreAardvark/Logging/ARKExceptionLogging.m

@@ -81,7 +81,7 @@ void ARKHandleUncaughtException(NSException *_Nonnull exception)
     }
 }
 
-void ARKEnableLogOnUncaughtException()
+void ARKEnableLogOnUncaughtException(void)
 {
     ARKEnableLogOnUncaughtExceptionToLogDistributor([ARKLogDistributor defaultDistributor]);
 }
@@ -105,7 +105,7 @@ void ARKEnableLogOnUncaughtExceptionToLogDistributor(ARKLogDistributor *_Nonnull
     [logDistributorsLock unlock];
 }
 
-void ARKDisableLogOnUncaughtException()
+void ARKDisableLogOnUncaughtException(void)
 {
     ARKDisableLogOnUncaughtExceptionToLogDistributor([ARKLogDistributor defaultDistributor]);
 }

Sources/CoreAardvark/PrivateCategories/NSFileHandle+ARKAdditions.h

@@ -22,6 +22,10 @@ extern NSUInteger const ARKInvalidDataBlockLength;
 
 @interface NSFileHandle (ARKAdditions)
 
+/// Should Aardvark stop logging after catching a `NSException`?
+/// Default: `NO` to avoid changing API behavior within the same major (as in major.minor.patch versioning) version
++ (void)ARK_setPreventWritesAfterException:(BOOL)preventWritesAfterException;
+
 /// Writes the length of dataBlock, and then its contents. Note: this writes (or over-writes) at the current offsetInFile.
 - (void)ARK_writeDataBlock:(nonnull NSData *)dataBlock;
 

Sources/CoreAardvark/PrivateCategories/NSFileHandle+ARKAdditions.m

@@ -18,9 +18,23 @@
 
 #import "AardvarkDefines.h"
 
+#import <stdatomic.h>
+#import <stdbool.h>
+
 
 NSUInteger const ARKInvalidDataBlockLength = NSUIntegerMax;
 
+
+// If any thread hits an exception, stop all future logging to all threads:
+// 1. @throw'n exceptions are in Objective-C are not expected to be recoverable;
+//    it is preferred to halt all action that could result in further bad behavior.
+// 2. Any exception caught by an @catch block will leak its contents, and
+//    leaking memory every time we fail to log (for example: due to being out
+//    of disk space) is almost as bad as crashing.
+static _Atomic bool __ARKHasEncounteredDiskSizeException = false;
+static _Atomic bool __ARKPreventsWritesAfterException = false;
+
+
 /// Type convenience.
 typedef unsigned long long ARKFileOffset;
 
@@ -32,8 +46,32 @@
 
 @implementation NSFileHandle (ARKAdditions)
 
++ (void)ARK_setPreventWritesAfterException:(BOOL)preventWritesAfterException
+{
+    // convert from ObjC BOOL (a `char` on some platforms) to stdbool
+    // see: Special Considerations on https://developer.apple.com/documentation/objectivec/bool
+    bool stdboolPreventWritesAfterException = !!preventWritesAfterException;
+    atomic_store(&__ARKPreventsWritesAfterException, stdboolPreventWritesAfterException);
+
+    if (!stdboolPreventWritesAfterException) {
+        // if we are re-enabling logging, reset state that could prevent writes,
+        // in case we previously encountered an error
+        atomic_store(&__ARKHasEncounteredDiskSizeException, false);
+    }
+}
+
+#pragma mark -
+
 - (void)ARK_writeDataBlock:(NSData *)dataBlock;
 {
+    bool preventWritesAfterException = atomic_load(&__ARKPreventsWritesAfterException);
+    if (preventWritesAfterException) {
+        bool hasEncounteredException = atomic_load(&__ARKHasEncounteredDiskSizeException);
+        if (hasEncounteredException) {
+            return;
+        }
+    }
+
     NSUInteger dataBlockLength = dataBlock.length;
 
     ARKCheckCondition(dataBlockLength > 0, , @"Can't write data block %@", dataBlock);
@@ -47,9 +85,13 @@ - (void)ARK_writeDataBlock:(NSData *)dataBlock;
         [self writeData:dataLengthData];
         [self writeData:dataBlock];
     } @catch (NSException *exception) {
-        NSLog(@"ERROR: -[%@ %@] Unable to write data block (%@ bytes) to disk",
+        NSLog(@"ERROR: -[%@ %@] Unable to write data block (%@ bytes) to disk: %@",
               NSStringFromClass([self class]), NSStringFromSelector(_cmd),
-              @(dataBlockLength));
+              @(dataBlockLength), exception);
+
+        if ([exception.name isEqualToString:NSFileHandleOperationException]) {
+            atomic_store(&__ARKHasEncounteredDiskSizeException, true);
+        }
     }
 }
 

Sources/CoreAardvarkTests/ARKFileHandleAdditionsTests.m

@@ -106,6 +106,8 @@ - (void)tearDown;
     [self.fileHandle closeFile];
     self.fileHandle = nil;
 
+    [NSFileHandle ARK_setPreventWritesAfterException:NO];
+
     [super tearDown];
 }
 
@@ -398,6 +400,54 @@ - (void)test_throwingExceptionDuringWrite_doesNotCrash;
     [handle ARK_writeDataBlock:data];
 }
 
+- (void)test_throwingExceptionDuringWrite_preventsSubsequentWrites;
+{
+    [ARKFileHandle ARK_setPreventWritesAfterException:YES];
+
+    XCTestExpectation *expectation = [self expectationWithDescription:@"data was written"];
+    ARKFileHandle *handle = [[ARKFileHandle alloc] init];
+
+    __block BOOL hasWrittenDataAtLeastOnce = NO;
+    handle.writeDataBlock = ^{
+        [expectation fulfill];
+
+        if (hasWrittenDataAtLeastOnce) {
+            @throw [NSException exceptionWithName:NSFileHandleOperationException reason:@"out of space" userInfo:nil];
+        } else {
+            hasWrittenDataAtLeastOnce = YES;
+        }
+    };
+
+    NSData *data = [@"hello" dataUsingEncoding:NSUTF8StringEncoding];
+    [handle ARK_writeDataBlock:data];
+    [handle ARK_writeDataBlock:data];
+    [handle ARK_writeDataBlock:data];
+
+    [self waitForExpectationsWithTimeout:5.0 handler:nil];
+
+    XCTAssertTrue(hasWrittenDataAtLeastOnce);
+}
+
+- (void)test_throwingExceptionDuringWrite_doesNotPreventSubsequentWrites_whenNotEnabled;
+{
+    XCTestExpectation *expectation = [self expectationWithDescription:@"data was written"];
+    expectation.expectedFulfillmentCount = 3;
+
+    ARKFileHandle *handle = [[ARKFileHandle alloc] init];
+    handle.writeDataBlock = ^{
+        [expectation fulfill];
+
+        @throw [NSException exceptionWithName:NSFileHandleOperationException reason:@"out of space" userInfo:nil];
+    };
+
+    NSData *data = [@"hello" dataUsingEncoding:NSUTF8StringEncoding];
+    [handle ARK_writeDataBlock:data];
+    [handle ARK_writeDataBlock:data];
+    [handle ARK_writeDataBlock:data];
+
+    [self waitForExpectationsWithTimeout:5.0 handler:nil];
+}
+
 #pragma mark - Private Methods
 
 - (NSData *)_blockForData:(NSData *)data;