Update the API service to log the returned message from the API (#6458)

* Update the API service to log the raw message from the API

* Update CHANGELOG

* Run style script

* Update log formatting and changelog

Author: schnecle

FirebaseAppDistribution/CHANGELOG.md

@@ -1,3 +1,9 @@
+# v0.9.3
+- [changed] Updated error log for non-200 API Service calls.
+
+# v0.9.2
+- [fixed] Made bug fixes (#6346) available in Zip build and Carthage.
+
 # v0.9.1
 - [changed] Updated header comments (#6321).
 - [fixed] Bug for customers with restricted API keys unable to fetch releases (#6346).

FirebaseAppDistribution/Sources/FIRFADApiService.m

@@ -78,6 +78,27 @@ + (NSMutableURLRequest *)createHTTPRequest:(NSString *)method
   return request;
 }
 
++ (NSString *)tryParseGoogleAPIErrorFromResponse:(NSData *)data {
+  NSError *parseError;
+  NSDictionary *responseDict = [NSJSONSerialization JSONObjectWithData:data
+                                                               options:0
+                                                                 error:&parseError];
+  if (parseError) {
+    return @"Could not parse additional details about this API error.";
+  } else {
+    NSDictionary *errorDict = [responseDict objectForKey:@"error"];
+    if (!errorDict) {
+      return @"Could not parse additional details about this API error.";
+    }
+
+    NSString *message = [errorDict objectForKey:@"message"];
+    if (!message) {
+      return @"Could not parse additional details about this API error.";
+    }
+    return message;
+  }
+}
+
 + (NSArray *)handleReleaseResponse:(NSData *)data
                           response:(NSURLResponse *)response
                              error:(NSError **_Nullable)error {
@@ -86,7 +107,8 @@ + (NSArray *)handleReleaseResponse:(NSData *)data
                 httpResponse);
 
   if ([self handleHttpResponseError:httpResponse error:error]) {
-    FIRFADErrorLog(@"App Tester API service error - %@", [*error localizedDescription]);
+    FIRFADErrorLog(@"App Tester API service error: %@. %@", [*error localizedDescription],
+                   [self tryParseGoogleAPIErrorFromResponse:data]);
     return nil;
   }
 
@@ -142,12 +164,12 @@ + (BOOL)handleHttpResponseError:(NSHTTPURLResponse *)httpResponse error:(NSError
 
 + (NSError *)createErrorFromStatusCode:(NSInteger)statusCode {
   if (statusCode == 401) {
-    return [self createErrorWithDescription:@"Tester not authenticated."
+    return [self createErrorWithDescription:@"Tester not authenticated"
                                        code:FIRFADApiErrorUnauthenticated];
   }
 
   if (statusCode == 403 || statusCode == 400) {
-    return [self createErrorWithDescription:@"Tester not authorized."
+    return [self createErrorWithDescription:@"Tester not authorized"
                                        code:FIRFADApiErrorUnauthorized];
   }
 
@@ -157,7 +179,7 @@ + (NSError *)createErrorFromStatusCode:(NSInteger)statusCode {
   }
 
   if (statusCode == 408 || statusCode == 504) {
-    return [self createErrorWithDescription:@"Request timeout." code:FIRFADApiErrorTimeout];
+    return [self createErrorWithDescription:@"Request timeout" code:FIRFADApiErrorTimeout];
   }
 
   FIRFADErrorLog(@"Encountered unmapped status code: %ld", (long)statusCode);

FirebaseAppDistribution/Tests/Unit/FIRFADApiServiceTests.m

@@ -17,6 +17,7 @@
 #import <XCTest/XCTest.h>
 
 #import "FirebaseAppDistribution/Sources/FIRFADApiService.h"
+#import "FirebaseAppDistribution/Sources/FIRFADLogger.h"
 #import "FirebaseCore/Sources/Private/FirebaseCoreInternal.h"
 #import "FirebaseInstallations/Source/Library/Private/FirebaseInstallationsInternal.h"
 
@@ -25,14 +26,22 @@
 @interface FIRFADApiServiceTests : XCTestCase
 @end
 
+@interface FIRFADApiService (PrivateUnitTesting)
+
++ (NSString *)tryParseGoogleAPIErrorFromResponse:(NSData *)data;
+
+@end
+
 @implementation FIRFADApiServiceTests {
   id _mockFIRAppClass;
   id _mockURLSession;
   id _mockFIRInstallations;
   id _mockInstallationToken;
   NSString *_mockAuthToken;
   NSString *_mockInstallationId;
+  NSString *_mockAPINotEnabledMessage;
   NSDictionary *_mockReleases;
+  NSDictionary *_mockAPINotEnabledResponse;
 }
 
 - (void)setUp {
@@ -65,6 +74,24 @@ - (void)setUp {
       }
     ]
   };
+
+  _mockAPINotEnabledMessage =
+      @"This is a long message about what's happening. This is a fake message from the Firebase "
+      @"App Testers API in project 123456789. This should be logged.";
+  _mockAPINotEnabledResponse = @{
+    @"error" : @{
+      @"code" : @403,
+      @"message" : _mockAPINotEnabledMessage,
+      @"status" : @"PERMISSION_DENIED",
+      @"details" : @[ @{
+        @"type" : @"type.fakeapis.com/appdistro.api.Help",
+        @"links" : @[ @{
+          @"description" : @"this is a short statement about enabling the api",
+          @"url" : @"this should be a link"
+        } ],
+      } ],
+    }
+  };
 }
 
 - (void)tearDown {
@@ -140,6 +167,37 @@ - (void)rejectUrlSessionResponseWithData {
                                completionHandler:[OCMArg isNotNil]]);
 }
 
+- (void)testTryParseGoogleAPIErrorFromResponseSuccess {
+  NSData *data = [NSJSONSerialization dataWithJSONObject:_mockAPINotEnabledResponse
+                                                 options:0
+                                                   error:nil];
+  NSString *message = [FIRFADApiService tryParseGoogleAPIErrorFromResponse:data];
+  XCTAssertTrue([message isEqualToString:_mockAPINotEnabledMessage]);
+}
+
+- (void)testTryParseGoogleAPIErrorFromResponseParseFailure {
+  NSData *data = [@"malformed{json[data" dataUsingEncoding:NSUTF8StringEncoding];
+  NSString *message = [FIRFADApiService tryParseGoogleAPIErrorFromResponse:data];
+  XCTAssertTrue(
+      [message isEqualToString:@"Could not parse additional details about this API error."]);
+}
+
+- (void)testTryParseGoogleAPIErrorFromResponseNoErrorFailure {
+  NSDictionary *errorDictionary = @{@"message" : @"This has no subdict"};
+  NSData *data = [NSJSONSerialization dataWithJSONObject:errorDictionary options:0 error:nil];
+  NSString *message = [FIRFADApiService tryParseGoogleAPIErrorFromResponse:data];
+  XCTAssertTrue(
+      [message isEqualToString:@"Could not parse additional details about this API error."]);
+}
+
+- (void)testTryParseGoogleAPIErrorFromResponseNoMessageFailure {
+  NSDictionary *errorDictionary = @{@"error" : @{@"status" : @"This has no message"}};
+  NSData *data = [NSJSONSerialization dataWithJSONObject:errorDictionary options:0 error:nil];
+  NSString *message = [FIRFADApiService tryParseGoogleAPIErrorFromResponse:data];
+  XCTAssertTrue(
+      [message isEqualToString:@"Could not parse additional details about this API error."]);
+}
+
 - (void)testGenerateAuthTokenWithCompletionSuccess {
   [self mockInstallationAuthCompletion:_mockInstallationToken error:nil];
   [self mockInstallationIdCompletion:_mockInstallationId error:nil];
@@ -153,8 +211,8 @@ - (void)testGenerateAuthTokenWithCompletionSuccess {
         XCTAssertNotNil(authTokenResult);
         XCTAssertNotNil(identifier);
         XCTAssertNil(error);
-        XCTAssertEqual(identifier, self->_mockInstallationId);
-        XCTAssertEqual([authTokenResult authToken], self -> _mockAuthToken);
+        XCTAssertTrue([identifier isEqualToString:self->_mockInstallationId]);
+        XCTAssertTrue([[authTokenResult authToken] isEqualToString:self->_mockAuthToken]);
         [expectation fulfill];
       }];
 
@@ -318,7 +376,7 @@ - (void)testFetchReleasesWithCompletionUnauthorized403Failure {
   OCMStub([fakeResponse statusCode]).andReturn(403);
   [self mockInstallationAuthCompletion:_mockInstallationToken error:nil];
   [self mockInstallationIdCompletion:_mockInstallationId error:nil];
-  [self mockUrlSessionResponse:_mockReleases response:fakeResponse error:nil];
+  [self mockUrlSessionResponse:_mockAPINotEnabledResponse response:fakeResponse error:nil];
   XCTestExpectation *expectation =
       [self expectationWithDescription:@"Fetch releases rejects with a 403."];