Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloud blocking functions errors are not handled properly #14052

Closed
amirandalibi opened this issue Nov 8, 2024 · 9 comments · Fixed by #14280
Closed

Cloud blocking functions errors are not handled properly #14052

amirandalibi opened this issue Nov 8, 2024 · 9 comments · Fixed by #14280
Assignees
Milestone

Comments

@amirandalibi
Copy link

amirandalibi commented Nov 8, 2024

Description

In my Swift app, errors returned from Firebase Blocking Functions are not properly handled, resulting in the following generic error message:

Error Domain=FIRAuthErrorDomain Code=17999 "An internal error has
occurred, print and inspect the error details for more information."
UserInfo={NSUnderlyingError=0x600000e8d0b0 {Error Domain=FIRAuthInternalErrorDomain Code=2 "(null)" UserInfo={NSUnderlyingError=0x600000cc2700 {Error Domain=NSCocoaErrorDomain Code=3840 "JSON text did 
not start with array or object and option to allow fragments not set. around line 1, column 0." 
UserInfo={NSDebugDescription=JSON text did not start with array or object and option to allow fragments not set.
around  line 1, column 0., NSJSONSerializationErrorIndex=0}}}}, NSLocalizedDescription=An internal error has occurred, print and inspect the error details for more information., FIRAuthErrorUserInfoNameKey=ERROR_INTERNAL_ERROR}

Blocking function

The blocking function is running on Node.js 20 using beforeUserCreated and throws an error in the following format:

throw new HttpsError("invalid-argument", "invalid email", {
  code: "invalid-email",
  message: "Please enter a valid email address to create your account.",
});

The expected raw error response from this blocking function should resemble:

