From c171be25459fa71990ed9b3dd73c02f34cd3a11a Mon Sep 17 00:00:00 2001 From: Matias Pequeno Date: Tue, 28 Mar 2023 19:15:13 -0300 Subject: [PATCH 1/4] Added logging config option to drop (default) or queue rate limited reps --- .../iosAppSwift/iosAppSwiftApp.swift | 8 ++- .../DTOs/RollbarLoggingOptions.m | 58 +++++++++++-------- .../RollbarDestinationRecord.h | 3 +- .../RollbarDestinationRecord.m | 15 +++-- .../Sources/RollbarNotifier/RollbarThread.m | 2 +- .../include/RollbarLoggingOptions.h | 19 +++++- 6 files changed, 69 insertions(+), 36 deletions(-) diff --git a/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift b/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift index 066112dd..d525d41e 100644 --- a/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift +++ b/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift @@ -31,11 +31,15 @@ class AppDelegate: NSObject, UIApplicationDelegate { config.loggingOptions.codeVersion = codeVersion + // Optionally defined whether rate limited occurrences should be dropped or + // kept in a queue. Defaults to drop. + //config.loggingOptions.rateLimitBehavior = .queue + // Optionally anonymize the IP address //config.loggingOptions.captureIp = RollbarCaptureIpType.anonymize - // Suppress (default) Rollbar internal logging - config.developerOptions.suppressSdkInfoLogging = true + // Suppress internal Rollbar logging. Defaults to true. + config.developerOptions.suppressSdkInfoLogging = false config.telemetry.enabled = true config.telemetry.captureLog = true diff --git a/RollbarNotifier/Sources/RollbarNotifier/DTOs/RollbarLoggingOptions.m b/RollbarNotifier/Sources/RollbarNotifier/DTOs/RollbarLoggingOptions.m index adf774da..43357de9 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/DTOs/RollbarLoggingOptions.m +++ b/RollbarNotifier/Sources/RollbarNotifier/DTOs/RollbarLoggingOptions.m @@ -5,6 +5,7 @@ static RollbarLevel const DEFAULT_LOG_LEVEL = RollbarLevel_Info; static RollbarLevel const DEFAULT_CRASH_LEVEL = RollbarLevel_Error; static NSUInteger const DEFAULT_MAX_REPORTS_PER_MINUTE = 60; +static RollbarRateLimitBehavior const DEFAULT_RATE_LIMIT_BEHAVIOR = RollbarRateLimitBehavior_Drop; static RollbarCaptureIpType const DEFAULT_IP_CAPTURE_TYPE = RollbarCaptureIpType_Full; #if TARGET_OS_IPHONE @@ -18,6 +19,7 @@ static NSString * const DFK_LOG_LEVEL = @"logLevel"; static NSString * const DFK_CRASH_LEVEL = @"crashLevel"; static NSString * const DFK_MAX_REPORTS_PER_MINUTE = @"maximumReportsPerMinute"; +static NSString * const DFK_RATE_LIMIT_BEHAVIOR = @"rateLimitBehavior"; static NSString * const DFK_IP_CAPTURE_TYPE = @"captureIp"; static NSString * const DFK_CODE_VERSION = @"codeVersion"; static NSString * const DFK_FRAMEWORK = @"framework"; @@ -32,15 +34,17 @@ @implementation RollbarLoggingOptions - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute + rateLimitBehavior:(RollbarRateLimitBehavior)rateLimitBehavior captureIp:(RollbarCaptureIpType)captureIp codeVersion:(nullable NSString *)codeVersion framework:(nullable NSString *)framework - requestId:(nullable NSString *)requestId { - + requestId:(nullable NSString *)requestId +{ self = [super initWithDictionary:@{ DFK_LOG_LEVEL: [RollbarLevelUtil RollbarLevelToString:logLevel], DFK_CRASH_LEVEL: [RollbarLevelUtil RollbarLevelToString:crashLevel], DFK_MAX_REPORTS_PER_MINUTE: [NSNumber numberWithUnsignedInteger:maximumReportsPerMinute], + DFK_RATE_LIMIT_BEHAVIOR: [NSNumber numberWithBool:rateLimitBehavior], DFK_IP_CAPTURE_TYPE: [RollbarCaptureIpTypeUtil CaptureIpTypeToString:DEFAULT_IP_CAPTURE_TYPE], DFK_CODE_VERSION: codeVersion ? codeVersion : [NSNull null], DFK_FRAMEWORK: framework ? framework : OPERATING_SYSTEM, @@ -52,13 +56,15 @@ - (instancetype)initWithLogLevel:(RollbarLevel)logLevel - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute + rateLimitBehavior:(RollbarRateLimitBehavior)rateLimitBehavior codeVersion:(nullable NSString *)codeVersion framework:(nullable NSString *)framework - requestId:(nullable NSString *)requestId { - + requestId:(nullable NSString *)requestId +{ self = [self initWithLogLevel:logLevel crashLevel:crashLevel maximumReportsPerMinute:maximumReportsPerMinute + rateLimitBehavior:rateLimitBehavior captureIp:DEFAULT_IP_CAPTURE_TYPE codeVersion:codeVersion framework:framework @@ -71,11 +77,12 @@ - (instancetype)initWithLogLevel:(RollbarLevel)logLevel captureIp:(RollbarCaptureIpType)captureIp codeVersion:(nullable NSString *)codeVersion framework:(nullable NSString *)framework - requestId:(nullable NSString *)requestId { - + requestId:(nullable NSString *)requestId +{ self = [self initWithLogLevel:logLevel crashLevel:crashLevel maximumReportsPerMinute:DEFAULT_MAX_REPORTS_PER_MINUTE + rateLimitBehavior:DEFAULT_RATE_LIMIT_BEHAVIOR captureIp:captureIp codeVersion:codeVersion framework:framework @@ -87,11 +94,12 @@ - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel codeVersion:(nullable NSString *)codeVersion framework:(nullable NSString *)framework - requestId:(nullable NSString *)requestId { - + requestId:(nullable NSString *)requestId +{ self = [self initWithLogLevel:logLevel crashLevel:crashLevel maximumReportsPerMinute:DEFAULT_MAX_REPORTS_PER_MINUTE + rateLimitBehavior:DEFAULT_RATE_LIMIT_BEHAVIOR captureIp:DEFAULT_IP_CAPTURE_TYPE codeVersion:codeVersion framework:framework @@ -101,11 +109,13 @@ - (instancetype)initWithLogLevel:(RollbarLevel)logLevel - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel - maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute { - + maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute + rateLimitBehavior:(RollbarRateLimitBehavior)rateLimitBehavior +{ self = [self initWithLogLevel:logLevel crashLevel:crashLevel maximumReportsPerMinute:maximumReportsPerMinute + rateLimitBehavior:rateLimitBehavior captureIp:DEFAULT_IP_CAPTURE_TYPE codeVersion:nil framework:nil @@ -118,7 +128,8 @@ - (instancetype)initWithLogLevel:(RollbarLevel)logLevel return [self initWithLogLevel:logLevel crashLevel:crashLevel - maximumReportsPerMinute:DEFAULT_MAX_REPORTS_PER_MINUTE]; + maximumReportsPerMinute:DEFAULT_MAX_REPORTS_PER_MINUTE + rateLimitBehavior:DEFAULT_RATE_LIMIT_BEHAVIOR]; } - (instancetype)init { @@ -144,6 +155,11 @@ - (NSUInteger)maximumReportsPerMinute { withDefault:DEFAULT_MAX_REPORTS_PER_MINUTE]; } +- (RollbarRateLimitBehavior)rateLimitBehavior { + return [self safelyGetBoolByKey:DFK_RATE_LIMIT_BEHAVIOR + withDefault:DEFAULT_RATE_LIMIT_BEHAVIOR]; +} + - (RollbarCaptureIpType)captureIp { NSString *valueString = [self safelyGetStringByKey:DFK_IP_CAPTURE_TYPE]; return [RollbarCaptureIpTypeUtil CaptureIpTypeFromString:valueString]; @@ -167,8 +183,7 @@ @implementation RollbarMutableLoggingOptions #pragma mark - initializers --(instancetype)init { - +- (instancetype)init { if (self = [super init]) { return self; } @@ -180,6 +195,7 @@ -(instancetype)init { @dynamic logLevel; @dynamic crashLevel; @dynamic maximumReportsPerMinute; +@dynamic rateLimitBehavior; @dynamic captureIp; @dynamic codeVersion; @dynamic framework; @@ -199,31 +215,23 @@ - (void)setMaximumReportsPerMinute:(NSUInteger)value { [self setUInteger:value forKey:DFK_MAX_REPORTS_PER_MINUTE]; } +- (void)setRateLimitBehavior:(RollbarRateLimitBehavior)behavior { + [self setInteger:behavior forKey:DFK_RATE_LIMIT_BEHAVIOR]; +} + - (void)setCaptureIp:(RollbarCaptureIpType)value { NSString *valueString = [RollbarCaptureIpTypeUtil CaptureIpTypeToString:value]; [self setString:valueString forKey:DFK_IP_CAPTURE_TYPE]; } -//- (nullable NSString *)codeVersion { -// return [self getDataByKey:DFK_CODE_VERSION]; -//} - - (void)setCodeVersion:(nullable NSString *)value { [self setData:value byKey:DFK_CODE_VERSION]; } -//- (nullable NSString *)framework; { -// return [self getDataByKey:DFK_FRAMEWORK]; -//} - - (void)setFramework:(nullable NSString *)value { [self setData:value byKey:DFK_FRAMEWORK]; } -//- (nullable NSString *)requestId { -// return [self getDataByKey:DFK_REQUEST_ID]; -//} - - (void)setRequestId:(nullable NSString *)value { [self setData:value byKey:DFK_REQUEST_ID]; } diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h index f0dafd3b..086efdd5 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h @@ -30,7 +30,8 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)canPost; - (BOOL)canPostWithConfig:(nonnull RollbarConfig *)config; -- (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply; +- (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply + withConfig:(nonnull RollbarConfig *)config; - (instancetype)initWithConfig:(nonnull RollbarConfig *)config andRegistry:(nonnull RollbarRegistry *)registry diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m index 86520b3f..ace3393f 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m @@ -77,8 +77,9 @@ - (BOOL)canPostWithConfig:(nonnull RollbarConfig *)config { return shouldPost; } -- (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply { - +- (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply + withConfig:(nonnull RollbarConfig *)config +{ if (!reply) { //no response from the server to our lates POST of a payload, //let's hold on on posting to the destination for 1 minute: @@ -91,10 +92,6 @@ - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply { } switch(reply.statusCode) { - case 429: // too many requests - self->_nextLocalWindowStart = [NSDate dateWithTimeIntervalSinceNow:reply.remainingSeconds];; - self->_serverWindowRemainingCount = 0; - break; case 403: // access denied case 404: // not found //let's hold on on posting to the destination for 1 minute: @@ -104,6 +101,12 @@ - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply { self->_nextLocalWindowStart = self->_nextEarliestPost; self->_nextServerWindowStart = nil; return; // nothing else to do... + case 429: // too many requests + if (!config.loggingOptions.rateLimitBehavior) { + self->_nextLocalWindowStart = [NSDate dateWithTimeIntervalSinceNow:reply.remainingSeconds]; + self->_serverWindowRemainingCount = 0; + break; + } case 200: // OK case 400: // bad request case 413: // request entity too large diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m index 58895b07..007ff701 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m @@ -577,7 +577,7 @@ - (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload } RollbarPayloadPostReply *reply = [[RollbarSender new] sendPayload:payload usingConfig:config]; - [record recordPostReply:reply]; + [record recordPostReply:reply withConfig:config]; if (!reply) { return RollbarTriStateFlag_None; // nothing obviously wrong with the payload - just there was no deterministic diff --git a/RollbarNotifier/Sources/RollbarNotifier/include/RollbarLoggingOptions.h b/RollbarNotifier/Sources/RollbarNotifier/include/RollbarLoggingOptions.h index 4fd347a1..4777408f 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/include/RollbarLoggingOptions.h +++ b/RollbarNotifier/Sources/RollbarNotifier/include/RollbarLoggingOptions.h @@ -7,6 +7,11 @@ NS_ASSUME_NONNULL_BEGIN +typedef NS_ENUM(NSInteger, RollbarRateLimitBehavior) { + RollbarRateLimitBehavior_Drop, + RollbarRateLimitBehavior_Queue +}; + /// Models logging settings of a configuration @interface RollbarLoggingOptions : RollbarDTO @@ -21,6 +26,9 @@ NS_ASSUME_NONNULL_BEGIN /// Reporting rate limit @property (nonatomic, readonly) NSUInteger maximumReportsPerMinute; +/// Whether to drop or queue rate limited occurrences, defaults to drop. +@property (nonatomic, readonly) RollbarRateLimitBehavior rateLimitBehavior; + /// A way of capturing IP addresses @property (nonatomic, readonly) RollbarCaptureIpType captureIp; @@ -39,6 +47,7 @@ NS_ASSUME_NONNULL_BEGIN /// @param logLevel minimum log level to start logging from /// @param crashLevel log level to mark crash reports with /// @param maximumReportsPerMinute Reporting rate limit +/// @param rateLimitBehavior Whether to drop or queue rate limited occurrences /// @param captureIp a way of capturing IP addresses /// @param codeVersion a code version to mark payloads with /// @param framework A framework tag to mark payloads with @@ -46,6 +55,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute + rateLimitBehavior:(RollbarRateLimitBehavior)rateLimitBehavior captureIp:(RollbarCaptureIpType)captureIp codeVersion:(nullable NSString *)codeVersion framework:(nullable NSString *)framework @@ -55,12 +65,14 @@ NS_ASSUME_NONNULL_BEGIN /// @param logLevel minimum log level to start logging from /// @param crashLevel log level to mark crash reports with /// @param maximumReportsPerMinute Reporting rate limit +/// @param rateLimitBehavior Whether to drop or queue rate limited occurrences /// @param codeVersion a code version to mark payloads with /// @param framework A framework tag to mark payloads with /// @param requestId A request ID to mark payloads with - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute + rateLimitBehavior:(RollbarRateLimitBehavior)rateLimitBehavior codeVersion:(nullable NSString *)codeVersion framework:(nullable NSString *)framework requestId:(nullable NSString *)requestId; @@ -95,9 +107,11 @@ NS_ASSUME_NONNULL_BEGIN /// @param logLevel minimum log level to start logging from /// @param crashLevel log level to mark crash reports with /// @param maximumReportsPerMinute Reporting rate limit +/// @param rateLimitBehavior Whether to drop or queue rate limited occurrences - (instancetype)initWithLogLevel:(RollbarLevel)logLevel crashLevel:(RollbarLevel)crashLevel - maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute; + maximumReportsPerMinute:(NSUInteger)maximumReportsPerMinute + rateLimitBehavior:(RollbarRateLimitBehavior)rateLimitBehavior; /// Initializer /// @param logLevel minimum log level to start logging from @@ -125,6 +139,9 @@ NS_DESIGNATED_INITIALIZER; /// Reporting rate limit @property (nonatomic, readwrite) NSUInteger maximumReportsPerMinute; +/// Whether to drop or queue rate limited occurrences, defaults to drop. +@property (nonatomic, readwrite) RollbarRateLimitBehavior rateLimitBehavior; + /// A way of capturing IP addresses @property (nonatomic, readwrite) RollbarCaptureIpType captureIp; From ad155db3fe663c76ad4c6b0f1613f549f727abef Mon Sep 17 00:00:00 2001 From: Matias Pequeno Date: Tue, 28 Mar 2023 22:07:54 -0300 Subject: [PATCH 2/4] Drop payloads on rate limiting if configured to do so --- .../iosAppSwift/iosAppSwiftApp.swift | 2 +- .../Sources/RollbarCommon/DTOs/RollbarDTO.m | 5 +- .../Sources/RollbarNotifier/Rollbar.m | 7 +- .../RollbarDestinationRecord.h | 1 - .../RollbarDestinationRecord.m | 22 +-- .../RollbarPayloadRepository.m | 2 +- .../Sources/RollbarNotifier/RollbarRegistry.m | 28 +--- .../Sources/RollbarNotifier/RollbarSender.m | 9 +- .../Sources/RollbarNotifier/RollbarThread.h | 5 +- .../Sources/RollbarNotifier/RollbarThread.m | 149 ++++++++---------- 10 files changed, 91 insertions(+), 139 deletions(-) diff --git a/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift b/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift index d525d41e..c198dd9a 100644 --- a/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift +++ b/Demos/iosAppSwift/iosAppSwift/iosAppSwiftApp.swift @@ -33,7 +33,7 @@ class AppDelegate: NSObject, UIApplicationDelegate { // Optionally defined whether rate limited occurrences should be dropped or // kept in a queue. Defaults to drop. - //config.loggingOptions.rateLimitBehavior = .queue + config.loggingOptions.rateLimitBehavior = .queue // Optionally anonymize the IP address //config.loggingOptions.captureIp = RollbarCaptureIpType.anonymize diff --git a/RollbarCommon/Sources/RollbarCommon/DTOs/RollbarDTO.m b/RollbarCommon/Sources/RollbarCommon/DTOs/RollbarDTO.m index d4269002..6d040087 100644 --- a/RollbarCommon/Sources/RollbarCommon/DTOs/RollbarDTO.m +++ b/RollbarCommon/Sources/RollbarCommon/DTOs/RollbarDTO.m @@ -330,7 +330,8 @@ -(BOOL)isEmpty { #pragma mark - Initializers -- (instancetype)initWithJSONString: (NSString *)jsonString { +- (instancetype)initWithJSONString:(NSString *)jsonString { + NSAssert(jsonString && jsonString.length > 0, @"jsonString cannot be nil"); self = [self init]; if (self) { [self deserializeFromJSONString:jsonString]; @@ -338,7 +339,7 @@ - (instancetype)initWithJSONString: (NSString *)jsonString { return self; } -- (instancetype)initWithJSONData: (NSData *)data { +- (instancetype)initWithJSONData:(NSData *)data { self = [self init]; if (self) { [self deserializeFromJSONData:data]; diff --git a/RollbarNotifier/Sources/RollbarNotifier/Rollbar.m b/RollbarNotifier/Sources/RollbarNotifier/Rollbar.m index 1813b181..5a4b4f1b 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/Rollbar.m +++ b/RollbarNotifier/Sources/RollbarNotifier/Rollbar.m @@ -380,16 +380,11 @@ + (void)criticalError:(NSError *)error data:(NSDictionary *)data #pragma mark - Send manually constructed JSON payload + (void)sendJsonPayload:(NSData *)payload { - - [[RollbarThread sharedInstance] sendPayload:payload - usingConfig:[Rollbar configuration] - ]; + [[RollbarThread sharedInstance] sendPayload:payload withConfig:[Rollbar configuration]]; } - #pragma mark - Telemetry API - #pragma mark - Dom + (void)recordViewEventForLevel:(RollbarLevel)level diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h index 086efdd5..280fee45 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.h @@ -28,7 +28,6 @@ NS_ASSUME_NONNULL_BEGIN @property (readonly, nonnull) RollbarRegistry *registry; -- (BOOL)canPost; - (BOOL)canPostWithConfig:(nonnull RollbarConfig *)config; - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply withConfig:(nonnull RollbarConfig *)config; diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m index ace3393f..b2e96a9e 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m @@ -49,14 +49,6 @@ - (instancetype)initWithDestinationID:(nonnull NSString *)destinationID #pragma mark - methods -- (BOOL)canPost { - if (!self->_nextEarliestPost) { - return NO; - } - - return [self->_nextEarliestPost compare:[NSDate date]] != NSOrderedDescending; -} - - (BOOL)canPostWithConfig:(nonnull RollbarConfig *)config { if (self->_nextLocalWindowStart && (self->_localWindowCount >= config.loggingOptions.maximumReportsPerMinute)) { // we already exceeded local rate limits, let's wait till the next local rate limiting window: @@ -71,7 +63,7 @@ - (BOOL)canPostWithConfig:(nonnull RollbarConfig *)config { BOOL shouldPost = [self->_nextEarliestPost compare:[NSDate date]] != NSOrderedDescending; if (shouldPost) { - RBLog(@"%@ ≤ %@ :: GO", self->_nextEarliestPost, [NSDate date]); + RBLog(@"Rate limit %@ ≤ %@ :: GO", self->_nextEarliestPost, [NSDate date]); } return shouldPost; @@ -91,9 +83,10 @@ - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply return; // nothing else to do... } - switch(reply.statusCode) { + switch (reply.statusCode) { case 403: // access denied case 404: // not found + RBLog(@"\tQueuing record"); //let's hold on on posting to the destination for 1 minute: self->_nextEarliestPost = [NSDate dateWithTimeIntervalSinceNow:60]; self->_localWindowCount = 0; @@ -102,7 +95,8 @@ - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply self->_nextServerWindowStart = nil; return; // nothing else to do... case 429: // too many requests - if (!config.loggingOptions.rateLimitBehavior) { + if (config.loggingOptions.rateLimitBehavior == RollbarRateLimitBehavior_Queue) { + RBLog(@"\tQueuing record"); self->_nextLocalWindowStart = [NSDate dateWithTimeIntervalSinceNow:reply.remainingSeconds]; self->_serverWindowRemainingCount = 0; break; @@ -112,6 +106,7 @@ - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply case 413: // request entity too large case 422: // unprocessable entity default: + RBLog(@"\tDropping record"); self->_nextServerWindowStart = [NSDate dateWithTimeIntervalSinceNow:reply.remainingSeconds];; self->_serverWindowRemainingCount = reply.remainingCount; if (self->_nextLocalWindowStart) { @@ -153,7 +148,6 @@ - (nonnull NSString *)description { " nextLocalWindowStart: %@\n" " nextServerWindowStart: %@\n" " nextEarliestPost: %@\n" - " canPost: %@\n" , super.description, self->_destinationID, @@ -162,9 +156,7 @@ - (nonnull NSString *)description { self->_serverWindowRemainingCount, self->_nextLocalWindowStart, self->_nextServerWindowStart, - self->_nextEarliestPost, - [self canPost] ? @"YES" : @"NO" - ]; + self->_nextEarliestPost]; return description; } diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarPayloadRepository.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarPayloadRepository.m index 44237b38..316023d6 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarPayloadRepository.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarPayloadRepository.m @@ -234,7 +234,7 @@ - (nonnull NSString *)getIDofDestinationWithEndpoint:(nonnull NSString *)endpoin } - (nullable NSDictionary *)getDestinationByID:(nonnull NSString *)destinationID { - + NSAssert(destinationID && destinationID.length > 0, @"destinationID cannot be nil"); NSString *sql = [NSString stringWithFormat: @"SELECT * FROM destinations WHERE id = '%@'", destinationID diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarRegistry.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarRegistry.m index 0bdc899b..a3915620 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarRegistry.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarRegistry.m @@ -17,7 +17,6 @@ - (instancetype)init { } - (nonnull RollbarDestinationRecord *)getRecordForConfig:(nonnull RollbarConfig *)config { - NSAssert(config, @"Config must not be null!"); NSAssert(config.destination, @"Destination must not be null!"); NSAssert(config.destination.endpoint, @"destination.endpoint must not be null!"); @@ -27,18 +26,14 @@ - (nonnull RollbarDestinationRecord *)getRecordForConfig:(nonnull RollbarConfig RollbarDestinationRecord *destinationRecord = [self getRecordForEndpoint:config.destination.endpoint andAccessToken:config.destination.accessToken]; - if (destinationRecord.localWindowLimit < config.loggingOptions.maximumReportsPerMinute) { - - // we use lagest configured limit per destination: - destinationRecord.localWindowLimit = config.loggingOptions.maximumReportsPerMinute; - } - + destinationRecord.localWindowLimit = MAX(destinationRecord.localWindowLimit, config.loggingOptions.maximumReportsPerMinute); + return destinationRecord; } - (nonnull RollbarDestinationRecord *)getRecordForEndpoint:(nonnull NSString *)endpoint - andAccessToken:(nonnull NSString *)accessToken { - + andAccessToken:(nonnull NSString *)accessToken \ +{ NSAssert(endpoint, @"endpoint must not be null!"); NSAssert(accessToken, @"accessToken must not be null!"); @@ -46,24 +41,13 @@ - (nonnull RollbarDestinationRecord *)getRecordForEndpoint:(nonnull NSString *)e RollbarDestinationRecord *destinationRecord = self->_destinationRecords[destinationID]; if (!destinationRecord) { destinationRecord = [[RollbarDestinationRecord alloc] initWithDestinationID:destinationID - andRegistry:self - ]; + andRegistry:self]; self->_destinationRecords[destinationID] = destinationRecord; } - -// if (destinationRecord.localWindowLimit < config.loggingOptions.maximumReportsPerMinute) { -// -// // we use lagest configured limit per destination: -// destinationRecord.localWindowLimit = config.loggingOptions.maximumReportsPerMinute; -// } - + return destinationRecord; } - - - - - (NSUInteger)totalDestinationRecords { return self->_destinationRecords.count; } diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarSender.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarSender.m index b6261c98..b80bd269 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarSender.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarSender.m @@ -90,6 +90,7 @@ - (nullable NSHTTPURLResponse *)postPayload:(nonnull NSData *)payload session = [NSURLSession sessionWithConfiguration:sessionConfig]; } + RBLog(@"\tSending payload..."); NSURLSessionDataTask *dataTask = [session dataTaskWithRequest:request completionHandler:^(NSData *data, NSURLResponse *response, NSError *error) { httpResponse = [self checkPayloadResponse:response error:error]; @@ -107,11 +108,11 @@ - (nullable NSHTTPURLResponse *)checkPayloadResponse:(NSURLResponse *)response e NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response; if (httpResponse.statusCode == 200) { - RBLog(@"OK response from Rollar"); + RBLog(@"\tOK response from Rollbar"); } else { - RBLog(@"There was a problem reporting to Rollbar:"); - RBLog(@"\tError: %@", [error localizedDescription]); - RBLog(@"\tResponse: %@", httpResponse); + RBLog(@"\tThere was a problem reporting to Rollbar:"); + RBLog(@"\t\tError: %@", [error localizedDescription]); + RBLog(@"\t\tResponse: %d", httpResponse.statusCode); } return httpResponse; diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h index 75f580ca..b9be7422 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h @@ -2,6 +2,7 @@ #import "RollbarConfig.h" #import "RollbarPayload.h" +#import "RollbarDestinationRecord.h" NS_ASSUME_NONNULL_BEGIN @@ -10,13 +11,11 @@ NS_ASSUME_NONNULL_BEGIN /// Signifies that the thread is active or not. @property(atomic) BOOL active; -//- (void)persist:(nonnull RollbarPayload *)payload; - - (void)persistPayload:(nonnull RollbarPayload *)payload withConfig:(nonnull RollbarConfig *)config; - (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload - usingConfig:(nonnull RollbarConfig *)config; + withConfig:(nonnull RollbarConfig *)config; /// Hides the initializer. - (instancetype)init NS_UNAVAILABLE; diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m index 007ff701..773d539c 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m @@ -106,11 +106,11 @@ - (void)run { NSTimeInterval timeIntervalInSeconds = 60.0 / _maxReportsPerMinute; _timer = [NSTimer timerWithTimeInterval:timeIntervalInSeconds - target:self - selector:@selector(checkItems) - userInfo:nil - repeats:YES - ]; + target:self + selector:@selector(checkItems) + userInfo:nil + repeats:YES + ]; NSRunLoop *runLoop = [NSRunLoop currentRunLoop]; [runLoop addTimer:_timer forMode:NSDefaultRunLoopMode]; @@ -159,7 +159,7 @@ + (nullable RollbarPayload *)modifyPayload:(nonnull RollbarPayload *)payload withConfig:(nonnull RollbarConfig *)config { if (config.modifyRollbarData) { - + @try { payload.data = config.modifyRollbarData(payload.data); } @@ -229,7 +229,7 @@ + (nonnull NSSet *)getScrubFields:(nullable RollbarScrubbingOptions *)scrubbingO } + (void)createMutableDataWithData:(NSMutableDictionary *)data - forPath:(NSString *)path { + forPath:(NSString *)path { NSArray *pathComponents = [path componentsSeparatedByString:@"."]; NSString *currentPath = @""; @@ -295,8 +295,6 @@ - (void)queuePayload_OnlyCallOnThisThread:(nonnull NSArray *)data { } - (void)savePayload:(nonnull RollbarPayload *)payload withConfig:(nonnull RollbarConfig *)config { - RBLog(@"RollbarThread::savePayload: %@", payload.data.body); - if ([RollbarThread shouldIgnorePayload:payload withConfig:config]) { if (config.developerOptions.logIncomingPayloads) { @@ -376,8 +374,8 @@ - (void)savePayload:(nonnull RollbarPayload *)payload withConfig:(nonnull Rollba RBErr(@"invalid configJson!"); return; } - - //[payload.data.notifier setData:config.jsonFriendlyData byKey:@"configured_options"]; + + RBLog(@"Queuing %@ (%d in queue)", payload.data.uuid, [self->_payloadsRepo getPayloadCount]); NSString *payloadJson = [payload serializeToJSONString]; NSDictionary *payloadDataRow = [self->_payloadsRepo addPayload:payloadJson withConfig:configJson @@ -421,8 +419,8 @@ - (BOOL)checkProcessStalePayload:(nonnull NSDictionary * ) { // we either have some sort of timestamp corruption or // we are processing a stale payload let's just drop it and call it done: - [self->_payloadsRepo removePayloadByID:payloadDataRow[@"id"]]; - + [self removePayloadByID:payloadDataRow[@"id"]]; + RollbarConfig *config = [[RollbarConfig alloc] initWithJSONString:payloadDataRow[@"config_json"]]; if (config && config.developerOptions.logTransmittedPayloads) { @@ -444,65 +442,45 @@ - (BOOL)checkProcessStalePayload:(nonnull NSDictionary * } - (void)processSavedPayload:(nonnull NSDictionary *)payloadDataRow { - if ([self checkProcessStalePayload:payloadDataRow]) { return; } - NSString *destinationKey = payloadDataRow[@"destination_key"]; - NSAssert(destinationKey && destinationKey.length > 0, @"destination_key is expected to be defined!"); - NSDictionary *destination = [self->_payloadsRepo getDestinationByID:destinationKey]; - - //TODO: remove this code-block before the upcoming major release: - if (!destination) { - RBLog(@"Aha!"); - [self->_payloadsRepo removePayloadByID:payloadDataRow[@"id"]]; - return; - } - - NSAssert(destination, @"destination can not be nil!"); - NSAssert(destination[@"endpoint"], @"destination endpoint can not be nil!"); - NSAssert(destination[@"access_token"], @"destination access_token can not be nil!"); + NSDictionary *destination = [self->_payloadsRepo getDestinationByID:payloadDataRow[@"destination_key"]]; RollbarDestinationRecord *destinationRecord = [self->_registry getRecordForEndpoint:destination[@"endpoint"] andAccessToken:destination[@"access_token"]]; - NSString *configJson = payloadDataRow[@"config_json"]; - NSAssert(configJson && configJson.length > 0, @"config_json is expected to be defined!"); - RollbarConfig *config = [[RollbarConfig alloc] initWithJSONString:configJson]; - NSAssert(config, @"config is expected to be defined!"); - + RollbarConfig *config = [[RollbarConfig alloc] initWithJSONString:payloadDataRow[@"config_json"]]; + RollbarPayload *payload = [[RollbarPayload alloc] initWithJSONString:payloadDataRow[@"payload_json"]]; + if (![destinationRecord canPostWithConfig:config]) { + if (config.loggingOptions.rateLimitBehavior == RollbarRateLimitBehavior_Drop) { + RBLog(@"Processing %@ (%d in queue)", payload.data.uuid, [self->_payloadsRepo getPayloadCount]); + RBLog(@"\tRate limited"); + [self removePayloadByID:payloadDataRow[@"id"]]; + RBLog(@"Dropped %@", payload.data.uuid); + } return; } - - NSString *payloadJson = payloadDataRow[@"payload_json"]; - RollbarPayload *payload = [[RollbarPayload alloc] initWithJSONString:payloadJson]; - NSAssert(payload, @"payload is expected to be defined!"); - + + RBLog(@"Processing %@ (%d in queue)", payload.data.uuid, [self->_payloadsRepo getPayloadCount]); + NSError *error; NSData *jsonPayload = [NSJSONSerialization rollbar_dataWithJSONObject:payload.jsonFriendlyData options:0 error:&error safe:true]; - if (nil == jsonPayload) { + if (jsonPayload == nil) { RBErr(@"Couldn't send jsonPayload that is nil"); - if (nil != error) { - RBErr(@" DETAILS: an error while generating JSON data: %@", error); - } - // there is nothing we can do with this payload - let's drop it: - RBErr(@"Dropping unprocessable payload: %@", payloadJson); - if (![self->_payloadsRepo removePayloadByID:payloadDataRow[@"id"]]) { - RBErr(@"Couldn't remove payload data row with ID: %@", payloadDataRow[@"id"]); + if (error != nil) { + RBErr(@"\tError while generating JSON data: %@", error); } + [self removePayloadByID:payloadDataRow[@"id"]]; return; } - RBLog(@"Processing %d payloads left", [self->_payloadsRepo getPayloadCount]); - RollbarTriStateFlag result = RollbarTriStateFlag_On; - if (!config) { - result = [self sendPayload:jsonPayload]; // backward compatibility with just upgraded very old SDKs... - } else if (config.developerOptions.transmit) { - result = [self sendPayload:jsonPayload usingConfig:config]; + if (config.developerOptions.transmit) { + result = [self sendPayload:jsonPayload toDestination:destinationRecord withConfig:config]; } NSString *payloadsLogFile = nil; @@ -510,20 +488,14 @@ - (void)processSavedPayload:(nonnull NSDictionary *)payl switch (result) { case RollbarTriStateFlag_On: // The payload is fully processed and transmitted. - // It can be removed from the repo: - if (![self->_payloadsRepo removePayloadByID:payloadDataRow[@"id"]]) { - RBErr(@"Couldn't remove payload data row with ID: %@", payloadDataRow[@"id"]); - } + [self removePayloadByID:payloadDataRow[@"id"]]; if (config.developerOptions.logTransmittedPayloads) { payloadsLogFile = config.developerOptions.transmittedPayloadsLogFile; } break; case RollbarTriStateFlag_Off: // The payload is fully processed but not accepted by the server due to some invalid content. - // It must be removed from the repo: - if (![self->_payloadsRepo removePayloadByID:payloadDataRow[@"id"]]) { - RBErr(@"Couldn't remove payload data row with ID: %@", payloadDataRow[@"id"]); - } + [self removePayloadByID:payloadDataRow[@"id"]]; if (config.developerOptions.logDroppedPayloads) { payloadsLogFile = config.developerOptions.droppedPayloadsLogFile; } @@ -541,11 +513,10 @@ - (void)processSavedPayload:(nonnull NSDictionary *)payl [RollbarFileWriter appendSafelyData:jsonPayload toFile:payloadsLogFilePath]; } - RBLog([self loggableStringFromPayload:jsonPayload result:result]); + RBLog([self loggableStringFromPayload:payload result:result]); } - (void)processSavedItems { - #if !TARGET_OS_WATCH if (!self->_isNetworkReachable) { RBLog(@"Processing saved items: no network!"); @@ -565,17 +536,23 @@ - (void)processSavedItems { } - (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload - usingConfig:(nonnull RollbarConfig *)config + withConfig:(nonnull RollbarConfig *)config +{ + RollbarDestinationRecord *destination = [self->_registry getRecordForConfig:config]; + if ([destination canPostWithConfig:config]) { + return [self sendPayload:payload toDestination:destination withConfig:config]; + } + return RollbarTriStateFlag_None; // nothing obviously wrong with the payload - just can not send at the moment +} + +- (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload + toDestination:(nullable RollbarDestinationRecord *)record + withConfig:(nonnull RollbarConfig *)config { if (!payload || !config) { return RollbarTriStateFlag_Off; //obviously invalid payload to sent or invalid destination... } - - RollbarDestinationRecord *record = [self->_registry getRecordForConfig:config]; - if (![record canPost]) { - return RollbarTriStateFlag_None; // nothing obviously wrong with the payload - just can not send at the moment - } - + RollbarPayloadPostReply *reply = [[RollbarSender new] sendPayload:payload usingConfig:config]; [record recordPostReply:reply withConfig:config]; @@ -584,28 +561,28 @@ - (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload // reply from the destination server } - switch(reply.statusCode) { + switch (reply.statusCode) { case 200: // OK return RollbarTriStateFlag_On; // the payload was successfully transmitted case 400: // bad request case 413: // request entity too large case 422: // unprocessable entity return RollbarTriStateFlag_Off; // unecceptable request/payload - should be dropped + case 429: // too many requests + switch (config.loggingOptions.rateLimitBehavior) { + case RollbarRateLimitBehavior_Queue: + return RollbarTriStateFlag_None; + case RollbarRateLimitBehavior_Drop: + default: + return RollbarTriStateFlag_Off; + } case 403: // access denied case 404: // not found - case 429: // too many requests default: return RollbarTriStateFlag_None; // worth retrying later } } -/// This is a DEPRECATED method left for some backward compatibility for very old clients eventually moving to this more recent implementation. -/// Use/maintain sendPayload:usingConfig: instead! -- (RollbarTriStateFlag)sendPayload:(NSData *)payload { - - return RollbarTriStateFlag_Off; -} - #pragma mark - Network telemetry data - (void)captureTelemetryDataForNetwork:(BOOL)reachable { @@ -638,16 +615,20 @@ - (void)captureTelemetryDataForNetwork:(BOOL)reachable { #pragma mark - -- (NSString *)loggableStringFromPayload:(NSData *)jsonPayload result:(RollbarTriStateFlag)result { - NSString *payloadString = [[NSString alloc] initWithData:jsonPayload encoding:NSUTF8StringEncoding]; - NSString *truncatedPayload = [payloadString substringToIndex:MIN(payloadString.length, 128)]; +- (void)removePayloadByID:(nonnull NSString *)payloadID { + if (![self->_payloadsRepo removePayloadByID:payloadID]) { + RBErr(@"\tCouldn't remove payload data row with ID: %@", payloadID); + } else { + RBLog(@"\tRecord dropped"); + } +} +- (NSString *)loggableStringFromPayload:(RollbarPayload *)payload result:(RollbarTriStateFlag)result { NSString *resultString = result == RollbarTriStateFlag_On ? @"Transmitted" : - result == RollbarTriStateFlag_Off ? @"Dropped" : - @"Unavailable will retry"; + result == RollbarTriStateFlag_Off ? @"Dropped" : @"Queued"; - return [NSString stringWithFormat:@"%@ payload: %@", resultString, truncatedPayload]; + return [NSString stringWithFormat:@"%@ %@", resultString, payload.data.uuid]; } #pragma mark - Singleton pattern From 141900d6d9d61fee2bf8437071cd2b7f579416b9 Mon Sep 17 00:00:00 2001 From: Matias Pequeno Date: Tue, 28 Mar 2023 22:47:20 -0300 Subject: [PATCH 3/4] Use global config for rate limit behavior instead of payload specific --- .../RollbarDestinationRecord.m | 7 +- .../RollbarNotifier/RollbarInfrastructure.m | 8 ++- .../Sources/RollbarNotifier/RollbarLogger.m | 22 ------- .../Sources/RollbarNotifier/RollbarThread.h | 12 +--- .../Sources/RollbarNotifier/RollbarThread.m | 66 ++++++++----------- 5 files changed, 43 insertions(+), 72 deletions(-) diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m index b2e96a9e..4fb2cb79 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarDestinationRecord.m @@ -1,5 +1,6 @@ #import "RollbarDestinationRecord.h" #import "RollbarRegistry.h" +#import "RollbarInfrastructure.h" #import "RollbarInternalLogging.h" @implementation RollbarDestinationRecord { @@ -8,6 +9,10 @@ @implementation RollbarDestinationRecord { #pragma mark - property accessors +- (RollbarRateLimitBehavior)rateLimitBehavior { + return RollbarInfrastructure.sharedInstance.configuration.loggingOptions.rateLimitBehavior; +} + #pragma mark - initializers - (instancetype)initWithConfig:(nonnull RollbarConfig *)config @@ -95,7 +100,7 @@ - (void)recordPostReply:(nullable RollbarPayloadPostReply *)reply self->_nextServerWindowStart = nil; return; // nothing else to do... case 429: // too many requests - if (config.loggingOptions.rateLimitBehavior == RollbarRateLimitBehavior_Queue) { + if (self.rateLimitBehavior == RollbarRateLimitBehavior_Queue) { RBLog(@"\tQueuing record"); self->_nextLocalWindowStart = [NSDate dateWithTimeIntervalSinceNow:reply.remainingSeconds]; self->_serverWindowRemainingCount = 0; diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarInfrastructure.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarInfrastructure.m index fd6a0927..364f9ec0 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarInfrastructure.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarInfrastructure.m @@ -6,6 +6,7 @@ #import "RollbarTelemetry.h" #import "RollbarCrashCollector.h" #import "RollbarInternalLogging.h" +#import "RollbarThread.h" NS_ASSUME_NONNULL_BEGIN @@ -49,8 +50,11 @@ - (nonnull instancetype)configureWith:(nonnull RollbarConfig *)config { [self.collector install]; [self.collector sendAllReports]; - NSLog(@"Rollbar is running") - + // Create RollbarThread and begin processing persisted occurrences + if ([[RollbarThread sharedInstance] active]) { + NSLog(@"Rollbar is running") + } + return self; } diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarLogger.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarLogger.m index 852c8a21..880b16f1 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarLogger.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarLogger.m @@ -186,38 +186,16 @@ - (BOOL)shouldSkipReporting:(RollbarLevel)level { #pragma mark - Payload queueing/reporting - (void)report:(RollbarPayload *)payload { - if (payload) { - [[RollbarThread sharedInstance] persistPayload:payload withConfig:self.configuration]; - -// NSDictionary *payloadJsonData = payload.jsonFriendlyData; -// if (payloadJsonData) { -// [[RollbarThread sharedInstance] persistPayload:payloadJsonData]; -// } } } #pragma mark - Update configuration methods - (void)updateConfiguration:(RollbarConfig *)configuration { - self.configuration = [configuration copy]; } -//- (void)updateAccessToken:(NSString *)accessToken { -// self.configuration.destination.accessToken = accessToken; -//} - -//- (void)updateReportingRate:(NSUInteger)maximumReportsPerMinute { -// if (nil != self.configuration) { -// self.configuration.loggingOptions.maximumReportsPerMinute = maximumReportsPerMinute; -// } -// -// //[[RollbarThread sharedInstance] cancel]; -// //[[RollbarThread sharedInstance] setReportingRate:maximumReportsPerMinute]; -// //[[RollbarThread sharedInstance] start]; -//} - @end diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h index b9be7422..4df679af 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.h @@ -17,19 +17,11 @@ NS_ASSUME_NONNULL_BEGIN - (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload withConfig:(nonnull RollbarConfig *)config; -/// Hides the initializer. - (instancetype)init NS_UNAVAILABLE; - -/// Hides the initializer. -- (instancetype)initWithTarget:(id)target - selector:(SEL)selector - object:(nullable id)argument -NS_UNAVAILABLE; - +- (instancetype)initWithTarget:(id)target selector:(SEL)selector object:(nullable id)argument NS_UNAVAILABLE; - (instancetype)initWithBlock:(void (^)(void))block NS_UNAVAILABLE; - -#pragma mark - Sigleton pattern +#pragma mark - Singleton pattern + (nonnull instancetype)sharedInstance; diff --git a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m index 773d539c..52e38297 100644 --- a/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m +++ b/RollbarNotifier/Sources/RollbarNotifier/RollbarThread.m @@ -13,12 +13,12 @@ #import "RollbarRegistry.h" #import "RollbarPayloadRepository.h" #import "RollbarInternalLogging.h" +#import "RollbarInfrastructure.h" static NSTimeInterval const DEFAULT_PAYLOAD_LIFETIME_SECONDS = 24 * 60 * 60; // hours-per day * 60 min-per-hour * 60 sec-per-min = 1 day in sec @implementation RollbarThread { - @private NSUInteger _maxReportsPerMinute; NSTimeInterval _payloadLifetimeInSeconds; @@ -31,32 +31,30 @@ @implementation RollbarThread { RollbarReachability *_reachability; BOOL _isNetworkReachable; #endif - } - (instancetype)initWithTarget:(id)target selector:(SEL)selector - object:(nullable id)argument { - - if ((self = [super initWithTarget:self - selector:@selector(run) - object:nil])) { - + object:(nullable id)argument +{ + self = [super initWithTarget:self + selector:@selector(run) + object:nil]; + if (self) { [self setupDataStorage]; - self->_maxReportsPerMinute = 240;//60; + self->_maxReportsPerMinute = 240; self->_payloadLifetimeInSeconds = DEFAULT_PAYLOAD_LIFETIME_SECONDS; self->_registry = [RollbarRegistry new]; - + #if !TARGET_OS_WATCH self->_reachability = nil; self->_isNetworkReachable = YES; #endif - self.name = [RollbarThread rollbar_objectClassName];//NSStringFromClass([RollbarThread class]); + self.name = [RollbarThread rollbar_objectClassName]; self.active = YES; - - + #if !TARGET_OS_WATCH // Listen for reachability status so that the items are only sent when the internet is available: self->_reachability = [RollbarReachability reachabilityForInternetConnection]; @@ -95,22 +93,16 @@ - (void)setupDataStorage { self->_payloadsRepo = [RollbarPayloadRepository persistentRepositoryWithPath:self->_payloadsRepoFilePath]; NSAssert([[NSFileManager defaultManager] fileExistsAtPath:self->_payloadsRepoFilePath], - @"Persistent payloads store was not created: %@!!!", self->_payloadsRepoFilePath - ); + @"Persistent payloads store was not created: %@!!!", self->_payloadsRepoFilePath); } - (void)run { - @autoreleasepool { - - NSTimeInterval timeIntervalInSeconds = 60.0 / _maxReportsPerMinute; - - _timer = [NSTimer timerWithTimeInterval:timeIntervalInSeconds + _timer = [NSTimer timerWithTimeInterval:60.0 / _maxReportsPerMinute target:self selector:@selector(checkItems) userInfo:nil - repeats:YES - ]; + repeats:YES]; NSRunLoop *runLoop = [NSRunLoop currentRunLoop]; [runLoop addTimer:_timer forMode:NSDefaultRunLoopMode]; @@ -127,7 +119,7 @@ - (void)persistPayload:(nonnull RollbarPayload *)payload withConfig:(nonnull RollbarConfig *)config { [self performSelector:@selector(queuePayload_OnlyCallOnThisThread:) - onThread:self //[RollbarThread sharedInstance] + onThread:self withObject:@[payload, config] waitUntilDone:NO ]; @@ -393,7 +385,6 @@ - (void)savePayload:(nonnull RollbarPayload *)payload withConfig:(nonnull Rollba #pragma mark - processing persisted payload items - (void)checkItems { - if (self.cancelled) { if (_timer) { [_timer invalidate]; @@ -408,15 +399,13 @@ - (void)checkItems { } - (BOOL)checkProcessStalePayload:(nonnull NSDictionary *)payloadDataRow { - - // let's make sure we are not dealng with a stale payload: + // let's make sure we are not dealing with a stale payload: NSString *timestampValue = payloadDataRow[@"created_at"]; NSScanner *scanner = [NSScanner scannerWithString:timestampValue]; double payloadTimestamp; - BOOL timestampParsingSuccess = [scanner scanDouble:&payloadTimestamp]; - if (!timestampParsingSuccess - || ((payloadTimestamp + self->_payloadLifetimeInSeconds) < [NSDate date].timeIntervalSince1970) - ) { + BOOL timestampDidParse = [scanner scanDouble:&payloadTimestamp]; + BOOL isStale = (payloadTimestamp + self->_payloadLifetimeInSeconds) < [[NSDate date] timeIntervalSince1970]; + if (!timestampDidParse || isStale) { // we either have some sort of timestamp corruption or // we are processing a stale payload let's just drop it and call it done: [self removePayloadByID:payloadDataRow[@"id"]]; @@ -433,7 +422,7 @@ - (BOOL)checkProcessStalePayload:(nonnull NSDictionary * } } - RBLog(@"Dropped a stale payload: %@", payloadDataRow[@"payload_json"]); + RBLog(@"Dropped stale payload: %@", payloadDataRow[@"payload_json"]); return YES; } @@ -449,11 +438,11 @@ - (void)processSavedPayload:(nonnull NSDictionary *)payl NSDictionary *destination = [self->_payloadsRepo getDestinationByID:payloadDataRow[@"destination_key"]]; RollbarDestinationRecord *destinationRecord = [self->_registry getRecordForEndpoint:destination[@"endpoint"] andAccessToken:destination[@"access_token"]]; - RollbarConfig *config = [[RollbarConfig alloc] initWithJSONString:payloadDataRow[@"config_json"]]; RollbarPayload *payload = [[RollbarPayload alloc] initWithJSONString:payloadDataRow[@"payload_json"]]; + RollbarConfig *config = [[RollbarConfig alloc] initWithJSONString:payloadDataRow[@"config_json"]]; if (![destinationRecord canPostWithConfig:config]) { - if (config.loggingOptions.rateLimitBehavior == RollbarRateLimitBehavior_Drop) { + if (self.rateLimitBehavior == RollbarRateLimitBehavior_Drop) { RBLog(@"Processing %@ (%d in queue)", payload.data.uuid, [self->_payloadsRepo getPayloadCount]); RBLog(@"\tRate limited"); [self removePayloadByID:payloadDataRow[@"id"]]; @@ -569,7 +558,7 @@ - (RollbarTriStateFlag)sendPayload:(nonnull NSData *)payload case 422: // unprocessable entity return RollbarTriStateFlag_Off; // unecceptable request/payload - should be dropped case 429: // too many requests - switch (config.loggingOptions.rateLimitBehavior) { + switch (self.rateLimitBehavior) { case RollbarRateLimitBehavior_Queue: return RollbarTriStateFlag_None; case RollbarRateLimitBehavior_Drop: @@ -615,6 +604,10 @@ - (void)captureTelemetryDataForNetwork:(BOOL)reachable { #pragma mark - +- (RollbarRateLimitBehavior)rateLimitBehavior { + return RollbarInfrastructure.sharedInstance.configuration.loggingOptions.rateLimitBehavior; +} + - (void)removePayloadByID:(nonnull NSString *)payloadID { if (![self->_payloadsRepo removePayloadByID:payloadID]) { RBErr(@"\tCouldn't remove payload data row with ID: %@", payloadID); @@ -634,16 +627,15 @@ - (NSString *)loggableStringFromPayload:(RollbarPayload *)payload result:(Rollba #pragma mark - Singleton pattern + (nonnull instancetype)sharedInstance { - static id singleton; static dispatch_once_t onceToken; - + dispatch_once(&onceToken, ^{ singleton = [[self alloc] initWithTarget:self selector:@selector(run) object:nil]; }); - + return singleton; } From 85201ee69554c4a27ce7bd622469f79469ad9691 Mon Sep 17 00:00:00 2001 From: Matias Pequeno Date: Tue, 28 Mar 2023 23:22:28 -0300 Subject: [PATCH 4/4] Updated unit tests to reflect changes. --- .../Tests/RollbarNotifierTests-ObjC/DTOsTests.m | 3 ++- .../RollbarRegistryTests.m | 14 +++++++------- .../RollbarNotifierDTOsTests.swift | 3 ++- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/DTOsTests.m b/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/DTOsTests.m index 4bc4b430..bc400fe3 100644 --- a/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/DTOsTests.m +++ b/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/DTOsTests.m @@ -282,7 +282,8 @@ - (void)testRollbarTelemetryOptionsDTO { - (void)testRollbarLoggingOptionsDTO { RollbarMutableLoggingOptions *dto = [[RollbarMutableLoggingOptions alloc] initWithLogLevel:RollbarLevel_Error crashLevel:RollbarLevel_Info - maximumReportsPerMinute:45]; + maximumReportsPerMinute:45 + rateLimitBehavior:RollbarRateLimitBehavior_Queue]; dto.captureIp = RollbarCaptureIpType_Anonymize; dto.codeVersion = @"CODEVERSION"; dto.framework = @"FRAMEWORK"; diff --git a/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/RollbarRegistryTests.m b/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/RollbarRegistryTests.m index b1da9799..ce8be89f 100644 --- a/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/RollbarRegistryTests.m +++ b/RollbarNotifier/Tests/RollbarNotifierTests-ObjC/RollbarRegistryTests.m @@ -40,22 +40,22 @@ - (void)testDestinationRecord { RollbarDestinationRecord *record = [registry getRecordForConfig:config]; XCTAssertEqual(1, registry.totalDestinationRecords); - XCTAssertEqual(YES, [record canPost]); + XCTAssertEqual(YES, [record canPostWithConfig:config]); XCTAssertNotNil(record.nextEarliestPost); XCTAssertNil(record.nextLocalWindowStart); XCTAssertNil(record.nextServerWindowStart); RollbarPayloadPostReply *reply = [RollbarPayloadPostReply greenReply]; - [record recordPostReply:reply]; - XCTAssertTrue([record canPost]); + [record recordPostReply:reply withConfig:config]; + XCTAssertTrue([record canPostWithConfig:config]); reply = [RollbarPayloadPostReply yellowReply]; - [record recordPostReply:reply]; - XCTAssertFalse([record canPost]); + [record recordPostReply:reply withConfig:config]; + XCTAssertFalse([record canPostWithConfig:config]); reply = [RollbarPayloadPostReply redReply]; - [record recordPostReply:reply]; - XCTAssertFalse([record canPost]); + [record recordPostReply:reply withConfig:config]; + XCTAssertFalse([record canPostWithConfig:config]); } #pragma mark - registry records tests diff --git a/RollbarNotifier/Tests/RollbarNotifierTests/RollbarNotifierDTOsTests.swift b/RollbarNotifier/Tests/RollbarNotifierTests/RollbarNotifierDTOsTests.swift index 913732f7..a95e9963 100644 --- a/RollbarNotifier/Tests/RollbarNotifierTests/RollbarNotifierDTOsTests.swift +++ b/RollbarNotifier/Tests/RollbarNotifierTests/RollbarNotifierDTOsTests.swift @@ -292,7 +292,8 @@ final class RollbarNotifierDTOsTests: XCTestCase { var dto = RollbarMutableLoggingOptions( logLevel: .error, crash: .info, - maximumReportsPerMinute: UInt(45) + maximumReportsPerMinute: UInt(45), + rateLimitBehavior: .queue ); dto.captureIp = .anonymize; dto.codeVersion = "CODEVERSION";