GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
1.16k stars 156 forks source link

FirebaseException should expose code as a property #298

Open rocketraman opened 2 years ago

rocketraman commented 2 years ago

Currently FirebaseException can be initialized with a code value, however the code is only used to build the default Exception message.

There are many cases in which the code is necessary to take the appropriate action -- to take one simple example: https://firebase.google.com/docs/reference/android/com/google/firebase/auth/FirebaseAuthInvalidUserException.

However, code is not a property.

Please change

FirebaseException(code: String?, cause: Throwable) : Exception("$code: ${cause.message}", cause)

to

FirebaseException(val code: String?, cause: Throwable) : Exception("$code: ${cause.message}", cause)
anqus commented 1 year ago

Bump. The only way I've found to handle different errors of the same type is to check what the message contains, as seen below. This is because the messages are formatted differently on iOS and Android, as mentioned in #317:

private fun mapFirebaseAuthErrors(e: FirebaseAuthException): AuthenticationError {
  println("Auth Error: ${e.message}")
  return with(e.message) {
    when {
      this?.contains("There is no user record corresponding to this identifier. The user may have been deleted.") == true -> EmailDoesNotExistError()
      this?.contains("The email address is badly formatted.") == true -> EmailInvalidError()
      this?.contains("The password is invalid or the user does not have a password.") == true -> PasswordWrongError()
      this?.contains("The user account has been disabled by an administrator.") == true -> UserDisabledError()
      this?.contains("The email address is already in use by another account.") == true -> EmailAlreadyExistsError()
      this?.contains("The password must be 6 characters long or more.") == true -> PasswordWeakError()
      else -> AuthenticationError(e.message)
    }
  }
}

It would be much cleaner if we could use the error codes across all platforms that are specified here:

https://github.com/firebase/firebase-ios-sdk/blob/0b3bf6124f6f9643b783c9e74aa9d38d7588b1ff/FirebaseAuth/Sources/Public/FirebaseAuth/FIRAuthErrors.h

pmodernme commented 1 year ago

Exposing codes would make our code more robust. Matching message strings is a really flimsy way to handle code, especially when exception handling needs to do more than just present an error to the user.

ubuntudroid commented 1 year ago

We don't just need the error code, but also the details property that one can pass from Firebase functions and which should end up in FirebaseFunctionsException as it does in the official SDK.

nbransby commented 1 year ago

Currently FirebaseException can be initialized with a code value, however the code is only used to build the default Exception message.

FirebaseException doesn't have any properties on Android: https://developers.google.com/android/reference/com/google/firebase/FirebaseException

Did you mean FirebaseAuthException?

nbransby commented 1 year ago

Did you mean FirebaseAuthException?

Actually I suspect you were referring to the FirebaseException in the JS target as that does have a code that's not exposed to match https://firebase.google.com/docs/reference/node/firebase.FirebaseError

Exposing it won't help much though as it will only be available to JS sourcesets - I'll see if we can expose the codes the same way the android sdk does and then attempt to map the js and ios code to the android ones

Note the pattern in in Firebas Auth is to use different FirebaseAuthException subtypes for each error type so you can switch on the type instead of code for auth and this is already implemented in the SDK

ubuntudroid commented 1 year ago

@nbransby I was referring to the Firebase iOS SDK'sFirebase(Functions)Exception.

rocketraman commented 1 year ago

Actually I suspect you were referring to the FirebaseException in the JS target as that does have a code that's not exposed to match https://firebase.google.com/docs/reference/node/firebase.FirebaseError

Yes, sorry I should have specified. I was indeed referring to FirebaseException in the JS target:

https://github.com/GitLiveApp/firebase-kotlin-sdk/blob/a28c34ec866a034b6d7bbb92a679b3885d078821/firebase-app/src/jsMain/kotlin/dev/gitlive/firebase/firebase.kt#L55

If the code here was made a val, then it could be accessed by other code in the JS source sets. It would be great if code was in the common source set, but since not all Firebase platforms support/use it, so be it.

