From 62ff252ddbc61a4dd0ce1b1248f91b856f25970f Mon Sep 17 00:00:00 2001 From: Jing Liu Date: Fri, 1 Nov 2024 19:29:30 -0700 Subject: [PATCH 1/3] When token fetch fails, return the failure reason to the client. Currently, when token fetch fails, a generic and misleading message like this is shown: > Invalid fetch response, expected 'token' or 'Error' key After this PR, the actual failure reason returned from the server will be logged and returned to the client. --- .../Token/FIRMessagingTokenFetchOperation.m | 5 ++ .../FIRMessagingTokenOperationsTest.m | 63 ++++++++++++++++--- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/FirebaseMessaging/Sources/Token/FIRMessagingTokenFetchOperation.m b/FirebaseMessaging/Sources/Token/FIRMessagingTokenFetchOperation.m index ffb3fdb736c..0bc1e79c3b0 100644 --- a/FirebaseMessaging/Sources/Token/FIRMessagingTokenFetchOperation.m +++ b/FirebaseMessaging/Sources/Token/FIRMessagingTokenFetchOperation.m @@ -172,6 +172,11 @@ - (void)handleResponseWithData:(NSData *)data FIRMessagingLoggerDebug(kFIRMessagingMessageCodeInternal001, @"%@", failureReason); responseError = [NSError messagingErrorWithCode:kFIRMessagingErrorCodeInvalidIdentity failureReason:failureReason]; + } else { + FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTokenFetchOperationRequestError, + @"Token fetch got an error from server: %@", errorValue); + responseError = [NSError messagingErrorWithCode:kFIRMessagingErrorCodeUnknown + failureReason:errorValue]; } } if (!responseError) { diff --git a/FirebaseMessaging/Tests/UnitTests/FIRMessagingTokenOperationsTest.m b/FirebaseMessaging/Tests/UnitTests/FIRMessagingTokenOperationsTest.m index 0459a8d3e38..a785b7f5b98 100644 --- a/FirebaseMessaging/Tests/UnitTests/FIRMessagingTokenOperationsTest.m +++ b/FirebaseMessaging/Tests/UnitTests/FIRMessagingTokenOperationsTest.m @@ -185,7 +185,7 @@ - (void)testThatTokenOperationsAuthHeaderStringMatchesCheckin { [FIRURLSessionOCMockStub stubURLSessionDataTaskWithResponse:expectedResponse - body:[self dataForResponseWithValidToken:YES] + body:[self dataForResponseWithValidToken:@""] error:nil URLSessionMock:self.URLSessionMock requestValidationBlock:^BOOL(NSURLRequest *_Nonnull sentRequest) { @@ -315,7 +315,7 @@ - (void)testThatOptionsDictionaryIsIncludedWithFetchRequest { [FIRURLSessionOCMockStub stubURLSessionDataTaskWithResponse:expectedResponse - body:[self dataForResponseWithValidToken:YES] + body:[self dataForResponseWithValidToken:@""] error:nil URLSessionMock:self.URLSessionMock requestValidationBlock:^BOOL(NSURLRequest *_Nonnull sentRequest) { @@ -368,7 +368,7 @@ - (void)testServerResetCommand { [FIRURLSessionOCMockStub stubURLSessionDataTaskWithResponse:expectedResponse - body:[self dataForResponseWithValidToken:NO] + body:[self dataForResponseWithValidToken:@"RST"] error:nil URLSessionMock:self.URLSessionMock requestValidationBlock:^BOOL(NSURLRequest *_Nonnull sentRequest) { @@ -392,6 +392,55 @@ - (void)testServerResetCommand { }]; } +- (void)testTooManyRegistrationsError { + XCTestExpectation *shouldResetIdentityExpectation = + [self expectationWithDescription:@"When server returns TOO_MANY_REGISTRATIONS, the client " + @"should report the failure reason."]; + int64_t tenHoursAgo = FIRMessagingCurrentTimestampInMilliseconds() - 10 * 60 * 60 * 1000; + FIRMessagingCheckinPreferences *checkinPreferences = + [self setCheckinPreferencesWithLastCheckinTime:tenHoursAgo]; + + FIRMessagingTokenFetchOperation *operation = [[FIRMessagingTokenFetchOperation alloc] + initWithAuthorizedEntity:kAuthorizedEntity + scope:kScope + options:nil + checkinPreferences:checkinPreferences + instanceID:self.instanceID + heartbeatLogger:[[FIRHeartbeatLoggerFake alloc] init]]; + NSURL *expectedRequestURL = [NSURL URLWithString:FIRMessagingTokenRegisterServer()]; + NSHTTPURLResponse *expectedResponse = [[NSHTTPURLResponse alloc] initWithURL:expectedRequestURL + statusCode:200 + HTTPVersion:@"HTTP/1.1" + headerFields:nil]; + + [FIRURLSessionOCMockStub + stubURLSessionDataTaskWithResponse:expectedResponse + body:[self dataForResponseWithValidToken: + @"TOO_MANY_REGISTRATIONS"] + error:nil + URLSessionMock:self.URLSessionMock + requestValidationBlock:^BOOL(NSURLRequest *_Nonnull sentRequest) { + return YES; + }]; + + [operation addCompletionHandler:^(FIRMessagingTokenOperationResult result, + NSString *_Nullable token, NSError *_Nullable error) { + XCTAssertEqual(result, FIRMessagingTokenOperationError); + XCTAssertNotNil(error); + XCTAssertEqual(error.code, kFIRMessagingErrorCodeUnknown); + XCTAssertEqualObjects(error.localizedFailureReason, @"TOO_MANY_REGISTRATIONS"); + + [shouldResetIdentityExpectation fulfill]; + }]; + + [operation start]; + + [self waitForExpectationsWithTimeout:0.25 + handler:^(NSError *_Nullable error) { + XCTAssertNil(error.localizedDescription); + }]; +} + - (void)testHTTPAuthHeaderGenerationFromCheckin { FIRMessagingCheckinPreferences *checkinPreferences = [[FIRMessagingCheckinPreferences alloc] initWithDeviceID:kDeviceID secretToken:kSecretToken]; @@ -447,7 +496,7 @@ - (void)assertTokenFetchOperationRequestContainsFirebaseUserAgentAndHeartbeatInf [FIRURLSessionOCMockStub stubURLSessionDataTaskWithResponse:expectedResponse - body:[self dataForResponseWithValidToken:NO] + body:[self dataForResponseWithValidToken:@"RST"] error:nil URLSessionMock:self.URLSessionMock requestValidationBlock:^BOOL(NSURLRequest *_Nonnull sentRequest) { @@ -471,12 +520,12 @@ - (void)assertTokenFetchOperationRequestContainsFirebaseUserAgentAndHeartbeatInf }]; } -- (NSData *)dataForResponseWithValidToken:(BOOL)validToken { +- (NSData *)dataForResponseWithValidToken:(NSString *)errorCode { NSString *response; - if (validToken) { + if (errorCode.length == 0) { response = [NSString stringWithFormat:@"token=%@", kRegistrationToken]; } else { - response = @"Error=RST"; + response = [NSString stringWithFormat:@"Error=%@", errorCode]; } return [response dataUsingEncoding:NSUTF8StringEncoding]; } From da7e0e53a2205cd322498aeaf8c13f586babacc8 Mon Sep 17 00:00:00 2001 From: Jing Liu Date: Tue, 5 Nov 2024 00:07:41 -0800 Subject: [PATCH 2/3] Add changelog. --- FirebaseMessaging/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/FirebaseMessaging/CHANGELOG.md b/FirebaseMessaging/CHANGELOG.md index cf83a783db6..8a7d4251d6f 100644 --- a/FirebaseMessaging/CHANGELOG.md +++ b/FirebaseMessaging/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Improve token-fetch failure logging with detailed error info. (#13997). + # 11.0.0 - [fixed] Completed Messaging's transition to NSSecureCoding (#12343). From 61fd1b92f925bc2a882e698a35322e62e92a117c Mon Sep 17 00:00:00 2001 From: Jing Liu Date: Tue, 5 Nov 2024 00:09:40 -0800 Subject: [PATCH 3/3] Delete a trailing whitespace. --- FirebaseMessaging/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FirebaseMessaging/CHANGELOG.md b/FirebaseMessaging/CHANGELOG.md index 8a7d4251d6f..795539da1dd 100644 --- a/FirebaseMessaging/CHANGELOG.md +++ b/FirebaseMessaging/CHANGELOG.md @@ -1,4 +1,4 @@ -# Unreleased +# Unreleased - [fixed] Improve token-fetch failure logging with detailed error info. (#13997). # 11.0.0