From 74c9e5721cc6d4849eed5c55470a8437ad1ad987 Mon Sep 17 00:00:00 2001 From: Marvin Liu Date: Tue, 7 Feb 2023 18:04:10 -0800 Subject: [PATCH] fix: response handler when 200 and various errors (#424) * fix: response handler when 200 and various errors * fix: existing ios upload test * fix: add error status code * fix: remove request_db_write_failed error handling case --- Sources/Amplitude/Amplitude.m | 31 ++++++---------- Tests/AmplitudeTVOSTests.m | 2 +- Tests/AmplitudeTests.m | 70 +++++++++++++++++++++++++++++++++++ Tests/AmplitudeiOSTests.m | 3 +- 4 files changed, 85 insertions(+), 21 deletions(-) diff --git a/Sources/Amplitude/Amplitude.m b/Sources/Amplitude/Amplitude.m index c5321590..8c7cbdad 100644 --- a/Sources/Amplitude/Amplitude.m +++ b/Sources/Amplitude/Amplitude.m @@ -1020,26 +1020,20 @@ - (void)makeEventUploadPostRequest:(NSString *)url events:(NSString *)events num BOOL uploadSuccessful = NO; NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response; if (response != nil) { + NSString *result = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; if ([httpResponse statusCode] == 200) { - NSString *result = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; - if ([result isEqualToString:@"success"]) { - // success, remove existing events from dictionary - uploadSuccessful = YES; - if (maxEventId >= 0) { - (void) [self.dbHelper removeEvents:maxEventId]; - } - if (maxIdentifyId >= 0) { - (void) [self.dbHelper removeIdentifys:maxIdentifyId]; - } - } else if ([result isEqualToString:@"invalid_api_key"]) { - AMPLITUDE_ERROR(@"ERROR: Invalid API Key, make sure your API key is correct in initializeApiKey:"); - } else if ([result isEqualToString:@"bad_checksum"]) { - AMPLITUDE_ERROR(@"ERROR: Bad checksum, post request was mangled in transit, will attempt to reupload later"); - } else if ([result isEqualToString:@"request_db_write_failed"]) { - AMPLITUDE_ERROR(@"ERROR: Couldn't write to request database on server, will attempt to reupload later"); - } else { - AMPLITUDE_ERROR(@"ERROR: %@, will attempt to reupload later", result); + // success, remove existing events from dictionary + uploadSuccessful = YES; + if (maxEventId >= 0) { + (void) [self.dbHelper removeEvents:maxEventId]; } + if (maxIdentifyId >= 0) { + (void) [self.dbHelper removeIdentifys:maxIdentifyId]; + } + } else if ([httpResponse statusCode] == 400 && [result isEqualToString:@"invalid_api_key"]) { + AMPLITUDE_ERROR(@"ERROR: Invalid API Key, make sure your API key is correct in initializeApiKey"); + } else if ([httpResponse statusCode] == 400 && [result isEqualToString:@"bad_checksum"]) { + AMPLITUDE_ERROR(@"ERROR: Bad checksum, post request was mangled in transit, will attempt to reupload later"); } else if ([httpResponse statusCode] == 413) { // If blocked by one massive event, drop it if (numEvents == 1) { @@ -1058,7 +1052,6 @@ - (void)makeEventUploadPostRequest:(NSString *)url events:(NSString *)events num AMPLITUDE_LOG(@"Request too large, will decrease size and attempt to reupload"); self->_updatingCurrently = NO; [self uploadEventsWithLimit:self->_backoffUploadBatchSize]; - } else { AMPLITUDE_ERROR(@"ERROR: Connection response received:%ld, %@", (long)[httpResponse statusCode], [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]); diff --git a/Tests/AmplitudeTVOSTests.m b/Tests/AmplitudeTVOSTests.m index 4985c160..507092fb 100644 --- a/Tests/AmplitudeTVOSTests.m +++ b/Tests/AmplitudeTVOSTests.m @@ -54,7 +54,7 @@ - (void)setupAsyncResponse: (NSMutableDictionary*) serverResponse { - (void)testLogEventUploadLogic { NSMutableDictionary *serverResponse = [NSMutableDictionary dictionaryWithDictionary: - @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:200 HTTPVersion:nil headerFields:@{}], + @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:400 HTTPVersion:nil headerFields:@{}], @"data" : [@"bad_checksum" dataUsingEncoding:NSUTF8StringEncoding] }]; [self setupAsyncResponse:serverResponse]; diff --git a/Tests/AmplitudeTests.m b/Tests/AmplitudeTests.m index ab2013c4..13232342 100644 --- a/Tests/AmplitudeTests.m +++ b/Tests/AmplitudeTests.m @@ -308,6 +308,76 @@ - (void)testUUIDInEvent { XCTAssertNotEqual([events[0] objectForKey:@"uuid"], [events[1] objectForKey:@"uuid"]); } +- (void)testLogEventSuccess { + [self.amplitude setEventUploadThreshold:1]; + // configure response body to be random, as we only care about the status code + NSMutableDictionary *serverResponse = [NSMutableDictionary dictionaryWithDictionary: + @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:200 HTTPVersion:nil headerFields:@{}], + @"data" : [@"random-response-body" dataUsingEncoding:NSUTF8StringEncoding] + }]; + + [self setupAsyncResponse:serverResponse]; + [self.amplitude logEvent:@"test"]; + [self.amplitude flushQueue]; + + XCTAssertEqual(_connectionCallCount, 1); + XCTAssertEqual([self.databaseHelper getEventCount], 0); + XCTAssertEqual([self.databaseHelper getIdentifyCount], 0); + XCTAssertEqual([self.databaseHelper getTotalEventCount], 0); +} + +- (void)testLogEventFailedWithInvalidApiKeyResponse { + [self.amplitude setEventUploadThreshold:1]; + NSMutableDictionary *serverResponse = [NSMutableDictionary dictionaryWithDictionary: + @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:400 HTTPVersion:nil headerFields:@{}], + @"data" : [@"invalid_api_key" dataUsingEncoding:NSUTF8StringEncoding] + }]; + + [self setupAsyncResponse:serverResponse]; + [self.amplitude logEvent:@"test"]; + [self.amplitude flushQueue]; + + XCTAssertEqual(_connectionCallCount, 1); + XCTAssertEqual([self.databaseHelper getEventCount], 1); + XCTAssertEqual([self.databaseHelper getIdentifyCount], 0); + XCTAssertEqual([self.databaseHelper getTotalEventCount], 1); +} + +- (void)testLogEventFailedWithBadChecksumResponse { + [self.amplitude setEventUploadThreshold:1]; + NSMutableDictionary *serverResponse = [NSMutableDictionary dictionaryWithDictionary: + @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:400 HTTPVersion:nil headerFields:@{}], + @"data" : [@"bad_checksum" dataUsingEncoding:NSUTF8StringEncoding] + }]; + + [self setupAsyncResponse:serverResponse]; + [self.amplitude logEvent:@"test"]; + [self.amplitude flushQueue]; + + XCTAssertEqual(_connectionCallCount, 1); + XCTAssertEqual([self.databaseHelper getEventCount], 1); + XCTAssertEqual([self.databaseHelper getIdentifyCount], 0); + XCTAssertEqual([self.databaseHelper getTotalEventCount], 1); +} + +- (void)testLogEventFailedWithRequestDbWriteFailedResponse { + [self.amplitude setEventUploadThreshold:1]; + // Note that "request_db_write_failed" error response might not exist anymore, leave this test to make sure we can handle it. + NSMutableDictionary *serverResponse = [NSMutableDictionary dictionaryWithDictionary: + @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:500 HTTPVersion:nil headerFields:@{}], + @"data" : [@"request_db_write_failed" dataUsingEncoding:NSUTF8StringEncoding] + }]; + + [self setupAsyncResponse:serverResponse]; + [self.amplitude logEvent:@"test"]; + [self.amplitude flushQueue]; + + XCTAssertEqual(_connectionCallCount, 1); + XCTAssertEqual([self.databaseHelper getEventCount], 1); + XCTAssertEqual([self.databaseHelper getIdentifyCount], 0); + XCTAssertEqual([self.databaseHelper getTotalEventCount], 1); +} + - (void)testIdentify { AMPDatabaseHelper *dbHelper = [AMPDatabaseHelper getDatabaseHelper]; [self.amplitude setEventUploadThreshold:2]; diff --git a/Tests/AmplitudeiOSTests.m b/Tests/AmplitudeiOSTests.m index 377ce226..23d7c6a5 100644 --- a/Tests/AmplitudeiOSTests.m +++ b/Tests/AmplitudeiOSTests.m @@ -53,7 +53,7 @@ - (void)setupAsyncResponse: (NSMutableDictionary*) serverResponse { - (void)testLogEventUploadLogic { NSMutableDictionary *serverResponse = [NSMutableDictionary dictionaryWithDictionary: - @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:200 HTTPVersion:nil headerFields:@{}], + @{ @"response" : [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:400 HTTPVersion:nil headerFields:@{}], @"data" : [@"bad_checksum" dataUsingEncoding:NSUTF8StringEncoding] }]; @@ -67,6 +67,7 @@ - (void)testLogEventUploadLogic { // no sent events, event count will be threshold + 1 XCTAssertEqual([self.amplitude queuedEventCount], kAMPEventUploadThreshold + 1); + [serverResponse setValue:[[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"/"] statusCode:500 HTTPVersion:nil headerFields:@{}] forKey:@"response"]; [serverResponse setValue:[@"request_db_write_failed" dataUsingEncoding:NSUTF8StringEncoding] forKey:@"data"]; [self setupAsyncResponse:serverResponse]; for (int i = 0; i < kAMPEventUploadThreshold; i++) {