Note the pattern in in Firebas Auth is to use different FirebaseAuthException subtypes for each error type so you can switch on the type instead of code for auth and this is already implemented in the SDK

That's true but not always sufficient. For example, look at the docs for https://firebase.google.com/docs/reference/android/com/google/firebase/auth/FirebaseAuthInvalidUserException.

In order to know why the user was invalid, the Firebase team expects you to look at the code. There is no subtype of FirebaseAuthInvalidUserException that differentiates these, as far as I know.

leoull commented 1 year ago

This is how extract the message for now:

/**
 * convert FirebaseAuth exception to user friendly message
 * the following error codes and messages are retrieved from [FIRAuthErrorUtils.m]
 * Note: there could be a cleaner way of doing this once [FirebaseException exposes error code as a property](https://github.com/GitLiveApp/firebase-kotlin-sdk/issues/298) is addressed
 */
private fun getErrorDescription(e: Exception): String {
    val message = e.message ?: return "Error occurred, please try again"

    // try extracting user-friendly message
    extractErrorMessage(message)?.let {
        return it
    }

    // there are about 88 error codes, but only the major user errors are parsed here [for example, other types of errors occur because of developer mistakes]
    return when {
        message.contains("ERROR_INVALID_EMAIL") -> "The email address is badly formatted"
        message.contains("ERROR_INVALID_CREDENTIAL") -> "The supplied auth credential is malformed or has expired"
        message.contains("ERROR_USER_DISABLED") -> "The user account has been disabled by an administrator"
        message.contains("ERROR_EMAIL_ALREADY_IN_USE") -> "The email address is already in use by another account"
        message.contains("ERROR_WRONG_PASSWORD") -> "The password is invalid or the user does not have a password"
        message.contains("ERROR_TOO_MANY_REQUESTS") -> "We have blocked all requests from this device due to unusual activity. Try again later."
        message.contains("ERROR_ACCOUNT_EXISTS_WITH_DIFFERENT_CREDENTIAL") -> "An account already exists with the same email address but different sign-in credentials. Sign in using a provider associated with this email address"
        message.contains("ERROR_REQUIRES_RECENT_LOGIN") -> "This operation is sensitive and requires recent authentication. Log in again before retrying this request."
        message.contains("ERROR_PROVIDER_ALREADY_LINKED") -> "User can only be linked to one identity for the given provider"
        message.contains("ERROR_NO_SUCH_PROVIDER") -> "User was not linked to an account with the given provider"
        message.contains("ERROR_NETWORK_REQUEST_FAILED") -> "Network Error. please try again"
        message.contains("ERROR_USER_NOT_FOUND") -> "There is no user record corresponding to this identifier. your account may have been deleted"
        message.contains("ERROR_USER_MISMATCH") -> "The supplied credentials do not correspond to the previously signed in user."
        message.contains("ERROR_WEAK_PASSWORD") -> "The password must be 6 characters long or more"
        message.contains("ERROR_INVALID_VERIFICATION_CODE") -> "invalid verification code"
        else -> "Error occurred, please try again"
    }
}

/**
 * Extract error message.
 *
 * for example, given:
 *      Error Domain=FIRAuthErrorDomain Code=17025
 *      "This credential is already associated with a different user account."
 *      UserInfo={FIRAuthErrorUserInfoNameKey=ERROR_CREDENTIAL_ALREADY_IN_USE,
 *      FIRAuthErrorUserInfoEmailKey=leoul.emiru@gmail.com,
 *      FIRAuthErrorUserInfoUpdatedCredentialKey=<FIROAuthCredential: 0×280152cb0>,
 *      NSLocalizedDescription=This credential is already associated with a different user account.}
 *
 * returns: This credential is already associated with a different user account
 */
private fun extractErrorMessage(errorString: String): String? {
    val regex = "NSLocalizedDescription=(.*?)[.]}".toRegex()
    val matchResult = regex.find(errorString)

    return matchResult?.groupValues?.get(1)
}

/**
 * convert the given exception into another with user-friendly description
 */
fun Exception.makeUserFriendly() = Exception(getErrorDescription(this))