-
Notifications
You must be signed in to change notification settings - Fork 268
Implementing Functions to Test Error Codes #131
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,8 +23,10 @@ import ( | |
| "strings" | ||
| "time" | ||
|
|
||
| "firebase.google.com/go/internal" | ||
| "golang.org/x/net/context" | ||
|
|
||
| "google.golang.org/api/googleapi" | ||
| "google.golang.org/api/identitytoolkit/v3" | ||
| "google.golang.org/api/iterator" | ||
| ) | ||
|
|
@@ -215,8 +217,10 @@ func (c *Client) DeleteUser(ctx context.Context, uid string) error { | |
|
|
||
| call := c.is.Relyingparty.DeleteAccount(request) | ||
| c.setHeader(call) | ||
| _, err := call.Context(ctx).Do() | ||
| return err | ||
| if _, err := call.Context(ctx).Do(); err != nil { | ||
| return handleServerError(err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // GetUser gets the user data corresponding to the specified user ID. | ||
|
|
@@ -279,7 +283,7 @@ func (it *UserIterator) fetch(pageSize int, pageToken string) (string, error) { | |
| it.client.setHeader(call) | ||
| resp, err := call.Context(it.ctx).Do() | ||
| if err != nil { | ||
| return "", err | ||
| return "", handleServerError(err) | ||
| } | ||
|
|
||
| for _, u := range resp.Users { | ||
|
|
@@ -345,10 +349,7 @@ func processClaims(p map[string]interface{}) error { | |
| return nil | ||
| } | ||
|
|
||
| claims, ok := cc.(map[string]interface{}) | ||
| if !ok { | ||
| return fmt.Errorf("unexpected type for custom claims") | ||
| } | ||
| claims := cc.(map[string]interface{}) | ||
| for _, key := range reservedClaims { | ||
| if _, ok := claims[key]; ok { | ||
| return fmt.Errorf("claim %q is reserved and must not be set", key) | ||
|
|
@@ -372,6 +373,83 @@ func processClaims(p map[string]interface{}) error { | |
| return nil | ||
| } | ||
|
|
||
| // Error handlers. | ||
|
|
||
| const ( | ||
| emailAlredyExists = "email-already-exists" | ||
| idTokenRevoked = "id-token-revoked" | ||
| insufficientPermission = "insufficient-permission" | ||
| phoneNumberAlreadyExists = "phone-number-already-exists" | ||
| projectNotFound = "project-not-found" | ||
| uidAlreadyExists = "uid-already-exists" | ||
| unknown = "unknown-error" | ||
| userNotFound = "user-not-found" | ||
| ) | ||
|
|
||
| // IsEmailAlreadyExists checks if the given error was due to a duplicate email. | ||
| func IsEmailAlreadyExists(err error) bool { | ||
| return internal.HasErrorCode(err, emailAlredyExists) | ||
| } | ||
|
|
||
| // IsIDTokenRevoked checks if the given error was due to a revoked ID token. | ||
| func IsIDTokenRevoked(err error) bool { | ||
| return internal.HasErrorCode(err, idTokenRevoked) | ||
| } | ||
|
|
||
| // IsInsufficientPermission checks if the given error was due to insufficient permissions. | ||
| func IsInsufficientPermission(err error) bool { | ||
| return internal.HasErrorCode(err, insufficientPermission) | ||
| } | ||
|
|
||
| // IsPhoneNumberAlreadyExists checks if the given error was due to a duplicate phone number. | ||
| func IsPhoneNumberAlreadyExists(err error) bool { | ||
| return internal.HasErrorCode(err, phoneNumberAlreadyExists) | ||
| } | ||
|
|
||
| // IsProjectNotFound checks if the given error was due to a non-existing project. | ||
| func IsProjectNotFound(err error) bool { | ||
| return internal.HasErrorCode(err, projectNotFound) | ||
| } | ||
|
|
||
| // IsUIDAlreadyExists checks if the given error was due to a duplicate uid. | ||
| func IsUIDAlreadyExists(err error) bool { | ||
| return internal.HasErrorCode(err, uidAlreadyExists) | ||
| } | ||
|
|
||
| // IsUnknown checks if the given error was due to a unknown server error. | ||
| func IsUnknown(err error) bool { | ||
| return internal.HasErrorCode(err, unknown) | ||
| } | ||
|
|
||
| // IsUserNotFound checks if the given error was due to non-existing user. | ||
| func IsUserNotFound(err error) bool { | ||
| return internal.HasErrorCode(err, userNotFound) | ||
| } | ||
|
|
||
| var serverError = map[string]string{ | ||
| "CONFIGURATION_NOT_FOUND": projectNotFound, | ||
| "DUPLICATE_EMAIL": emailAlredyExists, | ||
| "DUPLICATE_LOCAL_ID": uidAlreadyExists, | ||
| "EMAIL_EXISTS": emailAlredyExists, | ||
| "INSUFFICIENT_PERMISSION": insufficientPermission, | ||
| "PHONE_NUMBER_EXISTS": phoneNumberAlreadyExists, | ||
| "PROJECT_NOT_FOUND": projectNotFound, | ||
| } | ||
|
|
||
| func handleServerError(err error) error { | ||
| gerr, ok := err.(*googleapi.Error) | ||
| if !ok { | ||
| // Not a back-end error | ||
| return err | ||
| } | ||
| serverCode := gerr.Message | ||
| clientCode, ok := serverError[serverCode] | ||
| if !ok { | ||
| clientCode = unknown | ||
| } | ||
| return internal.Error(clientCode, err.Error()) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. We shouldn't be wrapping non-backend errors. Updated the implementation. |
||
| } | ||
|
|
||
| // Validators. | ||
|
|
||
| func validateDisplayName(val interface{}) error { | ||
|
|
@@ -532,7 +610,7 @@ func (c *Client) createUser(ctx context.Context, user *UserToCreate) (string, er | |
| c.setHeader(call) | ||
| resp, err := call.Context(ctx).Do() | ||
| if err != nil { | ||
| return "", err | ||
| return "", handleServerError(err) | ||
| } | ||
|
|
||
| return resp.LocalId, nil | ||
|
|
@@ -555,20 +633,29 @@ func (c *Client) updateUser(ctx context.Context, uid string, user *UserToUpdate) | |
|
|
||
| call := c.is.Relyingparty.SetAccountInfo(request) | ||
| c.setHeader(call) | ||
| _, err := call.Context(ctx).Do() | ||
|
|
||
| return err | ||
| if _, err := call.Context(ctx).Do(); err != nil { | ||
| return handleServerError(err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (c *Client) getUser(ctx context.Context, request *identitytoolkit.IdentitytoolkitRelyingpartyGetAccountInfoRequest) (*UserRecord, error) { | ||
| call := c.is.Relyingparty.GetAccountInfo(request) | ||
| c.setHeader(call) | ||
| resp, err := call.Context(ctx).Do() | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, handleServerError(err) | ||
| } | ||
| if len(resp.Users) == 0 { | ||
| return nil, fmt.Errorf("cannot find user given params: id:%v, phone:%v, email: %v", request.LocalId, request.PhoneNumber, request.Email) | ||
| var msg string | ||
| if len(request.LocalId) == 1 { | ||
| msg = fmt.Sprintf("cannot find user from uid: %q", request.LocalId[0]) | ||
| } else if len(request.Email) == 1 { | ||
| msg = fmt.Sprintf("cannot find user from email: %q", request.Email[0]) | ||
| } else { | ||
| msg = fmt.Sprintf("cannot find user from phone number: %q", request.PhoneNumber[0]) | ||
| } | ||
| return nil, internal.Error(userNotFound, msg) | ||
| } | ||
|
|
||
| eu, err := makeExportedUser(resp.Users[0]) | ||
|
|
@@ -581,8 +668,7 @@ func (c *Client) getUser(ctx context.Context, request *identitytoolkit.Identityt | |
| func makeExportedUser(r *identitytoolkit.UserInfo) (*ExportedUserRecord, error) { | ||
| var cc map[string]interface{} | ||
| if r.CustomAttributes != "" { | ||
| err := json.Unmarshal([]byte(r.CustomAttributes), &cc) | ||
| if err != nil { | ||
| if err := json.Unmarshal([]byte(r.CustomAttributes), &cc); err != nil { | ||
| return nil, err | ||
| } | ||
| if len(cc) == 0 { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,23 +149,21 @@ func TestGetNonExistingUser(t *testing.T) { | |
| s := echoServer([]byte(resp), t) | ||
| defer s.Close() | ||
|
|
||
| want := "cannot find user given params: id:[%s], phone:[%s], email: [%s]" | ||
|
|
||
| we := fmt.Sprintf(want, "id-nonexisting", "", "") | ||
| we := `cannot find user from uid: "id-nonexisting"` | ||
| user, err := s.Client.GetUser(context.Background(), "id-nonexisting") | ||
| if user != nil || err == nil || err.Error() != we { | ||
| if user != nil || err == nil || err.Error() != we || !IsUserNotFound(err) { | ||
| t.Errorf("GetUser(non-existing) = (%v, %q); want = (nil, %q)", user, err, we) | ||
| } | ||
|
|
||
| we = fmt.Sprintf(want, "", "", "[email protected]") | ||
| we = `cannot find user from email: "[email protected]"` | ||
| user, err = s.Client.GetUserByEmail(context.Background(), "[email protected]") | ||
| if user != nil || err == nil || err.Error() != we { | ||
| if user != nil || err == nil || err.Error() != we || !IsUserNotFound(err) { | ||
| t.Errorf("GetUserByEmail(non-existing) = (%v, %q); want = (nil, %q)", user, err, we) | ||
| } | ||
|
|
||
| we = fmt.Sprintf(want, "", "+12345678901", "") | ||
| we = `cannot find user from phone number: "+12345678901"` | ||
| user, err = s.Client.GetUserByPhoneNumber(context.Background(), "+12345678901") | ||
| if user != nil || err == nil || err.Error() != we { | ||
| if user != nil || err == nil || err.Error() != we || !IsUserNotFound(err) { | ||
| t.Errorf("GetUserPhoneNumber(non-existing) = (%v, %q); want = (nil, %q)", user, err, we) | ||
| } | ||
| } | ||
|
|
@@ -642,7 +640,6 @@ func TestInvalidDeleteUser(t *testing.T) { | |
| } | ||
|
|
||
| func TestMakeExportedUser(t *testing.T) { | ||
|
|
||
| rur := &identitytoolkit.UserInfo{ | ||
| LocalId: "testuser", | ||
| Email: "[email protected]", | ||
|
|
@@ -704,11 +701,39 @@ func TestHTTPError(t *testing.T) { | |
| } | ||
|
|
||
| want := `googleapi: got HTTP response code 500 with body: {"error":"test"}` | ||
| if err.Error() != want { | ||
| if err.Error() != want || !IsUnknown(err) { | ||
| t.Errorf("GetUser() = %v; want = %q", err, want) | ||
| } | ||
| } | ||
|
|
||
| func TestHTTPErrorWithCode(t *testing.T) { | ||
| errorCodes := map[string]func(error) bool{ | ||
| "CONFIGURATION_NOT_FOUND": IsProjectNotFound, | ||
| "DUPLICATE_EMAIL": IsEmailAlreadyExists, | ||
| "DUPLICATE_LOCAL_ID": IsUIDAlreadyExists, | ||
| "EMAIL_EXISTS": IsEmailAlreadyExists, | ||
| "INSUFFICIENT_PERMISSION": IsInsufficientPermission, | ||
| "PHONE_NUMBER_EXISTS": IsPhoneNumberAlreadyExists, | ||
| "PROJECT_NOT_FOUND": IsProjectNotFound, | ||
| } | ||
| s := echoServer(nil, t) | ||
| defer s.Close() | ||
| s.Status = http.StatusInternalServerError | ||
|
|
||
| for code, check := range errorCodes { | ||
| s.Resp = []byte(fmt.Sprintf(`{"error":{"message":"%s"}}`, code)) | ||
| u, err := s.Client.GetUser(context.Background(), "some uid") | ||
| if u != nil || err == nil { | ||
| t.Fatalf("GetUser() = (%v, %v); want = (nil, error)", u, err) | ||
| } | ||
|
|
||
| want := fmt.Sprintf("googleapi: Error 500: %s", code) | ||
| if err.Error() != want || !check(err) { | ||
| t.Errorf("GetUser() = %v; want = %q", err, want) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| type mockAuthServer struct { | ||
| Resp []byte | ||
| Header map[string]string | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you don't want to leave some kind of error reported here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is guaranteed to be a map since we have typed functions for setting custom claims (
SetCustomClaims()for instance).