feat: Improve rate limiting and local file cache (#230)

* feat: Add shouldDiscardEvent to callback

* feat: Add shouldQueueEvent callback

* feat: Add tests for queueing

* fix: fastfile

* fix: typo

* feat: rename functions

Sources/Sentry/SentryBreadcrumbStore.m

@@ -41,7 +41,7 @@ - (instancetype)initWithFileManager:(SentryFileManager *)fileManager {
 
 - (void)addBreadcrumb:(SentryBreadcrumb *)crumb {
     [SentryLog logWithMessage:[NSString stringWithFormat:@"Add breadcrumb: %@", crumb] andLevel:kSentryLogLevelDebug];
-    [self.fileManager storeBreadcrumb:crumb];
+    [self.fileManager storeBreadcrumb:crumb maxCount:self.maxBreadcrumbs];
 }
 
 - (NSUInteger)count {
@@ -52,22 +52,10 @@ - (void)clear {
     [self.fileManager deleteAllStoredBreadcrumbs];
 }
 
-- (void)removeOverflowBreadcrumbs:(NSArray<NSDictionary<NSString *, id> *>*)breadCrumbs {
-    NSInteger numberOfBreadcrumbsToRemove = ((NSInteger)breadCrumbs.count) - ((NSInteger)self.maxBreadcrumbs);
-    if (numberOfBreadcrumbsToRemove > 0) {
-        for (NSUInteger i = 0; i < numberOfBreadcrumbsToRemove; i++) {
-            [self.fileManager removeFileAtPath:[breadCrumbs objectAtIndex:i][@"path"]];
-        }
-        [SentryLog logWithMessage:[NSString stringWithFormat:@"Dropped %ld breadcrumb(s) due limit", (long)numberOfBreadcrumbsToRemove]
-                         andLevel:kSentryLogLevelDebug];
-    }
-}
-
 - (NSDictionary<NSString *, id> *)serialize {
     NSMutableDictionary *serializedData = [NSMutableDictionary new];
 
     NSArray<NSDictionary<NSString *, id> *> *breadCrumbs = [self.fileManager getAllStoredBreadcrumbs];
-    [self removeOverflowBreadcrumbs:breadCrumbs];
 
     NSMutableArray *crumbs = [NSMutableArray new];
     for (NSDictionary<NSString *, id> *crumb in breadCrumbs) {

Sources/Sentry/SentryClient.m

@@ -82,11 +82,12 @@ - (_Nullable instancetype)initWithDsn:(NSString *)dsn
 }
 
 - (_Nullable instancetype)initWithDsn:(NSString *)dsn
-                       requestManager:(id <SentryRequestManager>)requestManager
+requestManager:(id <SentryRequestManager>)requestManager
                      didFailWithError:(NSError *_Nullable *_Nullable)error {
     self = [super init];
     if (self) {
         [self restoreContextBeforeCrash];
+        [self setupQueueing];
         _extra = [NSDictionary new];
         _tags = [NSDictionary new];
         self.dsn = [[SentryDsn alloc] initWithString:dsn didFailWithError:error];
@@ -106,6 +107,24 @@ - (_Nullable instancetype)initWithDsn:(NSString *)dsn
     return self;
 }
 
+- (void)setupQueueing {
+    self.shouldQueueEvent = ^BOOL(SentryEvent *_Nonnull event, NSHTTPURLResponse *_Nullable response, NSError *_Nullable error) {
+        // Taken from Apple Docs:
+        // If a response from the server is received, regardless of whether the request completes successfully or fails,
+        // the response parameter contains that information.
+        if (response == nil) {
+            // In case response is nil, we want to queue the event locally since this
+            // indicates no internet connection
+            return YES;
+        } else if ([response statusCode] == 429) {
+            [SentryLog logWithMessage:@"Rate limit reached, event will be stored and sent later" andLevel:kSentryLogLevelError];
+            return YES;
+        }
+        // In all other cases we don't want to retry sending it and just discard the event
+        return NO;
+    };
+}
+
 - (void)enableAutomaticBreadcrumbTracking {
     [[SentryBreadcrumbTracker alloc] start];
 }
@@ -189,14 +208,16 @@ - (void)    sendEvent:(SentryEvent *)event
     NSString *storedEventPath = [self.fileManager storeEvent:event];
 
     __block SentryClient *_self = self;
-    [self sendRequest:request withCompletionHandler:^(NSError *_Nullable error) {
+    [self sendRequest:request withCompletionHandler:^(NSHTTPURLResponse *_Nullable response, NSError *_Nullable error) {
+        // We check if we should leave the event locally stored and try to send it again later
+        if (self.shouldQueueEvent == nil || self.shouldQueueEvent(event, response, error) == NO) {
+            [_self.fileManager removeFileAtPath:storedEventPath];
+        }
         if (nil == error) {
             _self.lastEvent = event;
             [NSNotificationCenter.defaultCenter postNotificationName:@"Sentry/eventSentSuccessfully"
                                                               object:nil
                                                             userInfo:[event serialize]];
-            [_self.fileManager removeFileAtPath:storedEventPath];
-
             // Send all stored events in background if the queue is ready
             if ([_self.requestManager isReady]) {
                 [_self sendAllStoredEvents];
@@ -209,7 +230,7 @@ - (void)    sendEvent:(SentryEvent *)event
 }
 
 - (void)  sendRequest:(SentryNSURLRequest *)request
-withCompletionHandler:(_Nullable SentryRequestFinished)completionHandler {
+withCompletionHandler:(_Nullable SentryRequestOperationFinished)completionHandler {
     if (nil != self.beforeSendRequest) {
         self.beforeSendRequest(request);
     }
@@ -221,16 +242,20 @@ - (void)sendAllStoredEvents {
         SentryNSURLRequest *request = [[SentryNSURLRequest alloc] initStoreRequestWithDsn:self.dsn
                                                                                   andData:fileDictionary[@"data"]
                                                                          didFailWithError:nil];
-        [self sendRequest:request withCompletionHandler:^(NSError *_Nullable error) {
+        [self sendRequest:request withCompletionHandler:^(NSHTTPURLResponse *_Nullable response, NSError *_Nullable error) {
             if (nil == error) {
                 NSDictionary *serializedEvent = [NSJSONSerialization JSONObjectWithData:fileDictionary[@"data"]
-                                                                                 options:0
-                                                                                   error:nil];
+                                                                                options:0
+                                                                                  error:nil];
                 if (nil != serializedEvent) {
                     [NSNotificationCenter.defaultCenter postNotificationName:@"Sentry/eventSentSuccessfully"
                                                                       object:nil
                                                                     userInfo:serializedEvent];
                 }
+            }
+            // We want to delete the event here no matter what (if we had an internet connection)
+            // since it has been tried already
+            if (response != nil) {
                 [self.fileManager removeFileAtPath:fileDictionary[@"path"]];
             }
         }];

Sources/Sentry/SentryFileManager.m

@@ -26,6 +26,9 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+NSInteger const maxEvents = 10;
+NSInteger const maxBreadcrumbs = 200;
+
 @interface SentryFileManager ()
 
 @property(nonatomic, copy) NSString *sentryPath;
@@ -139,11 +142,23 @@ - (BOOL)removeFileAtPath:(NSString *)path {
 }
 
 - (NSString *)storeEvent:(SentryEvent *)event {
-    return [self storeDictionary:[event serialize] toPath:self.eventsPath];
+    return [self storeEvent:event maxCount:maxEvents];
+}
+
+- (NSString *)storeEvent:(SentryEvent *)event maxCount:(NSUInteger)maxCount {
+    NSString *result = [self storeDictionary:[event serialize] toPath:self.eventsPath];
+    [self handleFileManagerLimit:self.eventsPath maxCount:MIN(maxCount, maxEvents)];
+    return result;
 }
 
 - (NSString *)storeBreadcrumb:(SentryBreadcrumb *)crumb {
-    return [self storeDictionary:[crumb serialize] toPath:self.breadcrumbsPath];
+    return [self storeBreadcrumb:crumb maxCount:maxBreadcrumbs];
+}
+
+- (NSString *)storeBreadcrumb:(SentryBreadcrumb *)crumb maxCount:(NSUInteger)maxCount {
+    NSString *result = [self storeDictionary:[crumb serialize] toPath:self.breadcrumbsPath];
+    [self handleFileManagerLimit:self.breadcrumbsPath maxCount:MIN(maxCount, maxBreadcrumbs)];
+    return result;
 }
 
 - (NSString *)storeDictionary:(NSDictionary *)dictionary toPath:(NSString *)path {
@@ -159,6 +174,18 @@ - (NSString *)storeDictionary:(NSDictionary *)dictionary toPath:(NSString *)path
     }
 }
 
+- (void)handleFileManagerLimit:(NSString *)path maxCount:(NSUInteger)maxCount {
+    NSArray<NSString *> *files = [self allFilesInFolder:path];
+    NSInteger numbersOfFilesToRemove = ((NSInteger)files.count) - maxCount;
+    if (numbersOfFilesToRemove > 0) {
+        for (NSUInteger i = 0; i < numbersOfFilesToRemove; i++) {
+            [self removeFileAtPath:[path stringByAppendingPathComponent:[files objectAtIndex:i]]];
+        }
+        [SentryLog logWithMessage:[NSString stringWithFormat:@"Removed %ld file(s) from <%@>", (long)numbersOfFilesToRemove, [path lastPathComponent]]
+                         andLevel:kSentryLogLevelDebug];
+    }
+}
+
 + (BOOL)createDirectoryAtPath:(NSString *)path withError:(NSError **)error {
     NSFileManager *fileManager = [NSFileManager defaultManager];
     return [fileManager createDirectoryAtPath:path

Sources/Sentry/SentryQueueableRequestManager.m

@@ -45,13 +45,13 @@ - (BOOL)isReady {
     return self.queue.operationCount <= 1;
 }
 
-- (void)addRequest:(NSURLRequest *)request completionHandler:(_Nullable SentryRequestFinished)completionHandler {
+- (void)addRequest:(NSURLRequest *)request completionHandler:(_Nullable SentryRequestOperationFinished)completionHandler {
     SentryRequestOperation *operation = [[SentryRequestOperation alloc] initWithSession:self.session
                                                                                 request:request
-                                                                      completionHandler:^(NSError *_Nullable error) {
+                                                                      completionHandler:^(NSHTTPURLResponse *_Nullable response, NSError *_Nullable error) {
                                                                           [SentryLog logWithMessage:[NSString stringWithFormat:@"Queued requests: %@", @(self.queue.operationCount - 1)] andLevel:kSentryLogLevelDebug];
                                                                           if (completionHandler) {
-                                                                              completionHandler(error);
+                                                                              completionHandler(response, error);
                                                                           }
                                                                       }];
     [self.queue addOperation:operation];

Sources/Sentry/SentryRequestOperation.m

@@ -11,11 +11,13 @@
 #import <Sentry/SentryRequestOperation.h>
 #import <Sentry/SentryLog.h>
 #import <Sentry/SentryError.h>
+#import <Sentry/SentryClient.h>
 
 #else
 #import "SentryRequestOperation.h"
 #import "SentryLog.h"
 #import "SentryError.h"
+#import "SentryClient.h"
 #endif
 
 NS_ASSUME_NONNULL_BEGIN
@@ -30,32 +32,26 @@ @interface SentryRequestOperation ()
 @implementation SentryRequestOperation
 
 - (instancetype)initWithSession:(NSURLSession *)session request:(NSURLRequest *)request
-              completionHandler:(_Nullable SentryRequestFinished)completionHandler {
+              completionHandler:(_Nullable SentryRequestOperationFinished)completionHandler {
     self = [super init];
     if (self) {
         self.request = request;
         self.task = [session dataTaskWithRequest:self.request completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response, NSError *_Nullable error) {
-            NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *) response;
+            NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response;
             NSInteger statusCode = [httpResponse statusCode];
-
+
+            // We only have these if's here because of performance reasons
             [SentryLog logWithMessage:[NSString stringWithFormat:@"Request status: %ld", (long) statusCode] andLevel:kSentryLogLevelDebug];
-            [SentryLog logWithMessage:[NSString stringWithFormat:@"Request response: %@", [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]] andLevel:kSentryLogLevelVerbose];
-
+            if (SentryClient.logLevel == kSentryLogLevelVerbose) {
+                [SentryLog logWithMessage:[NSString stringWithFormat:@"Request response: %@", [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]] andLevel:kSentryLogLevelVerbose];
+            }
+
             if (nil != error) {
                 [SentryLog logWithMessage:[NSString stringWithFormat:@"Request failed: %@", error] andLevel:kSentryLogLevelError];
             }
 
-            NSError *requestError = nil;
-            if (statusCode >= 400 && statusCode <= 500) {
-                requestError = NSErrorFromSentryError(kSentryErrorRequestError, [NSString stringWithFormat:@"Request errored with %ld", (long) statusCode]);
-                if (statusCode == 429) {
-                    [SentryLog logWithMessage:@"Rate limit reached, event will be stored and sent later" andLevel:kSentryLogLevelError];
-                }
-                [SentryLog logWithMessage:[NSString stringWithFormat:@"Request failed: %@", requestError] andLevel:kSentryLogLevelError];
-            }
-
             if (completionHandler) {
-                completionHandler(error ? error : requestError);
+                completionHandler(httpResponse, error);
             }
 
             [self completeOperation];

Sources/Sentry/include/SentryClient.h

@@ -101,6 +101,14 @@ NS_SWIFT_NAME(Client)
  */
 @property(nonatomic) float sampleRate;
 
+/**
+ * This block can be used to prevent the event from being deleted after a failed send attempt.
+ * Default is it will only be stored once after you hit a rate limit or there is no internet connect/cannot connect.
+ * Also note that if an event fails to be sent again after it was queued, it will be discarded regardless.
+ * @return BOOL YES = store and try again later, NO = delete
+ */
+@property(nonatomic, copy) SentryShouldQueueEvent _Nullable shouldQueueEvent;
+
 /**
  * Initializes a SentryClient. Pass your private DSN string.
  *

Sources/Sentry/include/SentryDefines.h

@@ -39,6 +39,13 @@
  * Block used for returning after a request finished
  */
 typedef void (^SentryRequestFinished)(NSError *_Nullable error);
+
+/**
+ * Block used for request operation finished, shouldDiscardEvent is YES if event should be deleted
+ * regardless if an error occured or not
+ */
+typedef void (^SentryRequestOperationFinished)(NSHTTPURLResponse *_Nullable response, NSError *_Nullable error);
+
 /**
  * Block can be used to mutate event before its send
  */
@@ -48,9 +55,16 @@ typedef void (^SentryBeforeSerializeEvent)(SentryEvent *_Nonnull event);
  */
 typedef void (^SentryBeforeSendRequest)(SentryNSURLRequest *_Nonnull request);
 /**
- * Block can to prevent the event from being sent
+ * Block can be used to prevent the event from being sent
  */
 typedef BOOL (^SentryShouldSendEvent)(SentryEvent *_Nonnull event);
+/**
+ * Block can be used to determine if an event should be queued and stored locally.
+ * It will be tried to send again after next successful send.
+ * Note that this will only be called once the event is created and send manully.
+ * Once it has been queued once it will be discarded if it fails again.
+ */
+typedef BOOL (^SentryShouldQueueEvent)(SentryEvent *_Nonnull event, NSHTTPURLResponse *_Nullable response, NSError *_Nullable error);
 /**
  * Loglevel
  */

Sources/Sentry/include/SentryFileManager.h

@@ -24,8 +24,10 @@ SENTRY_NO_INIT
 - (_Nullable instancetype)initWithDsn:(SentryDsn *)dsn didFailWithError:(NSError **)error;
 
 - (NSString *)storeEvent:(SentryEvent *)event;
+- (NSString *)storeEvent:(SentryEvent *)event maxCount:(NSUInteger)maxCount;
 
 - (NSString *)storeBreadcrumb:(SentryBreadcrumb *)crumb;
+- (NSString *)storeBreadcrumb:(SentryBreadcrumb *)crumb maxCount:(NSUInteger)maxCount;
 
 + (BOOL)createDirectoryAtPath:(NSString *)path withError:(NSError **)error;
 

Sources/Sentry/include/SentryQueueableRequestManager.h

@@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (instancetype)initWithSession:(NSURLSession *)session;
 
-- (void)addRequest:(NSURLRequest *)request completionHandler:(_Nullable SentryRequestFinished)completionHandler;
+- (void)addRequest:(NSURLRequest *)request completionHandler:(_Nullable SentryRequestOperationFinished)completionHandler;
 
 - (void)cancelAllOperations;
 

Sources/Sentry/include/SentryRequestOperation.h

@@ -23,7 +23,7 @@ NS_ASSUME_NONNULL_BEGIN
 @interface SentryRequestOperation : SentryAsynchronousOperation
 
 - (instancetype)initWithSession:(NSURLSession *)session request:(NSURLRequest *)request
-              completionHandler:(_Nullable SentryRequestFinished)completionHandler;
+              completionHandler:(_Nullable SentryRequestOperationFinished)completionHandler;
 
 @end
 

Tests/SentryTests/SentryBreadcrumbTests.m

@@ -59,8 +59,6 @@ - (void)testBreadcumbLimit {
     for (NSInteger i = 0; i <= 100; i++) {
         [client.breadcrumbs addBreadcrumb:[self getBreadcrumb]];
     }
-    XCTAssertEqual(client.breadcrumbs.count, (unsigned long)101);
-    [client.breadcrumbs serialize];
     XCTAssertEqual(client.breadcrumbs.count, (unsigned long)50);
 
     [client.breadcrumbs clear];
@@ -75,8 +73,6 @@ - (void)testBreadcumbLimit {
     for (NSInteger i = 0; i < 51; i++) {
         [client.breadcrumbs addBreadcrumb:[self getBreadcrumb]];
     }
-    XCTAssertEqual(client.breadcrumbs.count, (unsigned long)51);
-    [client.breadcrumbs serialize];
     XCTAssertEqual(client.breadcrumbs.count, (unsigned long)50);
 }
 

Tests/SentryTests/SentryFileManagerTests.m

@@ -90,4 +90,22 @@ - (void)testFailingStoreDictionary {
     XCTAssertNil([self.fileManager storeDictionary:@{@"date": [NSDate date]} toPath:@""]);
 }
 
+- (void)testEventStoringHardLimit {
+    SentryEvent *event = [[SentryEvent alloc] initWithLevel:kSentrySeverityInfo];
+    for (NSInteger i = 0; i <= 20; i++) {
+        [self.fileManager storeEvent:event];
+    }
+    NSArray<NSDictionary<NSString *, NSData *>*> *events = [self.fileManager getAllStoredEvents];
+    XCTAssertEqual(events.count, (unsigned long)10);
+}
+
+- (void)testBreadcrumbStoringHardLimit {
+    SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] initWithLevel:kSentrySeverityInfo category:@"category"];
+    for (NSInteger i = 0; i <= 210; i++) {
+        [self.fileManager storeBreadcrumb:crumb];
+    }
+    NSArray<NSDictionary<NSString *, NSData *>*> *crumbs = [self.fileManager getAllStoredBreadcrumbs];
+    XCTAssertEqual(crumbs.count, (unsigned long)200);
+}
+
 @end

Tests/SentryTests/SentryInterfacesTests.m

@@ -275,6 +275,7 @@ - (void)testBreadcrumb {
 
 - (void)testBreadcrumbStore {
     SentryBreadcrumbStore *store = [[SentryBreadcrumbStore alloc] initWithFileManager:[[SentryFileManager alloc] initWithDsn:[[SentryDsn alloc] initWithString:@"https://username:[email protected]/12345" didFailWithError:nil] didFailWithError:nil]];
+    [store clear];
     SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] initWithLevel:kSentrySeverityInfo category:@"http"];
     [store addBreadcrumb:crumb];
     NSDate *date = [NSDate date];