Admin: Make error codes for database errors more consistent (#5615)

* Admin: Make error codes for database errors more consistent

* More cases

* Nicer validation error message

admin/database/database.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto/rand"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"time"
 
@@ -276,12 +275,6 @@ type Tx interface {
 	Rollback() error
 }
 
-// ErrNotFound is returned for single row queries that return no values.
-var ErrNotFound = errors.New("database: not found")
-
-// ErrNotUnique is returned when a unique constraint is violated
-var ErrNotUnique = errors.New("database: violates unique constraint")
-
 // Organization represents a tenant.
 type Organization struct {
 	ID                                  string

admin/database/errors.go

@@ -0,0 +1,40 @@
+package database
+
+import "errors"
+
+// ErrNotFound is returned for single row queries that return no values.
+var ErrNotFound = errors.New("database: not found")
+
+// NewNotFoundError returns a new error that wraps ErrNotFound so checks with errors.Is(...) work.
+func NewNotFoundError(msg string) error {
+	return &wrappedError{msg: msg, err: ErrNotFound}
+}
+
+// ErrNotUnique is returned when a unique constraint is violated.
+var ErrNotUnique = errors.New("database: violates unique constraint")
+
+// NewNotUniqueError returns a new error that wraps ErrNotUnique so checks with errors.Is(...) work.
+func NewNotUniqueError(msg string) error {
+	return &wrappedError{msg: msg, err: ErrNotUnique}
+}
+
+// ErrValidation is returned when a validation check fails.
+var ErrValidation = errors.New("database: validation failed")
+
+// NewValidationError returns a new error that wraps ErrValidation so checks with errors.Is(...) work.
+func NewValidationError(msg string) error {
+	return &wrappedError{msg: msg, err: ErrValidation}
+}
+
+type wrappedError struct {
+	msg string
+	err error
+}
+
+func (e *wrappedError) Error() string {
+	return e.msg
+}
+
+func (e *wrappedError) Unwrap() error {
+	return e.err
+}

admin/database/postgres/postgres.go

@@ -2110,11 +2110,7 @@ func parseErr(target string, err error) error {
 		if target == "" {
 			return database.ErrNotFound
 		}
-		return &wrappedError{
-			msg: fmt.Sprintf("%s not found", target),
-			// wrap database.ErrNotFound so checks with errors.Is(...) still work
-			err: database.ErrNotFound,
-		}
+		return database.NewNotFoundError(fmt.Sprintf("%s not found", target))
 	}
 	var pgerr *pgconn.PgError
 	if !errors.As(err, &pgerr) {
@@ -2123,61 +2119,43 @@ func parseErr(target string, err error) error {
 	if pgerr.Code == "23505" { // unique_violation
 		switch pgerr.ConstraintName {
 		case "orgs_name_idx":
-			return newAlreadyExistsErr("an org with that name already exists")
+			return database.NewNotUniqueError("an org with that name already exists")
 		case "projects_name_idx":
-			return newAlreadyExistsErr("a project with that name already exists in the org")
+			return database.NewNotUniqueError("a project with that name already exists in the org")
 		case "users_email_idx":
-			return newAlreadyExistsErr("a user with that email already exists")
+			return database.NewNotUniqueError("a user with that email already exists")
 		case "usergroups_name_idx":
-			return newAlreadyExistsErr("a usergroup with that name already exists in the org")
+			return database.NewNotUniqueError("a usergroup with that name already exists in the org")
 		case "usergroups_users_pkey":
-			return newAlreadyExistsErr("user is already a member of the usergroup")
+			return database.NewNotUniqueError("user is already a member of the usergroup")
 		case "users_orgs_roles_pkey":
-			return newAlreadyExistsErr("user is already a member of the org")
+			return database.NewNotUniqueError("user is already a member of the org")
 		case "users_projects_roles_pkey":
-			return newAlreadyExistsErr("user is already a member of the project")
+			return database.NewNotUniqueError("user is already a member of the project")
 		case "usergroups_orgs_roles_pkey":
-			return newAlreadyExistsErr("usergroup is already a member of the org")
+			return database.NewNotUniqueError("usergroup is already a member of the org")
 		case "usergroups_projects_roles_pkey":
-			return newAlreadyExistsErr("usergroup is already a member of the project")
+			return database.NewNotUniqueError("usergroup is already a member of the project")
 		case "org_invites_email_org_idx":
-			return newAlreadyExistsErr("email has already been invited to the org")
+			return database.NewNotUniqueError("email has already been invited to the org")
 		case "project_invites_email_project_idx":
-			return newAlreadyExistsErr("email has already been invited to the project")
+			return database.NewNotUniqueError("email has already been invited to the project")
 		case "orgs_autoinvite_domains_org_id_domain_idx":
-			return newAlreadyExistsErr("domain has already been added for the org")
+			return database.NewNotUniqueError("domain has already been added for the org")
 		case "service_name_idx":
-			return newAlreadyExistsErr("a service with that name already exists in the org")
+			return database.NewNotUniqueError("a service with that name already exists in the org")
 		case "virtual_files_pkey":
-			return newAlreadyExistsErr("a virtual file already exists at that path")
+			return database.NewNotUniqueError("a virtual file already exists at that path")
 		default:
 			if target == "" {
 				return database.ErrNotUnique
 			}
-			return newAlreadyExistsErr(fmt.Sprintf("%s already exists", target))
+			return database.NewNotUniqueError(fmt.Sprintf("%s already exists", target))
 		}
 	}
 	return err
 }
 
-func newAlreadyExistsErr(msg string) error {
-	// wrap database.ErrNotUnique so checks with errors.Is(...) still work
-	return &wrappedError{msg: msg, err: database.ErrNotUnique}
-}
-
-type wrappedError struct {
-	msg string
-	err error
-}
-
-func (e *wrappedError) Error() string {
-	return e.msg
-}
-
-func (e *wrappedError) Unwrap() error {
-	return e.err
-}
-
 // encrypts plaintext using AES-GCM with the given key and returns the base64 encoded ciphertext
 func encrypt(plaintext string, key []byte) (string, error) {
 	block, err := aes.NewCipher(key)

admin/database/validate.go

@@ -1,14 +1,32 @@
 package database
 
 import (
+	"errors"
+	"fmt"
 	"regexp"
 
 	"github.com/go-playground/validator/v10"
 )
 
 // Validate validates a struct based on struct tags and other custom rules registered
 func Validate(v any) error {
-	return validate.Struct(v)
+	err := validate.Struct(v)
+	if err == nil {
+		return nil
+	}
+
+	var verrs validator.ValidationErrors
+	if errors.As(err, &verrs) && len(verrs) > 0 {
+		// Example: "Validation rule 'len=10' failed for field 'Name' ('InsertOptions.Name')"
+		verr := verrs[0]
+		var param string
+		if verr.Param() != "" {
+			param = fmt.Sprintf("=%s", verr.Param())
+		}
+		return NewValidationError(fmt.Sprintf("Validation rule '%s%s' failed for field '%s' ('%s')", verr.Tag(), param, verr.Field(), verr.StructNamespace()))
+	}
+
+	return NewValidationError(err.Error())
 }
 
 // validate caches parsed validation rules

admin/server/alerts.go

@@ -35,10 +35,7 @@ func (s *Server) GetAlertMeta(ctx context.Context, req *adminv1.GetAlertMetaRequ
 
 	proj, err := s.admin.DB.FindProject(ctx, req.ProjectId)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	permissions := auth.GetClaims(ctx).ProjectPermissions(ctx, proj.OrganizationID, proj.ID)
@@ -96,10 +93,7 @@ func (s *Server) CreateAlert(ctx context.Context, req *adminv1.CreateAlertReques
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	claims := auth.GetClaims(ctx)
@@ -163,10 +157,7 @@ func (s *Server) EditAlert(ctx context.Context, req *adminv1.EditAlertRequest) (
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	claims := auth.GetClaims(ctx)
@@ -234,10 +225,7 @@ func (s *Server) UnsubscribeAlert(ctx context.Context, req *adminv1.UnsubscribeA
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	claims := auth.GetClaims(ctx)
@@ -350,10 +338,7 @@ func (s *Server) DeleteAlert(ctx context.Context, req *adminv1.DeleteAlertReques
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	claims := auth.GetClaims(ctx)
@@ -427,10 +412,7 @@ func (s *Server) GetAlertYAML(ctx context.Context, req *adminv1.GetAlertYAMLRequ
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	claims := auth.GetClaims(ctx)

admin/server/bookmarks.go

@@ -52,10 +52,7 @@ func (s *Server) GetBookmark(ctx context.Context, req *adminv1.GetBookmarkReques
 
 	proj, err := s.admin.DB.FindProject(ctx, bookmark.ProjectID)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	permissions := claims.ProjectPermissions(ctx, proj.OrganizationID, proj.ID)
@@ -84,10 +81,7 @@ func (s *Server) CreateBookmark(ctx context.Context, req *adminv1.CreateBookmark
 
 	proj, err := s.admin.DB.FindProject(ctx, req.ProjectId)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	permissions := claims.ProjectPermissions(ctx, proj.OrganizationID, proj.ID)
@@ -128,7 +122,7 @@ func (s *Server) CreateBookmark(ctx context.Context, req *adminv1.CreateBookmark
 		Shared:       req.Shared,
 	})
 	if err != nil {
-		return nil, status.Error(codes.Internal, err.Error())
+		return nil, err
 	}
 
 	return &adminv1.CreateBookmarkResponse{
@@ -152,10 +146,7 @@ func (s *Server) UpdateBookmark(ctx context.Context, req *adminv1.UpdateBookmark
 
 	proj, err := s.admin.DB.FindProject(ctx, bookmark.ProjectID)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	permissions := claims.ProjectPermissions(ctx, proj.OrganizationID, proj.ID)
@@ -177,7 +168,7 @@ func (s *Server) UpdateBookmark(ctx context.Context, req *adminv1.UpdateBookmark
 		Shared:      req.Shared,
 	})
 	if err != nil {
-		return nil, status.Error(codes.Internal, err.Error())
+		return nil, err
 	}
 
 	return &adminv1.UpdateBookmarkResponse{}, nil
@@ -199,10 +190,7 @@ func (s *Server) RemoveBookmark(ctx context.Context, req *adminv1.RemoveBookmark
 
 	proj, err := s.admin.DB.FindProject(ctx, bookmark.ProjectID)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "project not found")
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	permissions := claims.ProjectPermissions(ctx, proj.OrganizationID, proj.ID)

admin/server/github.go

@@ -62,10 +62,7 @@ func (s *Server) GetGithubUserStatus(ctx context.Context, req *adminv1.GetGithub
 
 	user, err := s.admin.DB.FindUser(ctx, claims.OwnerID())
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "user not found")
-		}
-		return nil, status.Error(codes.Internal, err.Error())
+		return nil, err
 	}
 	if user.GithubUsername == "" {
 		// If we don't have user's github username we navigate user to installtion assuming they never installed github app

admin/server/magic_tokens.go

@@ -2,7 +2,6 @@ package server
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"time"
 
@@ -31,10 +30,7 @@ func (s *Server) IssueMagicAuthToken(ctx context.Context, req *adminv1.IssueMagi
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, fmt.Sprintf("project %q not found", req.Project))
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	org, err := s.admin.DB.FindOrganization(ctx, proj.OrganizationID)
@@ -109,10 +105,7 @@ func (s *Server) GetCurrentMagicAuthToken(ctx context.Context, req *adminv1.GetC
 
 	tkn, err := s.admin.DB.FindMagicAuthTokenWithUser(ctx, claims.OwnerID())
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, "magic auth token not found")
-		}
-		return nil, status.Error(codes.Internal, err.Error())
+		return nil, err
 	}
 
 	pb, err := magicAuthTokenToPB(tkn)
@@ -139,10 +132,7 @@ func (s *Server) ListMagicAuthTokens(ctx context.Context, req *adminv1.ListMagic
 
 	proj, err := s.admin.DB.FindProjectByName(ctx, req.Organization, req.Project)
 	if err != nil {
-		if errors.Is(err, database.ErrNotFound) {
-			return nil, status.Error(codes.NotFound, fmt.Sprintf("project %q not found", req.Project))
-		}
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		return nil, err
 	}
 
 	claims := auth.GetClaims(ctx)