From 2aedc9a8c8321c0a8e2410c8ec610f07800a6b50 Mon Sep 17 00:00:00 2001 From: Tito Ciuro Date: Fri, 19 May 2023 14:08:17 -0700 Subject: [PATCH 1/3] Fix error generation when status is .internal - when the error received is of type `.internal`, the current code rewrites the message with `descriptionForErrorCode`. This is pretty bad because any contextual information we might have received from Firebase is destroyed. - this fix attempts to extract the message from the response payload, preserving the original value. - if message is not found, the message is set to the value returned by `descriptionForErrorCode`, as was done before. --- FirebaseFunctions/Sources/FunctionsError.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/FirebaseFunctions/Sources/FunctionsError.swift b/FirebaseFunctions/Sources/FunctionsError.swift index 6a2c52e1f74..698f20f5895 100644 --- a/FirebaseFunctions/Sources/FunctionsError.swift +++ b/FirebaseFunctions/Sources/FunctionsError.swift @@ -225,18 +225,18 @@ internal func FunctionsErrorForResponse(status: NSInteger, if let status = errorDetails["status"] as? String { code = FunctionsErrorCode.errorCode(forName: status) + if let message = errorDetails["message"] as? String { + description = message + } else { + description = code.descriptionForErrorCode + } + // If the code in the body is invalid, treat the whole response as malformed. guard code != .internal else { - return code.generatedError(userInfo: nil) + return code.generatedError(userInfo: [NSLocalizedDescriptionKey: description]) } } - - if let message = errorDetails["message"] as? String { - description = message - } else { - description = code.descriptionForErrorCode - } - + details = errorDetails["details"] as AnyObject? if let innerDetails = details { // Just ignore the details if there an error decoding them. From 5fb298ff2172a009dad1e76cbb5377205cdb6b07 Mon Sep 17 00:00:00 2001 From: Tito Ciuro Date: Fri, 19 May 2023 14:55:57 -0700 Subject: [PATCH 2/3] Run the style.sh script --- FirebaseFunctions/Sources/FunctionsError.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FirebaseFunctions/Sources/FunctionsError.swift b/FirebaseFunctions/Sources/FunctionsError.swift index 698f20f5895..cd205351e19 100644 --- a/FirebaseFunctions/Sources/FunctionsError.swift +++ b/FirebaseFunctions/Sources/FunctionsError.swift @@ -230,13 +230,13 @@ internal func FunctionsErrorForResponse(status: NSInteger, } else { description = code.descriptionForErrorCode } - + // If the code in the body is invalid, treat the whole response as malformed. guard code != .internal else { return code.generatedError(userInfo: [NSLocalizedDescriptionKey: description]) } } - + details = errorDetails["details"] as AnyObject? if let innerDetails = details { // Just ignore the details if there an error decoding them. From 89adc2a82388070352219cc7d81e35087998050d Mon Sep 17 00:00:00 2001 From: Tito Ciuro Date: Sun, 21 May 2023 15:23:03 -0700 Subject: [PATCH 3/3] Update unit tests to reflect the new logic proposal --- .../Tests/Integration/IntegrationTests.swift | 8 ++++---- .../Tests/ObjCIntegration/FIRIntegrationTests.m | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/FirebaseFunctions/Tests/Integration/IntegrationTests.swift b/FirebaseFunctions/Tests/Integration/IntegrationTests.swift index 089eafd926d..d2961b7a787 100644 --- a/FirebaseFunctions/Tests/Integration/IntegrationTests.swift +++ b/FirebaseFunctions/Tests/Integration/IntegrationTests.swift @@ -442,7 +442,7 @@ class IntegrationTests: XCTestCase { } catch { let error = error as NSError XCTAssertEqual(FunctionsErrorCode.internal.rawValue, error.code) - XCTAssertEqual("INTERNAL", error.localizedDescription) + XCTAssertNotEqual("INTERNAL", error.localizedDescription) expectation.fulfill() return } @@ -473,7 +473,7 @@ class IntegrationTests: XCTestCase { } catch { let error = error as NSError XCTAssertEqual(FunctionsErrorCode.internal.rawValue, error.code) - XCTAssertEqual("INTERNAL", error.localizedDescription) + XCTAssertNotEqual("INTERNAL", error.localizedDescription) } } } @@ -498,7 +498,7 @@ class IntegrationTests: XCTestCase { } catch { let error = error as NSError XCTAssertEqual(FunctionsErrorCode.internal.rawValue, error.code) - XCTAssertEqual("INTERNAL", error.localizedDescription) + XCTAssertNotEqual("INTERNAL", error.localizedDescription) expectation.fulfill() return } @@ -528,7 +528,7 @@ class IntegrationTests: XCTestCase { } catch { let error = error as NSError XCTAssertEqual(FunctionsErrorCode.internal.rawValue, error.code) - XCTAssertEqual("INTERNAL", error.localizedDescription) + XCTAssertNotEqual("INTERNAL", error.localizedDescription) } } } diff --git a/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m b/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m index 124f2eb9044..320768c79f7 100644 --- a/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m +++ b/FirebaseFunctions/Tests/ObjCIntegration/FIRIntegrationTests.m @@ -226,7 +226,7 @@ - (void)testUnknownError { completion:^(FIRHTTPSCallableResult *_Nullable result, NSError *_Nullable error) { XCTAssertNotNil(error); XCTAssertEqual(FIRFunctionsErrorCodeInternal, error.code); - XCTAssertEqualObjects(@"INTERNAL", error.localizedDescription); + XCTAssertNotEqualObjects(@"INTERNAL", error.localizedDescription); [expectation fulfill]; }]; [self waitForExpectations:@[ expectation ] timeout:10];