From ae9b2a936f0497e67a2874de774223e5a89de083 Mon Sep 17 00:00:00 2001
From: Yakov Manshin <git@yakovmanshin.com>
Date: Mon, 11 Nov 2024 23:00:24 +0100
Subject: [PATCH] =?UTF-8?q?Refactored=20`Functions`=E2=80=99s=20Result=20H?=
 =?UTF-8?q?andling?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Replaced the `processedResponseData(from:error:)` method with separate methods for handling data and errors, to better suit the async / await function-calling method (which returns either data or error but not both)
---
 FirebaseFunctions/Sources/Functions.swift | 68 +++++++++++------------
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/FirebaseFunctions/Sources/Functions.swift b/FirebaseFunctions/Sources/Functions.swift
index d4cd4e4f54f..51e405b2f39 100644
--- a/FirebaseFunctions/Sources/Functions.swift
+++ b/FirebaseFunctions/Sources/Functions.swift
@@ -401,11 +401,9 @@ enum FunctionsConstants {
 
     do {
       let rawData = try await fetcher.beginFetch()
-      return try callableResultFromResponse(data: rawData, error: nil)
+      return try callableResult(fromResponseData: rawData)
     } catch {
-      // This method always throws when `error` is not `nil`, but ideally,
-      // it should be refactored so it looks less confusing.
-      return try callableResultFromResponse(data: nil, error: error)
+      throw processedError(fromResponseError: error)
     }
   }
 
@@ -455,10 +453,16 @@ enum FunctionsConstants {
 
     fetcher.beginFetch { [self] data, error in
       let result: Result<HTTPSCallableResult, any Error>
-      do {
-        result = try .success(callableResultFromResponse(data: data, error: error))
-      } catch {
-        result = .failure(error)
+      if let error {
+        result = .failure(processedError(fromResponseError: error))
+      } else if let data {
+        do {
+          result = try .success(callableResult(fromResponseData: data))
+        } catch {
+          result = .failure(error)
+        }
+      } else {
+        result = .failure(FunctionsError(.internal))
       }
 
       DispatchQueue.main.async {
@@ -519,9 +523,23 @@ enum FunctionsConstants {
     return fetcher
   }
 
-  private func callableResultFromResponse(data: Data?,
-                                          error: (any Error)?) throws -> HTTPSCallableResult {
-    let processedData = try processedResponseData(from: data, error: error)
+  private func processedError(fromResponseError error: any Error) -> any Error {
+    let error = error as NSError
+    let localError: (any Error)? = if error.domain == kGTMSessionFetcherStatusDomain {
+      FunctionsError(
+        httpStatusCode: error.code,
+        body: error.userInfo["data"] as? Data,
+        serializer: serializer
+      )
+    } else if error.domain == NSURLErrorDomain, error.code == NSURLErrorTimedOut {
+      FunctionsError(.deadlineExceeded)
+    } else { nil }
+
+    return localError ?? error
+  }
+
+  private func callableResult(fromResponseData data: Data) throws -> HTTPSCallableResult {
+    let processedData = try processedData(fromResponseData: data)
     let json = try responseDataJSON(from: processedData)
     // TODO: Refactor `decode(_:)` so it either returns a non-optional object or throws
     let payload = try serializer.decode(json)
@@ -529,36 +547,12 @@ enum FunctionsConstants {
     return HTTPSCallableResult(data: payload as Any)
   }
 
-  private func processedResponseData(from data: Data?, error: (any Error)?) throws -> Data {
-    // Case 1: `error` is not `nil` -> always throws
-    if let error = error as NSError? {
-      let localError: (any Error)?
-      if error.domain == kGTMSessionFetcherStatusDomain {
-        localError = FunctionsError(
-          httpStatusCode: error.code,
-          body: data ?? error.userInfo["data"] as? Data,
-          serializer: serializer
-        )
-      } else if error.domain == NSURLErrorDomain, error.code == NSURLErrorTimedOut {
-        localError = FunctionsError(.deadlineExceeded)
-      } else {
-        localError = nil
-      }
-
-      throw localError ?? error
-    }
-
-    // Case 2: `data` is `nil` -> always throws
-    guard let data else {
-      throw FunctionsError(.internal)
-    }
-
-    // Case 3: `data` is not `nil` but might specify a custom error -> throws conditionally
+  private func processedData(fromResponseData data: Data) throws -> Data {
+    // `data` might specify a custom error. If so, throw the error.
     if let bodyError = FunctionsError(httpStatusCode: 200, body: data, serializer: serializer) {
       throw bodyError
     }
 
-    // Case 4: `error` is `nil`; `data` is not `nil`; `data` doesn’t specify an error -> OK
     return data
   }