Implementing Functions to Test Error Codes #131
Conversation
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
| @@ -345,10 +349,7 @@ func processClaims(p map[string]interface{}) error { | |||
| return nil | |||
| } | |||
|
|
|||
| claims, ok := cc.(map[string]interface{}) | |||
| if !ok { | |||
There was a problem hiding this comment.
Are you sure you don't want to leave some kind of error reported here?
There was a problem hiding this comment.
This is guaranteed to be a map since we have typed functions for setting custom claims (SetCustomClaims() for instance).
auth/user_mgt.go
Outdated
| "PROJECT_NOT_FOUND": projectNotFound, | ||
| } | ||
|
|
||
| func handleServerError(err error) *internal.FirebaseError { |
There was a problem hiding this comment.
I would probably use error for the return type here, just because I've been bitten too many times by trying to use a specific error type.
| if !ok { | ||
| clientCode = unknown | ||
| } | ||
| return internal.Error(clientCode, err.Error()) |
There was a problem hiding this comment.
It seems like this might get called in cases where the error is generated on the client by a failure to connect or something. Do we want to always wrap error in our own type like this, possibly losing original error data other than the string? Or should we just pass on errors that aren't from the backend as-is? This is something I've been thinking about a lot on all of our SDKs.
There was a problem hiding this comment.
Good catch. We shouldn't be wrapping non-backend errors. Updated the implementation.
internal/internal.go
Outdated
| // If the error does not contain an error code, this function returns an empty string. Use this | ||
| // internal function for unit tests in packages that cannot directly import the root firebase | ||
| // package. | ||
| func Code(err error) string { |
There was a problem hiding this comment.
It looks like this isn't actually used anywhere?
* Changlog updates (#123) * Reading FCM error code from details section (#127) * Adding additional details to the error message when available (#129) * Implementing support for APNS content-mutable field (#126) * Implementing support for APNS content-mutable field * Corrected the mutable-content option name * Renamed CustomFields to CustomData * Added comment * Implementing Functions to Test Error Codes (#131) * Adding error codes to the auth package * Added error codes for messaging * Responding to review feedback; Updated changelog * Add Go import comments. (#132) * Bumped version to 2.7.0 (#133) * Bumped version to 2.7.0 * Updated changelog
* Adding error codes to the auth package * Added error codes for messaging * Responding to review feedback; Updated changelog
This PR improves error handling in the SDK by providing explicit functions that test for various error codes: