RoleStore migration (#15017)

Automatic Merge

app/import_functions_test.go

@@ -98,43 +98,43 @@ func TestImportImportScheme(t *testing.T) {
 	assert.Equal(t, *data.Description, scheme.Description)
 	assert.Equal(t, *data.Scope, scheme.Scope)
 
-	role, err := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
+	role, nErr := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
@@ -155,43 +155,43 @@ func TestImportImportScheme(t *testing.T) {
 	assert.Equal(t, *data.Description, scheme.Description)
 	assert.Equal(t, *data.Scope, scheme.Scope)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
@@ -285,43 +285,43 @@ func TestImportImportSchemeWithoutGuestRoles(t *testing.T) {
 	assert.Equal(t, *data.Description, scheme.Description)
 	assert.Equal(t, *data.Scope, scheme.Scope)
 
-	role, err := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
+	role, nErr := th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
@@ -342,43 +342,43 @@ func TestImportImportSchemeWithoutGuestRoles(t *testing.T) {
 	assert.Equal(t, *data.Description, scheme.Description)
 	assert.Equal(t, *data.Scope, scheme.Scope)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultTeamGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelAdminRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelUserRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
 	assert.True(t, role.SchemeManaged)
 
-	role, err = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
+	role, nErr = th.App.Srv().Store.Role().GetByName(scheme.DefaultChannelGuestRole)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName)
 	assert.False(t, role.BuiltIn)
@@ -412,8 +412,8 @@ func TestImportImportRole(t *testing.T) {
 	err := th.App.importRole(&data, true, false)
 	require.NotNil(t, err, "Should have failed to import.")
 
-	_, err = th.App.Srv().Store.Role().GetByName(rid1)
+	_, nErr := th.App.Srv().Store.Role().GetByName(rid1)
-	require.NotNil(t, err, "Should have failed to import.")
+	require.NotNil(t, nErr, "Should have failed to import.")
 
 	// Try importing the valid role in dryRun mode.
 	data.DisplayName = ptrStr("display name")
@@ -421,8 +421,8 @@ func TestImportImportRole(t *testing.T) {
 	err = th.App.importRole(&data, true, false)
 	require.Nil(t, err, "Should have succeeded.")
 
-	_, err = th.App.Srv().Store.Role().GetByName(rid1)
+	_, nErr = th.App.Srv().Store.Role().GetByName(rid1)
-	require.NotNil(t, err, "Role should not have imported as we are in dry run mode.")
+	require.NotNil(t, nErr, "Role should not have imported as we are in dry run mode.")
 
 	// Try importing an invalid role.
 	data.DisplayName = nil
@@ -430,8 +430,8 @@ func TestImportImportRole(t *testing.T) {
 	err = th.App.importRole(&data, false, false)
 	require.NotNil(t, err, "Should have failed to import.")
 
-	_, err = th.App.Srv().Store.Role().GetByName(rid1)
+	_, nErr = th.App.Srv().Store.Role().GetByName(rid1)
-	require.NotNil(t, err, "Role should not have imported.")
+	require.NotNil(t, nErr, "Role should not have imported.")
 
 	// Try importing a valid role with all params set.
 	data.DisplayName = ptrStr("display name")
@@ -441,8 +441,8 @@ func TestImportImportRole(t *testing.T) {
 	err = th.App.importRole(&data, false, false)
 	require.Nil(t, err, "Should have succeeded.")
 
-	role, err := th.App.Srv().Store.Role().GetByName(rid1)
+	role, nErr := th.App.Srv().Store.Role().GetByName(rid1)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.Name, role.Name)
 	assert.Equal(t, *data.DisplayName, role.DisplayName)
@@ -459,8 +459,8 @@ func TestImportImportRole(t *testing.T) {
 	err = th.App.importRole(&data, false, true)
 	require.Nil(t, err, "Should have succeeded. %v", err)
 
-	role, err = th.App.Srv().Store.Role().GetByName(rid1)
+	role, nErr = th.App.Srv().Store.Role().GetByName(rid1)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data.Name, role.Name)
 	assert.Equal(t, *data.DisplayName, role.DisplayName)
@@ -478,8 +478,8 @@ func TestImportImportRole(t *testing.T) {
 	err = th.App.importRole(&data2, false, false)
 	require.Nil(t, err, "Should have succeeded.")
 
-	role, err = th.App.Srv().Store.Role().GetByName(rid1)
+	role, nErr = th.App.Srv().Store.Role().GetByName(rid1)
-	require.Nil(t, err, "Should have found the imported role.")
+	require.Nil(t, nErr, "Should have found the imported role.")
 
 	assert.Equal(t, *data2.Name, role.Name)
 	assert.Equal(t, *data2.DisplayName, role.DisplayName)

app/migrations.go

@@ -122,8 +122,8 @@ func (a *App) DoEmojisPermissionsMigration() {
 
 	if role != nil {
 		role.Permissions = append(role.Permissions, model.PERMISSION_CREATE_EMOJIS.Id, model.PERMISSION_DELETE_EMOJIS.Id)
-		if _, err = a.Srv().Store.Role().Save(role); err != nil {
+		if _, nErr := a.Srv().Store.Role().Save(role); nErr != nil {
-			mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
+			mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(nErr))
 			return
 		}
 	}

app/permissions.go

@@ -50,7 +50,7 @@ func (a *App) ResetPermissionsSystem() *model.AppError {
 
 	// Purge all roles from the database.
 	if err := a.Srv().Store.Role().PermanentDeleteAll(); err != nil {
-		return err
+		return model.NewAppError("ResetPermissionsSystem", "app.role.permanent_delete_all.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	// Remove the "System" table entry that marks the advanced permissions migration as done.

app/permissions_migrations.go

@@ -4,9 +4,12 @@
 package app
 
 import (
+	"errors"
+	"net/http"
 	"strings"
 
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
 )
 
 type permissionTransformation struct {
@@ -162,7 +165,13 @@ func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *m
 	for _, role := range roles {
 		role.Permissions = applyPermissionsMap(role, roleMap, migrationMap)
 		if _, err := a.Srv().Store.Role().Save(role); err != nil {
-			return err
+			var invErr *store.ErrInvalidInput
+			switch {
+			case errors.As(err, &invErr):
+				return model.NewAppError("doPermissionsMigration", "app.role.save.invalid_role.app_error", nil, invErr.Error(), http.StatusBadRequest)
+			default:
+				return model.NewAppError("doPermissionsMigration", "app.role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError)
+			}
 		}
 	}
 

app/role.go

@@ -4,29 +4,53 @@
 package app
 
 import (
+	"errors"
 	"net/http"
 	"reflect"
 	"strings"
 
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
 	"github.com/mattermost/mattermost-server/v5/utils"
 )
 
 func (a *App) GetRole(id string) (*model.Role, *model.AppError) {
-	return a.Srv().Store.Role().Get(id)
+	role, err := a.Srv().Store.Role().Get(id)
+	if err != nil {
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(err, &nfErr):
+			return nil, model.NewAppError("GetRole", "app.role.get.app_error", nil, nfErr.Error(), http.StatusNotFound)
+		default:
+			return nil, model.NewAppError("GetRole", "app.role.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
+	}
+
+	return role, nil
 }
 
 func (a *App) GetAllRoles() ([]*model.Role, *model.AppError) {
-	return a.Srv().Store.Role().GetAll()
+	roles, err := a.Srv().Store.Role().GetAll()
+	if err != nil {
+		return nil, model.NewAppError("GetAllRoles", "app.role.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+
+	return roles, nil
 }
 
 func (s *Server) GetRoleByName(name string) (*model.Role, *model.AppError) {
-	role, err := s.Store.Role().GetByName(name)
+	role, nErr := s.Store.Role().GetByName(name)
-	if err != nil {
+	if nErr != nil {
-		return nil, err
+		var nfErr *store.ErrNotFound
+		switch {
+		case errors.As(nErr, &nfErr):
+			return nil, model.NewAppError("GetRoleByName", "app.role.get_by_name.app_error", nil, nfErr.Error(), http.StatusNotFound)
+		default:
+			return nil, model.NewAppError("GetRoleByName", "app.role.get_by_name.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+		}
 	}
 
-	err = s.mergeChannelHigherScopedPermissions([]*model.Role{role})
+	err := s.mergeChannelHigherScopedPermissions([]*model.Role{role})
 	if err != nil {
 		return nil, err
 	}
@@ -39,12 +63,12 @@ func (a *App) GetRoleByName(name string) (*model.Role, *model.AppError) {
 }
 
 func (a *App) GetRolesByNames(names []string) ([]*model.Role, *model.AppError) {
-	roles, err := a.Srv().Store.Role().GetByNames(names)
+	roles, nErr := a.Srv().Store.Role().GetByNames(names)
-	if err != nil {
+	if nErr != nil {
-		return nil, err
+		return nil, model.NewAppError("GetRolesByNames", "app.role.get_by_names.app_error", nil, nErr.Error(), http.StatusInternalServerError)
 	}
 
-	err = a.mergeChannelHigherScopedPermissions(roles)
+	err := a.mergeChannelHigherScopedPermissions(roles)
 	if err != nil {
 		return nil, err
 	}
@@ -69,7 +93,7 @@ func (s *Server) mergeChannelHigherScopedPermissions(roles []*model.Role) *model
 
 	higherScopedPermissionsMap, err := s.Store.Role().ChannelHigherScopedPermissions(higherScopeNamesToQuery)
 	if err != nil {
-		return err
+		return model.NewAppError("mergeChannelHigherScopedPermissions", "app.role.get_by_names.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	for _, role := range roles {
@@ -112,14 +136,31 @@ func (a *App) CreateRole(role *model.Role) (*model.Role, *model.AppError) {
 	role.BuiltIn = false
 	role.SchemeManaged = false
 
-	return a.Srv().Store.Role().Save(role)
+	var err error
+	role, err = a.Srv().Store.Role().Save(role)
+	if err != nil {
+		var invErr *store.ErrInvalidInput
+		switch {
+		case errors.As(err, &invErr):
+			return nil, model.NewAppError("CreateRole", "app.role.save.invalid_role.app_error", nil, invErr.Error(), http.StatusBadRequest)
+		default:
+			return nil, model.NewAppError("CreateRole", "app.role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
+	}
 
+	return role, nil
 }
 
 func (a *App) UpdateRole(role *model.Role) (*model.Role, *model.AppError) {
 	savedRole, err := a.Srv().Store.Role().Save(role)
 	if err != nil {
-		return nil, err
+		var invErr *store.ErrInvalidInput
+		switch {
+		case errors.As(err, &invErr):
+			return nil, model.NewAppError("UpdateRole", "app.role.save.invalid_role.app_error", nil, invErr.Error(), http.StatusBadRequest)
+		default:
+			return nil, model.NewAppError("UpdateRole", "app.role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
 	}
 
 	builtInChannelRoles := []string{
@@ -138,23 +179,33 @@ func (a *App) UpdateRole(role *model.Role) (*model.Role, *model.AppError) {
 
 	if utils.StringInSlice(savedRole.Name, builtInChannelRoles) {
 		roleRetrievalFunc = func() ([]*model.Role, *model.AppError) {
-			return a.Srv().Store.Role().AllChannelSchemeRoles()
+			roles, nErr := a.Srv().Store.Role().AllChannelSchemeRoles()
+			if nErr != nil {
+				return nil, model.NewAppError("UpdateRole", "app.role.get.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+			}
+
+			return roles, nil
 		}
 	} else {
 		roleRetrievalFunc = func() ([]*model.Role, *model.AppError) {
-			return a.Srv().Store.Role().ChannelRolesUnderTeamRole(savedRole.Name)
+			roles, nErr := a.Srv().Store.Role().ChannelRolesUnderTeamRole(savedRole.Name)
+			if nErr != nil {
+				return nil, model.NewAppError("UpdateRole", "app.role.get.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+			}
+
+			return roles, nil
 		}
 	}
 
-	impactedRoles, err := roleRetrievalFunc()
+	impactedRoles, appErr := roleRetrievalFunc()
-	if err != nil {
+	if appErr != nil {
-		return nil, err
+		return nil, appErr
 	}
 	impactedRoles = append(impactedRoles, role)
 
-	err = a.mergeChannelHigherScopedPermissions(impactedRoles)
+	appErr = a.mergeChannelHigherScopedPermissions(impactedRoles)
-	if err != nil {
+	if appErr != nil {
-		return nil, err
+		return nil, appErr
 	}
 
 	for _, ir := range impactedRoles {

i18n/en.json

@@ -4298,6 +4298,34 @@
     "id": "app.role.check_roles_exist.role_not_found",
     "translation": "The provided role does not exist"
   },
+  {
+    "id": "app.role.get.app_error",
+    "translation": "Unable to get role."
+  },
+  {
+    "id": "app.role.get_all.app_error",
+    "translation": "Unable to get all the roles."
+  },
+  {
+    "id": "app.role.get_by_name.app_error",
+    "translation": "Unable to get role."
+  },
+  {
+    "id": "app.role.get_by_names.app_error",
+    "translation": "Unable to get roles."
+  },
+  {
+    "id": "app.role.permanent_delete_all.app_error",
+    "translation": "We could not permanently delete all the roles."
+  },
+  {
+    "id": "app.role.save.insert.app_error",
+    "translation": "Unable to save new role."
+  },
+  {
+    "id": "app.role.save.invalid_role.app_error",
+    "translation": "The role was not valid."
+  },
   {
     "id": "app.save_config.app_error",
     "translation": "An error occurred saving the configuration."
@@ -7258,50 +7286,6 @@
     "id": "store.sql_post.update.app_error",
     "translation": "Unable to update the Post."
   },
-  {
-    "id": "store.sql_role.delete.update.app_error",
-    "translation": "Unable to delete the role."
-  },
-  {
-    "id": "store.sql_role.get.app_error",
-    "translation": "Unable to get role."
-  },
-  {
-    "id": "store.sql_role.get_all.app_error",
-    "translation": "Unable to get all the roles."
-  },
-  {
-    "id": "store.sql_role.get_by_name.app_error",
-    "translation": "Unable to get role."
-  },
-  {
-    "id": "store.sql_role.get_by_names.app_error",
-    "translation": "Unable to get roles."
-  },
-  {
-    "id": "store.sql_role.permanent_delete_all.app_error",
-    "translation": "We could not permanently delete all the roles."
-  },
-  {
-    "id": "store.sql_role.save.insert.app_error",
-    "translation": "Unable to save new role."
-  },
-  {
-    "id": "store.sql_role.save.invalid_role.app_error",
-    "translation": "The role was not valid."
-  },
-  {
-    "id": "store.sql_role.save.open_transaction.app_error",
-    "translation": "Failed to open the transaction to save the role."
-  },
-  {
-    "id": "store.sql_role.save.update.app_error",
-    "translation": "Unable to update role."
-  },
-  {
-    "id": "store.sql_role.save_role.commit_transaction.app_error",
-    "translation": "Failed to commit the transaction to save the role."
-  },
   {
     "id": "store.sql_status.get.app_error",
     "translation": "Encountered an error retrieving the status."

store/localcachelayer/role_layer.go

@@ -32,7 +32,7 @@ func (s *LocalCacheRoleStore) handleClusterInvalidateRolePermissions(msg *model.
 	}
 }
 
-func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
+func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, error) {
 	if len(role.Name) != 0 {
 		defer s.rootStore.doInvalidateCacheCluster(s.rootStore.roleCache, role.Name)
 		defer s.rootStore.doClearCacheCluster(s.rootStore.rolePermissionsCache)
@@ -40,7 +40,7 @@ func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, *model.AppErro
 	return s.RoleStore.Save(role)
 }
 
-func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, *model.AppError) {
+func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, error) {
 	var role *model.Role
 	if err := s.rootStore.doStandardReadCache(s.rootStore.roleCache, name, &role); err == nil {
 		return role, nil
@@ -54,7 +54,7 @@ func (s LocalCacheRoleStore) GetByName(name string) (*model.Role, *model.AppErro
 	return role, nil
 }
 
-func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
+func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, error) {
 	var foundRoles []*model.Role
 	var rolesToQuery []string
 
@@ -76,7 +76,7 @@ func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, *model.A
 	return append(foundRoles, roles...), nil
 }
 
-func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
+func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, error) {
 	role, err := s.RoleStore.Delete(roleId)
 
 	if err == nil {
@@ -86,7 +86,7 @@ func (s LocalCacheRoleStore) Delete(roleId string) (*model.Role, *model.AppError
 	return role, err
 }
 
-func (s LocalCacheRoleStore) PermanentDeleteAll() *model.AppError {
+func (s LocalCacheRoleStore) PermanentDeleteAll() error {
 	defer s.rootStore.roleCache.Purge()
 	defer s.rootStore.doClearCacheCluster(s.rootStore.roleCache)
 	defer s.rootStore.doClearCacheCluster(s.rootStore.rolePermissionsCache)
@@ -94,7 +94,7 @@ func (s LocalCacheRoleStore) PermanentDeleteAll() *model.AppError {
 	return s.RoleStore.PermanentDeleteAll()
 }
 
-func (s LocalCacheRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) {
+func (s LocalCacheRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) {
 	sort.Strings(roleNames)
 	cacheKey := strings.Join(roleNames, "/")
 	var rolePermissionsMap map[string]*model.RolePermissions

store/opentracinglayer/opentracinglayer.go

@@ -5617,7 +5617,7 @@ func (s *OpenTracingLayerReactionStore) Save(reaction *model.Reaction) (*model.R
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.AllChannelSchemeRoles")
 	s.Root.Store.SetContext(newCtx)
@@ -5635,7 +5635,7 @@ func (s *OpenTracingLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *mod
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.ChannelHigherScopedPermissions")
 	s.Root.Store.SetContext(newCtx)
@@ -5653,7 +5653,7 @@ func (s *OpenTracingLayerRoleStore) ChannelHigherScopedPermissions(roleNames []s
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.ChannelRolesUnderTeamRole")
 	s.Root.Store.SetContext(newCtx)
@@ -5671,7 +5671,7 @@ func (s *OpenTracingLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) (
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) Delete(roleId string) (*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.Delete")
 	s.Root.Store.SetContext(newCtx)
@@ -5689,7 +5689,7 @@ func (s *OpenTracingLayerRoleStore) Delete(roleId string) (*model.Role, *model.A
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) Get(roleId string) (*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) Get(roleId string) (*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.Get")
 	s.Root.Store.SetContext(newCtx)
@@ -5707,7 +5707,7 @@ func (s *OpenTracingLayerRoleStore) Get(roleId string) (*model.Role, *model.AppE
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) GetAll() ([]*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.GetAll")
 	s.Root.Store.SetContext(newCtx)
@@ -5725,7 +5725,7 @@ func (s *OpenTracingLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) {
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) GetByName(name string) (*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) GetByName(name string) (*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.GetByName")
 	s.Root.Store.SetContext(newCtx)
@@ -5743,7 +5743,7 @@ func (s *OpenTracingLayerRoleStore) GetByName(name string) (*model.Role, *model.
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) GetByNames(names []string) ([]*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.GetByNames")
 	s.Root.Store.SetContext(newCtx)
@@ -5761,7 +5761,7 @@ func (s *OpenTracingLayerRoleStore) GetByNames(names []string) ([]*model.Role, *
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerRoleStore) PermanentDeleteAll() *model.AppError {
+func (s *OpenTracingLayerRoleStore) PermanentDeleteAll() error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.PermanentDeleteAll")
 	s.Root.Store.SetContext(newCtx)
@@ -5779,7 +5779,7 @@ func (s *OpenTracingLayerRoleStore) PermanentDeleteAll() *model.AppError {
 	return resultVar0
 }
 
-func (s *OpenTracingLayerRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
+func (s *OpenTracingLayerRoleStore) Save(role *model.Role) (*model.Role, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "RoleStore.Save")
 	s.Root.Store.SetContext(newCtx)

store/sqlstore/role_store.go

@@ -6,7 +6,6 @@ package sqlstore
 import (
 	"database/sql"
 	"fmt"
-	"net/http"
 	"strings"
 
 	sq "github.com/Masterminds/squirrel"
@@ -97,24 +96,24 @@ func newSqlRoleStore(sqlStore SqlStore) store.RoleStore {
 	return s
 }
 
-func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
+func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) {
 	// Check the role is valid before proceeding.
 	if !role.IsValidWithoutId() {
-		return nil, model.NewAppError("SqlRoleStore.Save", "store.sql_role.save.invalid_role.app_error", nil, "", http.StatusBadRequest)
+		return nil, store.NewErrInvalidInput("Role", "<any>", fmt.Sprintf("%v", role))
 	}
 
 	if len(role.Id) == 0 {
 		transaction, err := s.GetMaster().Begin()
 		if err != nil {
-			return nil, model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save.open_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "begin_transaction")
 		}
 		defer finalizeTransaction(transaction)
 		createdRole, err := s.createRole(role, transaction)
 		if err != nil {
-			transaction.Rollback()
+			_ = transaction.Rollback()
-			return nil, model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save.insert.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "unable to create Role")
 		} else if err := transaction.Commit(); err != nil {
-			return nil, model.NewAppError("SqlRoleStore.RoleSave", "store.sql_role.save_role.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "commit_transaction")
 		}
 		return createdRole, nil
 	}
@@ -122,9 +121,9 @@ func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
 	dbRole := NewRoleFromModel(role)
 	dbRole.UpdateAt = model.GetMillis()
 	if rowsChanged, err := s.GetMaster().Update(dbRole); err != nil {
-		return nil, model.NewAppError("SqlRoleStore.Save", "store.sql_role.save.update.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to update Role")
 	} else if rowsChanged != 1 {
-		return nil, model.NewAppError("SqlRoleStore.Save", "store.sql_role.save.update.app_error", nil, "no record to update", http.StatusInternalServerError)
+		return nil, fmt.Errorf("invalid number of updated rows, expected 1 but got %d", rowsChanged)
 	}
 
 	return dbRole.ToModel(), nil
@@ -149,27 +148,24 @@ func (s *SqlRoleStore) createRole(role *model.Role, transaction *gorp.Transactio
 	return dbRole.ToModel(), nil
 }
 
-func (s *SqlRoleStore) Get(roleId string) (*model.Role, *model.AppError) {
+func (s *SqlRoleStore) Get(roleId string) (*model.Role, error) {
 	var dbRole Role
 
 	if err := s.GetReplica().SelectOne(&dbRole, "SELECT * from Roles WHERE Id = :Id", map[string]interface{}{"Id": roleId}); err != nil {
 		if err == sql.ErrNoRows {
-			return nil, model.NewAppError("SqlRoleStore.Get", "store.sql_role.get.app_error", nil, "Id="+roleId+", "+err.Error(), http.StatusNotFound)
+			return nil, store.NewErrNotFound("Role", roleId)
 		}
-		return nil, model.NewAppError("SqlRoleStore.Get", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to get Role")
 	}
 
 	return dbRole.ToModel(), nil
 }
 
-func (s *SqlRoleStore) GetAll() ([]*model.Role, *model.AppError) {
+func (s *SqlRoleStore) GetAll() ([]*model.Role, error) {
 	var dbRoles []Role
 
 	if _, err := s.GetReplica().Select(&dbRoles, "SELECT * from Roles", map[string]interface{}{}); err != nil {
-		if err == sql.ErrNoRows {
+		return nil, errors.Wrap(err, "failed to find Roles")
-			return nil, model.NewAppError("SqlRoleStore.GetAll", "store.sql_role.get_all.app_error", nil, err.Error(), http.StatusNotFound)
-		}
-		return nil, model.NewAppError("SqlRoleStore.GetAll", "store.sql_role.get_all.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	var roles []*model.Role
@@ -179,40 +175,36 @@ func (s *SqlRoleStore) GetAll() ([]*model.Role, *model.AppError) {
 	return roles, nil
 }
 
-func (s *SqlRoleStore) GetByName(name string) (*model.Role, *model.AppError) {
+func (s *SqlRoleStore) GetByName(name string) (*model.Role, error) {
 	var dbRole Role
 
 	if err := s.GetReplica().SelectOne(&dbRole, "SELECT * from Roles WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil {
 		if err == sql.ErrNoRows {
-			return nil, model.NewAppError("SqlRoleStore.GetByName", "store.sql_role.get_by_name.app_error", nil, "name="+name+",err="+err.Error(), http.StatusNotFound)
+			return nil, store.NewErrNotFound("Role", fmt.Sprintf("name=%s", name))
 		}
-		return nil, model.NewAppError("SqlRoleStore.GetByName", "store.sql_role.get_by_name.app_error", nil, "name="+name+",err="+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to find Roles with name=%s", name)
 	}
 
 	return dbRole.ToModel(), nil
 }
 
-func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
+func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, error) {
 	if len(names) == 0 {
 		return []*model.Role{}, nil
 	}
 
-	failure := func(e error) ([]*model.Role, *model.AppError) {
-		return nil, model.NewAppError("SqlRoleStore.GetByNames", "store.sql_role.get_by_names.app_error", nil, e.Error(), http.StatusInternalServerError)
-	}
-
 	query := s.getQueryBuilder().
 		Select("Id, Name, DisplayName, Description, CreateAt, UpdateAt, DeleteAt, Permissions, SchemeManaged, BuiltIn").
 		From("Roles").
 		Where(sq.Eq{"Name": names})
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return failure(err)
+		return nil, errors.Wrap(err, "role_tosql")
 	}
 
 	rows, err := s.GetReplica().Db.Query(queryString, args...)
 	if err != nil {
-		return failure(err)
+		return nil, errors.Wrap(err, "failed to find Roles")
 	}
 
 	var roles []*model.Role
@@ -224,26 +216,25 @@ func (s *SqlRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppErro
 			&role.CreateAt, &role.UpdateAt, &role.DeleteAt, &role.Permissions,
 			&role.SchemeManaged, &role.BuiltIn)
 		if err != nil {
-			return failure(err)
+			return nil, errors.Wrap(err, "failed to scan values")
 		}
 		roles = append(roles, role.ToModel())
 	}
-	err = rows.Err()
+	if err = rows.Err(); err != nil {
-	if err != nil {
+		return nil, errors.Wrap(err, "unable to iterate over rows")
-		return failure(err)
 	}
 
 	return roles, nil
 }
 
-func (s *SqlRoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
+func (s *SqlRoleStore) Delete(roleId string) (*model.Role, error) {
 	// Get the role.
 	var role *Role
 	if err := s.GetReplica().SelectOne(&role, "SELECT * from Roles WHERE Id = :Id", map[string]interface{}{"Id": roleId}); err != nil {
 		if err == sql.ErrNoRows {
-			return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.get.app_error", nil, "Id="+roleId+", "+err.Error(), http.StatusNotFound)
+			return nil, store.NewErrNotFound("Role", roleId)
 		}
-		return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to get Role with id=%s", roleId)
 	}
 
 	time := model.GetMillis()
@@ -251,16 +242,16 @@ func (s *SqlRoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
 	role.UpdateAt = time
 
 	if rowsChanged, err := s.GetMaster().Update(role); err != nil {
-		return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.delete.update.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to update Role")
 	} else if rowsChanged != 1 {
-		return nil, model.NewAppError("SqlRoleStore.Delete", "store.sql_role.delete.update.app_error", nil, "no record to update", http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "invalid number of updated rows, expected 1 but got %d", rowsChanged)
 	}
 	return role.ToModel(), nil
 }
 
-func (s *SqlRoleStore) PermanentDeleteAll() *model.AppError {
+func (s *SqlRoleStore) PermanentDeleteAll() error {
 	if _, err := s.GetMaster().Exec("DELETE FROM Roles"); err != nil {
-		return model.NewAppError("SqlRoleStore.PermanentDeleteAll", "store.sql_role.permanent_delete_all.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrap(err, "failed to delete Roles")
 	}
 
 	return nil
@@ -340,12 +331,12 @@ func (s *SqlRoleStore) channelHigherScopedPermissionsQuery(roleNames []string) s
 	)
 }
 
-func (s *SqlRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) {
+func (s *SqlRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) {
-	sql := s.channelHigherScopedPermissionsQuery(roleNames)
+	query := s.channelHigherScopedPermissionsQuery(roleNames)
 
 	var rolesPermissions []*channelRolesPermissions
-	if _, err := s.GetReplica().Select(&rolesPermissions, sql); err != nil {
+	if _, err := s.GetReplica().Select(&rolesPermissions, query); err != nil {
-		return nil, model.NewAppError("SqlRoleStore.HigherScopedPermissions", "store.sql_role.get_by_names.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to find RolePermissions")
 	}
 
 	roleNameHigherScopedPermissions := map[string]*model.RolePermissions{}
@@ -359,7 +350,7 @@ func (s *SqlRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[s
 	return roleNameHigherScopedPermissions, nil
 }
 
-func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) {
+func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) {
 	query := s.getQueryBuilder().
 		Select("Roles.*").
 		From("Schemes").
@@ -370,12 +361,12 @@ func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError)
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlRoleStore.AllChannelSchemeManagedRoles", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "role_tosql")
 	}
 
 	var dbRoles []*Role
 	if _, err = s.GetReplica().Select(&dbRoles, queryString, args...); err != nil {
-		return nil, model.NewAppError("SqlRoleStore.AllChannelSchemeManagedRoles", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to find Roles")
 	}
 
 	var roles []*model.Role
@@ -387,7 +378,7 @@ func (s *SqlRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError)
 }
 
 // ChannelRolesUnderTeamRole finds all of the channel-scheme roles under the team of the given team-scheme role.
-func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) {
+func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) {
 	query := s.getQueryBuilder().
 		Select("ChannelSchemeRoles.*").
 		From("Roles AS HigherScopedRoles").
@@ -407,12 +398,12 @@ func (s *SqlRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlRoleStore.ChannelRolesUnderTeamRole", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "role_tosql")
 	}
 
 	var dbRoles []*Role
 	if _, err = s.GetReplica().Select(&dbRoles, queryString, args...); err != nil {
-		return nil, model.NewAppError("SqlRoleStore.ChannelRolesUnderTeamRole", "store.sql_role.get.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "failed to find Roles")
 	}
 
 	var roles []*model.Role

store/store.go

@@ -603,24 +603,24 @@ type PluginStore interface {
 }
 
 type RoleStore interface {
-	Save(role *model.Role) (*model.Role, *model.AppError)
+	Save(role *model.Role) (*model.Role, error)
-	Get(roleId string) (*model.Role, *model.AppError)
+	Get(roleId string) (*model.Role, error)
-	GetAll() ([]*model.Role, *model.AppError)
+	GetAll() ([]*model.Role, error)
-	GetByName(name string) (*model.Role, *model.AppError)
+	GetByName(name string) (*model.Role, error)
-	GetByNames(names []string) ([]*model.Role, *model.AppError)
+	GetByNames(names []string) ([]*model.Role, error)
-	Delete(roleId string) (*model.Role, *model.AppError)
+	Delete(roleId string) (*model.Role, error)
-	PermanentDeleteAll() *model.AppError
+	PermanentDeleteAll() error
 
 	// HigherScopedPermissions retrieves the higher-scoped permissions of a list of role names. The higher-scope
 	// (either team scheme or system scheme) is determined based on whether the team has a scheme or not.
-	ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError)
+	ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error)
 
 	// AllChannelSchemeRoles returns all of the roles associated to channel schemes.
-	AllChannelSchemeRoles() ([]*model.Role, *model.AppError)
+	AllChannelSchemeRoles() ([]*model.Role, error)
 
 	// ChannelRolesUnderTeamRole returns all of the non-deleted roles that are affected by updates to the
 	// given role.
-	ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError)
+	ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error)
 }
 
 type SchemeStore interface {

store/storetest/mocks/RoleStore.go

@@ -15,7 +15,7 @@ type RoleStore struct {
 }
 
 // AllChannelSchemeRoles provides a mock function with given fields:
-func (_m *RoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) {
+func (_m *RoleStore) AllChannelSchemeRoles() ([]*model.Role, error) {
 	ret := _m.Called()
 
 	var r0 []*model.Role
@@ -27,20 +27,18 @@ func (_m *RoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func() *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func() error); ok {
 		r1 = rf()
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // ChannelHigherScopedPermissions provides a mock function with given fields: roleNames
-func (_m *RoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) {
+func (_m *RoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) {
 	ret := _m.Called(roleNames)
 
 	var r0 map[string]*model.RolePermissions
@@ -52,20 +50,18 @@ func (_m *RoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[str
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func([]string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func([]string) error); ok {
 		r1 = rf(roleNames)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // ChannelRolesUnderTeamRole provides a mock function with given fields: roleName
-func (_m *RoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) {
+func (_m *RoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) {
 	ret := _m.Called(roleName)
 
 	var r0 []*model.Role
@@ -77,20 +73,18 @@ func (_m *RoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role,
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(roleName)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // Delete provides a mock function with given fields: roleId
-func (_m *RoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
+func (_m *RoleStore) Delete(roleId string) (*model.Role, error) {
 	ret := _m.Called(roleId)
 
 	var r0 *model.Role
@@ -102,20 +96,18 @@ func (_m *RoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(roleId)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // Get provides a mock function with given fields: roleId
-func (_m *RoleStore) Get(roleId string) (*model.Role, *model.AppError) {
+func (_m *RoleStore) Get(roleId string) (*model.Role, error) {
 	ret := _m.Called(roleId)
 
 	var r0 *model.Role
@@ -127,20 +119,18 @@ func (_m *RoleStore) Get(roleId string) (*model.Role, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(roleId)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetAll provides a mock function with given fields:
-func (_m *RoleStore) GetAll() ([]*model.Role, *model.AppError) {
+func (_m *RoleStore) GetAll() ([]*model.Role, error) {
 	ret := _m.Called()
 
 	var r0 []*model.Role
@@ -152,20 +142,18 @@ func (_m *RoleStore) GetAll() ([]*model.Role, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func() *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func() error); ok {
 		r1 = rf()
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetByName provides a mock function with given fields: name
-func (_m *RoleStore) GetByName(name string) (*model.Role, *model.AppError) {
+func (_m *RoleStore) GetByName(name string) (*model.Role, error) {
 	ret := _m.Called(name)
 
 	var r0 *model.Role
@@ -177,20 +165,18 @@ func (_m *RoleStore) GetByName(name string) (*model.Role, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(string) error); ok {
 		r1 = rf(name)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // GetByNames provides a mock function with given fields: names
-func (_m *RoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
+func (_m *RoleStore) GetByNames(names []string) ([]*model.Role, error) {
 	ret := _m.Called(names)
 
 	var r0 []*model.Role
@@ -202,36 +188,32 @@ func (_m *RoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError)
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func([]string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func([]string) error); ok {
 		r1 = rf(names)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // PermanentDeleteAll provides a mock function with given fields:
-func (_m *RoleStore) PermanentDeleteAll() *model.AppError {
+func (_m *RoleStore) PermanentDeleteAll() error {
 	ret := _m.Called()
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func() *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func() error); ok {
 		r0 = rf()
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
 }
 
 // Save provides a mock function with given fields: role
-func (_m *RoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
+func (_m *RoleStore) Save(role *model.Role) (*model.Role, error) {
 	ret := _m.Called(role)
 
 	var r0 *model.Role
@@ -243,13 +225,11 @@ func (_m *RoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
 		}
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(*model.Role) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(*model.Role) error); ok {
 		r1 = rf(role)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1

store/storetest/scheme_store.go

@@ -537,7 +537,7 @@ func testCountWithoutPermission(t *testing.T, ss store.Store) {
 	}
 
 	getRoles := func(scheme *model.Scheme) (channelUser, channelGuest *model.Role) {
-		var err *model.AppError
+		var err error
 		channelUser, err = ss.Role().GetByName(scheme.DefaultChannelUserRole)
 		require.Nil(t, err)
 		require.NotNil(t, channelUser)

store/timerlayer/timerlayer.go

@@ -5085,7 +5085,7 @@ func (s *TimerLayerReactionStore) Save(reaction *model.Reaction) (*model.Reactio
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.AllChannelSchemeRoles()
@@ -5101,7 +5101,7 @@ func (s *TimerLayerRoleStore) AllChannelSchemeRoles() ([]*model.Role, *model.App
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, *model.AppError) {
+func (s *TimerLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string) (map[string]*model.RolePermissions, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.ChannelHigherScopedPermissions(roleNames)
@@ -5117,7 +5117,7 @@ func (s *TimerLayerRoleStore) ChannelHigherScopedPermissions(roleNames []string)
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.ChannelRolesUnderTeamRole(roleName)
@@ -5133,7 +5133,7 @@ func (s *TimerLayerRoleStore) ChannelRolesUnderTeamRole(roleName string) ([]*mod
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) Delete(roleId string) (*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) Delete(roleId string) (*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.Delete(roleId)
@@ -5149,7 +5149,7 @@ func (s *TimerLayerRoleStore) Delete(roleId string) (*model.Role, *model.AppErro
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) Get(roleId string) (*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) Get(roleId string) (*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.Get(roleId)
@@ -5165,7 +5165,7 @@ func (s *TimerLayerRoleStore) Get(roleId string) (*model.Role, *model.AppError)
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) GetAll() ([]*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.GetAll()
@@ -5181,7 +5181,7 @@ func (s *TimerLayerRoleStore) GetAll() ([]*model.Role, *model.AppError) {
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) GetByName(name string) (*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) GetByName(name string) (*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.GetByName(name)
@@ -5197,7 +5197,7 @@ func (s *TimerLayerRoleStore) GetByName(name string) (*model.Role, *model.AppErr
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) GetByNames(names []string) ([]*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) GetByNames(names []string) ([]*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.GetByNames(names)
@@ -5213,7 +5213,7 @@ func (s *TimerLayerRoleStore) GetByNames(names []string) ([]*model.Role, *model.
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerRoleStore) PermanentDeleteAll() *model.AppError {
+func (s *TimerLayerRoleStore) PermanentDeleteAll() error {
 	start := timemodule.Now()
 
 	resultVar0 := s.RoleStore.PermanentDeleteAll()
@@ -5229,7 +5229,7 @@ func (s *TimerLayerRoleStore) PermanentDeleteAll() *model.AppError {
 	return resultVar0
 }
 
-func (s *TimerLayerRoleStore) Save(role *model.Role) (*model.Role, *model.AppError) {
+func (s *TimerLayerRoleStore) Save(role *model.Role) (*model.Role, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.RoleStore.Save(role)