BLOCKING_FUNCTION_ERROR_RESPONSE : ((HTTP request to http://127.0.0.1:9999/project-id/us-central1/beforeUserCreated returned HTTP error 400: {"error":{"details":{"code":"invalid-email"},"message":"invalid email","status":"INVALID_ARGUMENT"}})) 

The root cause stems from two issues in the Firebase iOS SDK that lead to an improperly formatted error.

  1. Parsing Issue in AuthBackend.swift
    In AuthBackend.swift, the SDK attempts to split the error message string by colons. The intent was to separate the initial part (shortErrorMessage) from the remaining part (serverDetailErrorMessage). However, splitting by : inadvertently captures portions of the URL, resulting in a truncated message ((HTTP request to http, which breaks the error handling.

Current code

let split = serverErrorMessage.split(separator: ":")
let shortErrorMessage = split.first?.trimmingCharacters(in: .whitespacesAndNewlines)
let serverDetailErrorMessage = String(split.count > 1 ? split[1] : "")
.trimmingCharacters(in: .whitespacesAndNewlines)

Suggested Fix

To address this, I propose updating the parsing logic to identify the first colon’s index and split the string at that point, preserving the intended structure:

let splitIndex = serverErrorMessage.firstIndex(of: ":")!
let shortErrorMessage = String(serverErrorMessage[..<splitIndex])
  .trimmingCharacters(in: .whitespacesAndNewlines)
let serverDetailErrorMessage = String(serverErrorMessage[serverErrorMessage.index(after: splitIndex)...] ?? "")
  .trimmingCharacters(in: .whitespacesAndNewlines)
  1. Hardcoded String Dependency in AuthErrorUtils.swift
    In AuthErrorUtils.swift, the blockingCloudFunctionServerResponse function relies on replacing a hardcoded string to extract JSON data from the error. This dependency fails because the string "HTTP Cloud Function returned an error:" is absent, leading to a JSON serialization error.

Current code

static func blockingCloudFunctionServerResponse(message: String?) -> Error {
guard let message else {
return error(code: .blockingCloudFunctionError, message: message)
}
var jsonString = message.replacingOccurrences(
of: "HTTP Cloud Function returned an error:",
with: ""
)
jsonString = jsonString.trimmingCharacters(in: .whitespaces)
let jsonData = jsonString.data(using: .utf8) ?? Data()
do {
let jsonDict = try JSONSerialization
.jsonObject(with: jsonData, options: []) as? [String: Any] ?? [:]
let errorDict = jsonDict["error"] as? [String: Any] ?? [:]
let errorMessage = errorDict["message"] as? String
return error(code: .blockingCloudFunctionError, message: errorMessage)
} catch {
return JSONSerializationError(underlyingError: error)
}
}

Suggested Fix

To avoid hardcoded string dependencies, I suggest using regex to directly capture the JSON object within the error string, making the SDK more resilient to changes. Here is the updated function:

static func blockingCloudFunctionServerResponse(message: String?) -> Error {
  do {
    guard let message = message,
      let match = try NSRegularExpression(pattern: "\\{.*\\}", options: .dotMatchesLineSeparators)
        .firstMatch(
          in: message,
          range: NSRange(message.startIndex..., in: message)
        ),
      let range = Range(match.range, in: message)
    else {
      return error(code: .blockingCloudFunctionError, message: nil)
    }

    let jsonData = String(message[range]).data(using: .utf8) ?? Data()
    let jsonDict: [String: Any]? = try JSONSerialization
      .jsonObject(with: jsonData) as? [String: Any]
    let errorMessage = (jsonDict?["error"] as? [String: Any])?["message"] as? String
    return error(code: .blockingCloudFunctionError, message: errorMessage)
  } catch {
    return JSONSerializationError(underlyingError: error)
  }
}

With this fix, the Blocking Function error should be correctly handled, producing a clear and actionable message:

Error Domain=FIRAuthErrorDomain Code=17105 "invalid email" UserInfo={FIRAuthErrorUserInfoNameKey=ERROR_BLOCKING_CLOUD_FUNCTION_RETURNED_ERROR, NSLocalizedDescription=invalid email}

As recommended in the contribution guide, I wanted to share this approach before opening a pull request to gather any insights or feedback that could help further refine these fixes. Thank you for your time and consideration!

Reproducing the issue

step 1: throw a httpsError in your beforeUserCreated Blocking Cloud Function (can be any blocking function)
step 2: create a user in the swift app and catch the error, here's an example code

do {
  try await Auth.auth().createUser(withEmail: email, password: password)
} catch {
  print("\(error)")
}

Firebase SDK Version

11.4.0

Xcode Version

Version 15.4 (15F31d)

Installation Method

Swift Package Manager

Firebase Product(s)

Authentication, Functions

Targeted Platforms

iOS

Relevant Log Output

No response

If using Swift Package Manager, the project's Package.resolved

No response

If using CocoaPods, the project's Podfile.lock

No response

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@paulb777
Copy link
Member

paulb777 commented Nov 8, 2024

@amirandalibi Thanks for sharing the issue and analysis. Your analysis sounds good to me and we would appreciate a pull request including a unit test that demonstrates the issue and fix. Also, my preference would be to use Swift Regular Expression syntax instead of NSRegularExpression.

@0xPr0xy
Copy link

0xPr0xy commented Dec 6, 2024

Is there any update on this? It's pretty crucial functionality

@paulb777
Copy link
Member

We haven't yet been able to prioritize in on the core team. In the meantime, we're open to a PR. See the discussion above.

@john-hutchinson
Copy link

Hi @paulb777 is this going to be worked on at all? We updated our project and found the problem so have reverted dependencies for now but it seems to be a very temporary fix.

@paulb777 paulb777 added this to the 11.7.0 - M158 milestone Dec 13, 2024
@paulb777
Copy link
Member

I'll put this on the list for our next release and try to get it in.

@AronTMC
Copy link

AronTMC commented Dec 13, 2024

Do you have a rough ETA for the next release? Months aways or weeks?

@paulb777 paulb777 self-assigned this Dec 20, 2024
@paulb777
Copy link
Member

@AronTMC Planning the week of January 14th

@paulb777
Copy link
Member

There is now a fix available in #14280. I'd appreciate if anyone seeing the issue could confirm the fix it releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants