Improve error logging for FIRAuthInternalErrorCodeUnexpectedErrorResponse errors (#8704)

rosalyntan · web-flow · commit ab74442509cc · 2021-09-28T16:08:55.000-04:00
* Add underlyingError to FIRAuthInternalErrorCodeUnexpectedErrorResponse errors.

* Revert some formatting changes that make it hard to see the actual diffs.

* Fix unit tests.

* Apparently the formatting is required.

* Update changelog.

* Re-ran the formatting script after updating clang-format.
diff --git a/FirebaseAuth/CHANGELOG.md b/FirebaseAuth/CHANGELOG.md
@@ -1,8 +1,11 @@
+# Unreleased
+- [changed] Improved error logging. (#8704)
+
 # 8.8.0
 - [fixed] Fall back to reCAPTCHA for phone auth app verification if the push notification is not received before the timeout. (#8653)
 
 # 8.6.0
-- [fixed] Annotated platform-level availability using `API_UNAVAILABLE` instead of conditionally compiling certain methods with `#if` directives (#8451).
+- [fixed] Annotated platform-level availability using `API_UNAVAILABLE` instead of conditionally compiling certain methods with `#if` directives. (#8451)
 
 # 8.5.0
 - [fixed] Fixed an analyze issue introduced in Xcode 12.5. (#8411)
diff --git a/FirebaseAuth/Sources/Backend/FIRAuthBackend.m b/FirebaseAuth/Sources/Backend/FIRAuthBackend.m
@@ -1041,7 +1041,8 @@ - (void)postWithRequest:(id<FIRAuthRPCRequest>)request
                            if (![dictionary isKindOfClass:[NSDictionary class]]) {
                              if (error) {
                                callback([FIRAuthErrorUtils
-                                   unexpectedErrorResponseWithDeserializedResponse:dictionary]);
+                                   unexpectedErrorResponseWithDeserializedResponse:dictionary
+                                                                   underlyingError:error]);
                              } else {
                                callback([FIRAuthErrorUtils
                                    unexpectedResponseWithDeserializedResponse:dictionary]);
@@ -1074,15 +1075,16 @@ - (void)postWithRequest:(id<FIRAuthRPCRequest>)request
                                // Not a message we know, return the message directly.
                                if (errorMessage) {
                                  NSError *unexpecterErrorResponse = [FIRAuthErrorUtils
-                                     unexpectedErrorResponseWithDeserializedResponse:
-                                         errorDictionary];
+                                     unexpectedErrorResponseWithDeserializedResponse:errorDictionary
+                                                                     underlyingError:error];
                                  callback(unexpecterErrorResponse);
                                  return;
                                }
                              }
                              // No error message at all, return the decoded response.
                              callback([FIRAuthErrorUtils
-                                 unexpectedErrorResponseWithDeserializedResponse:dictionary]);
+                                 unexpectedErrorResponseWithDeserializedResponse:dictionary
+                                                                 underlyingError:error]);
                              return;
                            }
 
diff --git a/FirebaseAuth/Sources/Utilities/FIRAuthErrorUtils.h b/FirebaseAuth/Sources/Utilities/FIRAuthErrorUtils.h
@@ -90,6 +90,18 @@ NS_ASSUME_NONNULL_BEGIN
  */
 + (NSError *)unexpectedErrorResponseWithDeserializedResponse:(id)deserializedResponse;
 
+/** @fn unexpectedErrorResponseWithDeserializedResponse:underlyingError:
+    @brief Constructs an @c NSError with the @c FIRAuthInternalErrorCodeUnexpectedErrorResponse
+        code, and populated @c FIRAuthErrorUserInfoDeserializedResponseKey and
+        @c NSUnderlyingErrorKey keys in the @c NSError.userInfo dictionary.
+    @param deserializedResponse The value of the @c FIRAuthErrorUserInfoDeserializedResponseKey key.
+    @param underlyingError The value of the @c NSUnderlyingErrorKey key.
+    @remarks This error is used when a network request results in an error, and the body data was
+        deserializable as JSON, but couldn't be decoded as an error.
+ */
++ (NSError *)unexpectedErrorResponseWithDeserializedResponse:(id)deserializedResponse
+                                             underlyingError:(NSError *)underlyingError;
+
 /** @fn malformedJWTErrorWithToken:underlyingError:
     @brief Constructs an @c NSError with the code set to @c FIRAuthErrorCodeMalformedJWT and
         populates the userInfo dictionary with an error message, the bad token, and an underlying
diff --git a/FirebaseAuth/Sources/Utilities/FIRAuthErrorUtils.m b/FirebaseAuth/Sources/Utilities/FIRAuthErrorUtils.m
@@ -1013,6 +1013,19 @@ + (NSError *)unexpectedErrorResponseWithDeserializedResponse:(id)deserializedRes
   return [self errorWithCode:FIRAuthInternalErrorCodeUnexpectedErrorResponse userInfo:userInfo];
 }
 
++ (NSError *)unexpectedErrorResponseWithDeserializedResponse:(id)deserializedResponse
+                                             underlyingError:(NSError *)underlyingError {
+  NSMutableDictionary *userInfo = [NSMutableDictionary dictionary];
+  if (deserializedResponse) {
+    userInfo[FIRAuthErrorUserInfoDeserializedResponseKey] = deserializedResponse;
+  }
+  if (underlyingError) {
+    userInfo[NSUnderlyingErrorKey] = underlyingError;
+  }
+  return [self errorWithCode:FIRAuthInternalErrorCodeUnexpectedErrorResponse
+                    userInfo:[userInfo copy]];
+}
+
 + (NSError *)malformedJWTErrorWithToken:(NSString *)token
                         underlyingError:(NSError *_Nullable)underlyingError {
   NSMutableDictionary *userInfo =
diff --git a/FirebaseAuth/Tests/Unit/FIRAuthBackendRPCImplementationTests.m b/FirebaseAuth/Tests/Unit/FIRAuthBackendRPCImplementationTests.m
@@ -575,10 +575,10 @@ - (void)testUnparsableSuccessResponse {
 /** @fn testNonDictionaryErrorResponse
     @brief This test checks the behaviour of @c postWithRequest:response:callback: when the
         response deserialized by @c NSJSONSerialization is not a dictionary, and an error was
-        expected. We are expecting to receive an @c NSError with the code
-        @c FIRAuthErrorCodeUnexpectedErrorServerResponse with the decoded response in the
-        @c NSError.userInfo dictionary associated with the key
-        @c FIRAuthErrorUserInfoDecodedResponseKey.
+        expected. We are expecting to receive the original network error wrapped in an @c NSError
+        with the code @c FIRAuthInternalErrorCodeUnexpectedErrorResponse with the decoded response
+        in the @c NSError.userInfo dictionary associated with the key
+        @c FIRAuthErrorUserInfoDeserializedResponseKey.
  */
 - (void)testNonDictionaryErrorResponse {
   FIRFakeRequest *request = [FIRFakeRequest fakeRequest];
@@ -613,7 +613,9 @@ - (void)testNonDictionaryErrorResponse {
   XCTAssertEqual(underlyingError.code, FIRAuthInternalErrorCodeUnexpectedErrorResponse);
 
   NSError *underlyingUnderlyingError = underlyingError.userInfo[NSUnderlyingErrorKey];
-  XCTAssertNil(underlyingUnderlyingError);
+  XCTAssertNotNil(underlyingUnderlyingError);
+  XCTAssertEqualObjects(underlyingUnderlyingError.domain, kFakeErrorDomain);
+  XCTAssertEqual(underlyingUnderlyingError.code, kFakeErrorCode);
 
   id deserializedResponse = underlyingError.userInfo[FIRAuthErrorUserInfoDeserializedResponseKey];
   XCTAssertNotNil(deserializedResponse);
@@ -677,9 +679,9 @@ - (void)testNonDictionarySuccessResponse {
     @brief This test checks the behaviour of @c postWithRequest:response:callback: when the
         we get an error message indicating captcha is required. The backend should not be returning
         this error to mobile clients. If it does, we should wrap it in an @c NSError with the code
-        @c FIRAuthErrorCodeUnexpectedServerResponse with the decoded error message in the
+        @c FIRAuthInternalErrorCodeUnexpectedErrorResponse with the decoded error message in the
         @c NSError.userInfo dictionary associated with the key
-        @c FIRAuthErrorUserInfoDecodedErrorResponseKey.
+        @c FIRAuthErrorUserInfoDeserializedResponseKey.
  */
 - (void)testCaptchaRequiredResponse {
   FIRFakeRequest *request = [FIRFakeRequest fakeRequest];
@@ -709,7 +711,9 @@ - (void)testCaptchaRequiredResponse {
   XCTAssertEqual(underlyingError.code, FIRAuthInternalErrorCodeUnexpectedErrorResponse);
 
   NSError *underlyingUnderlyingError = underlyingError.userInfo[NSUnderlyingErrorKey];
-  XCTAssertNil(underlyingUnderlyingError);
+  XCTAssertNotNil(underlyingUnderlyingError);
+  XCTAssertEqualObjects(underlyingUnderlyingError.domain, kFakeErrorDomain);
+  XCTAssertEqual(underlyingUnderlyingError.code, kFakeErrorCode);
 
   id deserializedResponse = underlyingError.userInfo[FIRAuthErrorUserInfoDeserializedResponseKey];
   XCTAssertNotNil(deserializedResponse);
@@ -756,9 +760,9 @@ - (void)testCaptchaCheckFailedResponse {
         we get an error message indicating captcha is required and an invalid password was entered.
         The backend should not be returning this error to mobile clients. If it does, we should wrap
         it in an @c NSError with the code
-        @c FIRAuthErrorCodeUnexpectedServerResponse with the decoded error message in the
+        @c FIRAuthInternalErrorCodeUnexpectedErrorResponse with the decoded error message in the
         @c NSError.userInfo dictionary associated with the key
-        @c FIRAuthErrorUserInfoDecodedErrorResponseKey.
+        @c FIRAuthErrorUserInfoDeserializedResponseKey.
  */
 - (void)testCaptchaRequiredInvalidPasswordResponse {
   FIRFakeRequest *request = [FIRFakeRequest fakeRequest];
@@ -789,7 +793,9 @@ - (void)testCaptchaRequiredInvalidPasswordResponse {
   XCTAssertEqual(underlyingError.code, FIRAuthInternalErrorCodeUnexpectedErrorResponse);
 
   NSError *underlyingUnderlyingError = underlyingError.userInfo[NSUnderlyingErrorKey];
-  XCTAssertNil(underlyingUnderlyingError);
+  XCTAssertNotNil(underlyingUnderlyingError);
+  XCTAssertEqualObjects(underlyingUnderlyingError.domain, kFakeErrorDomain);
+  XCTAssertEqual(underlyingUnderlyingError.code, kFakeErrorCode);
 
   id deserializedResponse = underlyingError.userInfo[FIRAuthErrorUserInfoDeserializedResponseKey];
   XCTAssertNotNil(deserializedResponse);
@@ -804,9 +810,10 @@ - (void)testCaptchaRequiredInvalidPasswordResponse {
     @brief This test checks the behaviour of @c postWithRequest:response:callback: when the
         response deserialized by @c NSJSONSerialization represents a valid error response (and an
         error was indicated) but we didn't receive an error message we know about. We are expecting
-        an @c NSError with the code @c FIRAuthErrorCodeUnexpectedServerResponse with the decoded
+        to receive the original network error wrapped in an @c NSError with the code
+        @c FIRAuthInternalErrorCodeUnexpectedErrorResponse with the decoded
         error message in the @c NSError.userInfo dictionary associated with the key
-        @c FIRAuthErrorUserInfoDecodedErrorResponseKey.
+        @c FIRAuthErrorUserInfoDeserializedResponseKey.
  */
 - (void)testDecodableErrorResponseWithUnknownMessage {
   FIRFakeRequest *request = [FIRFakeRequest fakeRequest];
@@ -838,7 +845,9 @@ - (void)testDecodableErrorResponseWithUnknownMessage {
   XCTAssertEqual(underlyingError.code, FIRAuthInternalErrorCodeUnexpectedErrorResponse);
 
   NSError *underlyingUnderlyingError = underlyingError.userInfo[NSUnderlyingErrorKey];
-  XCTAssertNil(underlyingUnderlyingError);
+  XCTAssertNotNil(underlyingUnderlyingError);
+  XCTAssertEqualObjects(underlyingUnderlyingError.domain, kFakeErrorDomain);
+  XCTAssertEqual(underlyingUnderlyingError.code, kFakeErrorCode);
 
   id deserializedResponse = underlyingError.userInfo[FIRAuthErrorUserInfoDeserializedResponseKey];
   XCTAssertNotNil(deserializedResponse);
@@ -852,10 +861,11 @@ - (void)testDecodableErrorResponseWithUnknownMessage {
 /** @fn testErrorResponseWithNoErrorMessage
     @brief This test checks the behaviour of @c postWithRequest:response:callback: when the
         response deserialized by @c NSJSONSerialization is a dictionary, and an error was indicated,
-        but no error information was present in the decoded response. We are expecting an @c NSError
-        with the code @c FIRAuthErrorCodeUnexpectedServerResponse with the decoded
+        but no error information was present in the decoded response. We are expecting to receive
+        the original network error wrapped in an @c NSError with the code
+        @c FIRAuthErrorCodeUnexpectedServerResponse with the decoded
         response message in the @c NSError.userInfo dictionary associated with the key
-        @c FIRAuthErrorUserInfoDecodedResponseKey.
+        @c FIRAuthErrorUserInfoDeserializedResponseKey.
  */
 - (void)testErrorResponseWithNoErrorMessage {
   FIRFakeRequest *request = [FIRFakeRequest fakeRequest];
@@ -885,7 +895,9 @@ - (void)testErrorResponseWithNoErrorMessage {
   XCTAssertEqual(underlyingError.code, FIRAuthInternalErrorCodeUnexpectedErrorResponse);
 
   NSError *underlyingUnderlyingError = underlyingError.userInfo[NSUnderlyingErrorKey];
-  XCTAssertNil(underlyingUnderlyingError);
+  XCTAssertNotNil(underlyingUnderlyingError);
+  XCTAssertEqualObjects(underlyingUnderlyingError.domain, kFakeErrorDomain);
+  XCTAssertEqual(underlyingUnderlyingError.code, kFakeErrorCode);
 
   id deserializedResponse = underlyingError.userInfo[FIRAuthErrorUserInfoDeserializedResponseKey];
   XCTAssertNotNil(deserializedResponse);
