From f7056e9c8957f57dd7589c3b318390278b5b7cf7 Mon Sep 17 00:00:00 2001 From: Yakov Manshin Date: Fri, 23 Aug 2024 16:47:38 +0200 Subject: [PATCH 1/2] Updated Functions Errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Moved `FunctionsCodeForHTTPStatus(_:)` and `FunctionsErrorForResponse(status:body:serializer:)` to a `FunctionsErrorCode` extension to reduce the number of global symbols * Updated `FunctionsCodeForHTTPStatus(_:)`’s documentation to reflect the actual behavior (return value for unknown status codes) --- FirebaseFunctions/Sources/Functions.swift | 4 +- .../Sources/FunctionsError.swift | 135 ++++++++---------- 2 files changed, 63 insertions(+), 76 deletions(-) diff --git a/FirebaseFunctions/Sources/Functions.swift b/FirebaseFunctions/Sources/Functions.swift index 19b3da0d675..aa9daaa543b 100644 --- a/FirebaseFunctions/Sources/Functions.swift +++ b/FirebaseFunctions/Sources/Functions.swift @@ -509,7 +509,7 @@ enum FunctionsConstants { if let error = error as NSError? { let localError: (any Error)? if error.domain == kGTMSessionFetcherStatusDomain { - localError = FunctionsErrorForResponse( + localError = FunctionsErrorCode.errorForResponse( status: error.code, body: data, serializer: serializer @@ -529,7 +529,7 @@ enum FunctionsConstants { } // Case 3: `data` is not `nil` but might specify a custom error -> throws conditionally - if let bodyError = FunctionsErrorForResponse( + if let bodyError = FunctionsErrorCode.errorForResponse( status: 200, body: data, serializer: serializer diff --git a/FirebaseFunctions/Sources/FunctionsError.swift b/FirebaseFunctions/Sources/FunctionsError.swift index 08355031fa1..5091189a2b8 100644 --- a/FirebaseFunctions/Sources/FunctionsError.swift +++ b/FirebaseFunctions/Sources/FunctionsError.swift @@ -101,45 +101,32 @@ public let FunctionsErrorDetailsKey: String = "details" case unauthenticated = 16 } -/** - * Takes an HTTP status code and returns the corresponding `FIRFunctionsErrorCode` error code. - * This is the standard HTTP status code -> error mapping defined in: - * https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto - * - Parameter status An HTTP status code. - * - Returns: The corresponding error code, or `FIRFunctionsErrorCodeUnknown` if none. - */ -func FunctionsCodeForHTTPStatus(_ status: NSInteger) -> FunctionsErrorCode { - switch status { - case 200: - return .OK - case 400: - return .invalidArgument - case 401: - return .unauthenticated - case 403: - return .permissionDenied - case 404: - return .notFound - case 409: - return .alreadyExists - case 429: - return .resourceExhausted - case 499: - return .cancelled - case 500: - return .internal - case 501: - return .unimplemented - case 503: - return .unavailable - case 504: - return .deadlineExceeded - default: - return .internal +extension FunctionsErrorCode { + /// Takes an HTTP status code and returns the corresponding `FIRFunctionsErrorCode` error code. + /// + /// + This is the standard HTTP status code -> error mapping defined in: + /// https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto + /// + /// - Parameter status: An HTTP status code. + /// - Returns: A `FunctionsErrorCode`. Falls back to `internal` for unknown status codes. + static func errorCode(forHTTPStatus status: Int) -> Self { + switch status { + case 200: .OK + case 400: .invalidArgument + case 401: .unauthenticated + case 403: .permissionDenied + case 404: .notFound + case 409: .alreadyExists + case 429: .resourceExhausted + case 499: .cancelled + case 500: .internal + case 501: .unimplemented + case 503: .unavailable + case 504: .deadlineExceeded + default: .internal + } } -} -extension FunctionsErrorCode { static func errorCode(forName name: String) -> FunctionsErrorCode { switch name { case "OK": return .OK @@ -207,53 +194,53 @@ extension FunctionsErrorCode { code: rawValue, userInfo: userInfo ?? [NSLocalizedDescriptionKey: descriptionForErrorCode]) } -} -func FunctionsErrorForResponse(status: NSInteger, + static func errorForResponse(status: Int, body: Data?, serializer: FunctionsSerializer) -> NSError? { - // Start with reasonable defaults from the status code. - var code = FunctionsCodeForHTTPStatus(status) - var description = code.descriptionForErrorCode - - var details: AnyObject? + // Start with reasonable defaults from the status code. + var code = FunctionsErrorCode.errorCode(forHTTPStatus: status) + var description = code.descriptionForErrorCode + var details: AnyObject? + + // Then look through the body for explicit details. + if let body, + let json = try? JSONSerialization.jsonObject(with: body) as? NSDictionary, + let errorDetails = json["error"] as? NSDictionary { + if let status = errorDetails["status"] as? String { + code = .errorCode(forName: status) + + // If the code in the body is invalid, treat the whole response as malformed. + guard code != .internal else { + return code.generatedError(userInfo: nil) + } + } - // Then look through the body for explicit details. - if let body, - let json = try? JSONSerialization.jsonObject(with: body) as? NSDictionary, - let errorDetails = json["error"] as? NSDictionary { - 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) + // Update `details` only if the object can be decoded. + if let innerDetails = errorDetails["details"], + let decodedDetails = try? serializer.decode(innerDetails) { + details = decodedDetails } } - if let message = errorDetails["message"] as? String { - description = message - } else { - description = code.descriptionForErrorCode + if code == .OK { + // Technically, there's an edge case where a developer could explicitly return an error code + // of + // OK, and we will treat it as success, but that seems reasonable. + return nil } - details = errorDetails["details"] as AnyObject? - if let innerDetails = details { - // Just ignore the details if there an error decoding them. - details = try? serializer.decode(innerDetails) + var userInfo = [String: Any]() + userInfo[NSLocalizedDescriptionKey] = description + if let details { + userInfo[FunctionsErrorDetailsKey] = details } + return code.generatedError(userInfo: userInfo) } - - if code == .OK { - // Technically, there's an edge case where a developer could explicitly return an error code of - // OK, and we will treat it as success, but that seems reasonable. - return nil - } - - var userInfo = [String: Any]() - userInfo[NSLocalizedDescriptionKey] = description - if let details { - userInfo[FunctionsErrorDetailsKey] = details - } - return code.generatedError(userInfo: userInfo) } From 19ce33f2821ac742ed41e738981e6a8322f8723a Mon Sep 17 00:00:00 2001 From: Yakov Manshin Date: Mon, 26 Aug 2024 22:26:14 +0200 Subject: [PATCH 2/2] Updated Decoding of Error Details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * If they can’t be decoded, the original object is included in the error * This changes the method’s behavior: Currently, undecodable objects are discarded --- FirebaseFunctions/Sources/FunctionsError.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/FirebaseFunctions/Sources/FunctionsError.swift b/FirebaseFunctions/Sources/FunctionsError.swift index 5091189a2b8..8755b362bd1 100644 --- a/FirebaseFunctions/Sources/FunctionsError.swift +++ b/FirebaseFunctions/Sources/FunctionsError.swift @@ -222,8 +222,10 @@ extension FunctionsErrorCode { description = code.descriptionForErrorCode } - // Update `details` only if the object can be decoded. - if let innerDetails = errorDetails["details"], + details = errorDetails["details"] as AnyObject? + // Update `details` only if decoding succeeds; + // otherwise, keep the original object. + if let innerDetails = details, let decodedDetails = try? serializer.decode(innerDetails) { details = decodedDetails }