Pluginstore migration (#14971)

Automatic Merge

app/plugin_key_value_store.go

@@ -6,10 +6,12 @@ package app
 import (
 	"crypto/sha256"
 	"encoding/base64"
+	"errors"
 	"net/http"
 
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/store"
 )
 
 func getKeyHash(key string) string {
@@ -47,7 +49,13 @@ func (a *App) SetPluginKeyWithOptions(pluginId string, key string, value []byte,
 	updated, err := a.Srv().Store.Plugin().SetWithOptions(pluginId, key, value, options)
 	if err != nil {
 		mlog.Error("Failed to set plugin key value with options", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return updated, err
+		var appErr *model.AppError
+		switch {
+		case errors.As(err, &appErr):
+			return false, appErr
+		default:
+			return false, model.NewAppError("SetPluginKeyWithOptions", "app.plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
 	}
 
 	// Clean up a previous entry using the hashed key, if it exists.
@@ -67,7 +75,13 @@ func (a *App) CompareAndDeletePluginKey(pluginId string, key string, oldValue []
 	deleted, err := a.Srv().Store.Plugin().CompareAndDelete(kv, oldValue)
 	if err != nil {
 		mlog.Error("Failed to compare and delete plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return deleted, err
+		var appErr *model.AppError
+		switch {
+		case errors.As(err, &appErr):
+			return deleted, appErr
+		default:
+			return false, model.NewAppError("CompareAndDeletePluginKey", "app.plugin_store.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
+		}
 	}
 
 	// Clean up a previous entry using the hashed key, if it exists.
@@ -81,17 +95,17 @@ func (a *App) CompareAndDeletePluginKey(pluginId string, key string, oldValue []
 func (a *App) GetPluginKey(pluginId string, key string) ([]byte, *model.AppError) {
 	if kv, err := a.Srv().Store.Plugin().Get(pluginId, key); err == nil {
 		return kv.Value, nil
-	} else if err.StatusCode != http.StatusNotFound {
+	} else if nfErr := new(store.ErrNotFound); !errors.As(err, &nfErr) {
 		mlog.Error("Failed to query plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return nil, err
+		return nil, model.NewAppError("GetPluginKey", "app.plugin_store.get.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	// Lookup using the hashed version of the key for keys written prior to v5.6.
 	if kv, err := a.Srv().Store.Plugin().Get(pluginId, getKeyHash(key)); err == nil {
 		return kv.Value, nil
-	} else if err.StatusCode != http.StatusNotFound {
+	} else if nfErr := new(store.ErrNotFound); !errors.As(err, &nfErr) {
 		mlog.Error("Failed to query plugin key value using hashed key", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return nil, err
+		return nil, model.NewAppError("GetPluginKey", "app.plugin_store.get.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	return nil, nil
@@ -100,13 +114,13 @@ func (a *App) GetPluginKey(pluginId string, key string) ([]byte, *model.AppError
 func (a *App) DeletePluginKey(pluginId string, key string) *model.AppError {
 	if err := a.Srv().Store.Plugin().Delete(pluginId, getKeyHash(key)); err != nil {
 		mlog.Error("Failed to delete plugin key value", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return err
+		return model.NewAppError("DeletePluginKey", "app.plugin_store.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	// Also delete the key without hashing
 	if err := a.Srv().Store.Plugin().Delete(pluginId, key); err != nil {
 		mlog.Error("Failed to delete plugin key value using hashed key", mlog.String("plugin_id", pluginId), mlog.String("key", key), mlog.Err(err))
-		return err
+		return model.NewAppError("DeletePluginKey", "app.plugin_store.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	return nil
@@ -115,7 +129,7 @@ func (a *App) DeletePluginKey(pluginId string, key string) *model.AppError {
 func (a *App) DeleteAllKeysForPlugin(pluginId string) *model.AppError {
 	if err := a.Srv().Store.Plugin().DeleteAllForPlugin(pluginId); err != nil {
 		mlog.Error("Failed to delete all plugin key values", mlog.String("plugin_id", pluginId), mlog.Err(err))
-		return err
+		return model.NewAppError("DeleteAllKeysForPlugin", "app.plugin_store.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	return nil
@@ -128,7 +142,7 @@ func (a *App) DeleteAllExpiredPluginKeys() *model.AppError {
 
 	if err := a.Srv().Store.Plugin().DeleteAllExpired(); err != nil {
 		mlog.Error("Failed to delete all expired plugin key values", mlog.Err(err))
-		return err
+		return model.NewAppError("DeleteAllExpiredPluginKeys", "app.plugin_store.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	return nil
@@ -139,7 +153,7 @@ func (a *App) ListPluginKeys(pluginId string, page, perPage int) ([]string, *mod
 
 	if err != nil {
 		mlog.Error("Failed to list plugin key values", mlog.Int("page", page), mlog.Int("perPage", perPage), mlog.Err(err))
-		return nil, err
+		return nil, model.NewAppError("ListPluginKeys", "app.plugin_store.list.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
 	return data, nil

app/plugin_test.go

@@ -75,8 +75,8 @@ func TestPluginKeyValueStore(t *testing.T) {
 		ExpireAt: 0,
 	}
 
-	_, err = th.App.Srv().Store.Plugin().SaveOrUpdate(kv)
-	assert.Nil(t, err)
+	_, nErr := th.App.Srv().Store.Plugin().SaveOrUpdate(kv)
+	assert.Nil(t, nErr)
 
 	// Test fetch by keyname (this key does not exist but hashed key will be used for lookup)
 	ret, err = th.App.GetPluginKey(pluginId, "key2")

i18n/en.json

@@ -4270,6 +4270,22 @@
     "id": "app.plugin.write_file.saving.app_error",
     "translation": "An error occurred while saving the file."
   },
+  {
+    "id": "app.plugin_store.delete.app_error",
+    "translation": "Could not delete plugin key value."
+  },
+  {
+    "id": "app.plugin_store.get.app_error",
+    "translation": "Could not get plugin key value."
+  },
+  {
+    "id": "app.plugin_store.list.app_error",
+    "translation": "Unable to list all the plugin keys."
+  },
+  {
+    "id": "app.plugin_store.save.app_error",
+    "translation": "Could not save or update plugin key value."
+  },
   {
     "id": "app.post.delete.app_error",
     "translation": "Unable to delete the post."
@@ -7202,30 +7218,6 @@
     "id": "store.sql_job.update.app_error",
     "translation": "Unable to update the job."
   },
-  {
-    "id": "store.sql_plugin_store.compare_and_set.mysql_select.app_error",
-    "translation": "Failed to query for existing row on MySQL after KVCompareAndSet with unchanged value."
-  },
-  {
-    "id": "store.sql_plugin_store.compare_and_set.too_many_rows.app_error",
-    "translation": "Found more than 1 row on MySQL after KVCompareAndSet with unchanged value."
-  },
-  {
-    "id": "store.sql_plugin_store.delete.app_error",
-    "translation": "Could not delete plugin key value."
-  },
-  {
-    "id": "store.sql_plugin_store.get.app_error",
-    "translation": "Could not get plugin key value."
-  },
-  {
-    "id": "store.sql_plugin_store.list.app_error",
-    "translation": "Unable to list all the plugin keys."
-  },
-  {
-    "id": "store.sql_plugin_store.save.app_error",
-    "translation": "Could not save or update plugin key value."
-  },
   {
     "id": "store.sql_post.analytics_posts_count.app_error",
     "translation": "Unable to get post counts."

store/layer_generators/main.go

@@ -247,7 +247,7 @@ func generateLayer(name, templateFile string) ([]byte, error) {
 		"errorToBoolean": func(results []string) string {
 			for _, typeName := range results {
 				if isError(typeName) {
-					return fmt.Sprintf("err == nil")
+					return "err == nil"
 				}
 			}
 			return "true"

store/opentracinglayer/opentracinglayer.go

@@ -4513,7 +4513,7 @@ func (s *OpenTracingLayerOAuthStore) UpdateApp(app *model.OAuthApp) (*model.OAut
 	return result, err
 }
 
-func (s *OpenTracingLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (s *OpenTracingLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.CompareAndDelete")
 	s.Root.Store.SetContext(newCtx)
@@ -4531,7 +4531,7 @@ func (s *OpenTracingLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyVa
 	return result, err
 }
 
-func (s *OpenTracingLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (s *OpenTracingLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.CompareAndSet")
 	s.Root.Store.SetContext(newCtx)
@@ -4549,7 +4549,7 @@ func (s *OpenTracingLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue
 	return result, err
 }
 
-func (s *OpenTracingLayerPluginStore) Delete(pluginId string, key string) *model.AppError {
+func (s *OpenTracingLayerPluginStore) Delete(pluginId string, key string) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.Delete")
 	s.Root.Store.SetContext(newCtx)
@@ -4567,7 +4567,7 @@ func (s *OpenTracingLayerPluginStore) Delete(pluginId string, key string) *model
 	return err
 }
 
-func (s *OpenTracingLayerPluginStore) DeleteAllExpired() *model.AppError {
+func (s *OpenTracingLayerPluginStore) DeleteAllExpired() error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.DeleteAllExpired")
 	s.Root.Store.SetContext(newCtx)
@@ -4585,7 +4585,7 @@ func (s *OpenTracingLayerPluginStore) DeleteAllExpired() *model.AppError {
 	return err
 }
 
-func (s *OpenTracingLayerPluginStore) DeleteAllForPlugin(PluginId string) *model.AppError {
+func (s *OpenTracingLayerPluginStore) DeleteAllForPlugin(PluginId string) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.DeleteAllForPlugin")
 	s.Root.Store.SetContext(newCtx)
@@ -4603,7 +4603,7 @@ func (s *OpenTracingLayerPluginStore) DeleteAllForPlugin(PluginId string) *model
 	return err
 }
 
-func (s *OpenTracingLayerPluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, *model.AppError) {
+func (s *OpenTracingLayerPluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.Get")
 	s.Root.Store.SetContext(newCtx)
@@ -4621,7 +4621,7 @@ func (s *OpenTracingLayerPluginStore) Get(pluginId string, key string) (*model.P
 	return result, err
 }
 
-func (s *OpenTracingLayerPluginStore) List(pluginId string, page int, perPage int) ([]string, *model.AppError) {
+func (s *OpenTracingLayerPluginStore) List(pluginId string, page int, perPage int) ([]string, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.List")
 	s.Root.Store.SetContext(newCtx)
@@ -4639,7 +4639,7 @@ func (s *OpenTracingLayerPluginStore) List(pluginId string, page int, perPage in
 	return result, err
 }
 
-func (s *OpenTracingLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+func (s *OpenTracingLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.SaveOrUpdate")
 	s.Root.Store.SetContext(newCtx)
@@ -4657,7 +4657,7 @@ func (s *OpenTracingLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue)
 	return result, err
 }
 
-func (s *OpenTracingLayerPluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, *model.AppError) {
+func (s *OpenTracingLayerPluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PluginStore.SetWithOptions")
 	s.Root.Store.SetContext(newCtx)

store/retrylayer/retrylayer.go

@@ -3024,57 +3024,183 @@ func (s *RetryLayerOAuthStore) UpdateApp(app *model.OAuthApp) (*model.OAuthApp,
 
 }
 
-func (s *RetryLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (s *RetryLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 
-	return s.PluginStore.CompareAndDelete(keyVal, oldValue)
+	tries := 0
+	for {
+		result, err := s.PluginStore.CompareAndDelete(keyVal, oldValue)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (s *RetryLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 
-	return s.PluginStore.CompareAndSet(keyVal, oldValue)
+	tries := 0
+	for {
+		result, err := s.PluginStore.CompareAndSet(keyVal, oldValue)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) Delete(pluginId string, key string) *model.AppError {
+func (s *RetryLayerPluginStore) Delete(pluginId string, key string) error {
 
-	return s.PluginStore.Delete(pluginId, key)
+	tries := 0
+	for {
+		err := s.PluginStore.Delete(pluginId, key)
+		if err == nil {
+			return err
+		}
+		if !isRepeatableError(err) {
+			return err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) DeleteAllExpired() *model.AppError {
+func (s *RetryLayerPluginStore) DeleteAllExpired() error {
 
-	return s.PluginStore.DeleteAllExpired()
+	tries := 0
+	for {
+		err := s.PluginStore.DeleteAllExpired()
+		if err == nil {
+			return err
+		}
+		if !isRepeatableError(err) {
+			return err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) DeleteAllForPlugin(PluginId string) *model.AppError {
+func (s *RetryLayerPluginStore) DeleteAllForPlugin(PluginId string) error {
 
-	return s.PluginStore.DeleteAllForPlugin(PluginId)
+	tries := 0
+	for {
+		err := s.PluginStore.DeleteAllForPlugin(PluginId)
+		if err == nil {
+			return err
+		}
+		if !isRepeatableError(err) {
+			return err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, *model.AppError) {
+func (s *RetryLayerPluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, error) {
 
-	return s.PluginStore.Get(pluginId, key)
+	tries := 0
+	for {
+		result, err := s.PluginStore.Get(pluginId, key)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) List(pluginId string, page int, perPage int) ([]string, *model.AppError) {
+func (s *RetryLayerPluginStore) List(pluginId string, page int, perPage int) ([]string, error) {
 
-	return s.PluginStore.List(pluginId, page, perPage)
+	tries := 0
+	for {
+		result, err := s.PluginStore.List(pluginId, page, perPage)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+func (s *RetryLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 
-	return s.PluginStore.SaveOrUpdate(keyVal)
+	tries := 0
+	for {
+		result, err := s.PluginStore.SaveOrUpdate(keyVal)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 
-func (s *RetryLayerPluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, *model.AppError) {
+func (s *RetryLayerPluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, error) {
 
-	return s.PluginStore.SetWithOptions(pluginId, key, value, options)
+	tries := 0
+	for {
+		result, err := s.PluginStore.SetWithOptions(pluginId, key, value, options)
+		if err == nil {
+			return result, err
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+	}
 
 }
 

store/sqlstore/plugin_store.go

@@ -7,9 +7,9 @@ import (
 	"bytes"
 	"database/sql"
 	"fmt"
-	"net/http"
 
 	sq "github.com/Masterminds/squirrel"
+	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/store"
@@ -39,7 +39,7 @@ func newSqlPluginStore(sqlStore SqlStore) store.PluginStore {
 func (ps SqlPluginStore) createIndexesIfNotExists() {
 }
 
-func (ps SqlPluginStore) SaveOrUpdate(kv *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+func (ps SqlPluginStore) SaveOrUpdate(kv *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 	if err := kv.IsValid(); err != nil {
 		return nil, err
 	}
@@ -58,11 +58,11 @@ func (ps SqlPluginStore) SaveOrUpdate(kv *model.PluginKeyValue) (*model.PluginKe
 		// Unfortunately PostgreSQL pre-9.5 does not have an atomic upsert, so we use
 		// separate update and insert queries to accomplish our upsert
 		if rowsAffected, err := ps.GetMaster().Update(kv); err != nil {
-			return nil, model.NewAppError("SqlPluginStore.SaveOrUpdate", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "failed to update PluginKeyValue")
 		} else if rowsAffected == 0 {
 			// No rows were affected by the update, so let's try an insert
 			if err := ps.GetMaster().Insert(kv); err != nil {
-				return nil, model.NewAppError("SqlPluginStore.SaveOrUpdate", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusBadRequest)
+				return nil, errors.Wrap(err, "failed to save PluginKeyValue")
 			}
 		}
 	} else if ps.DriverName() == model.DATABASE_DRIVER_MYSQL {
@@ -74,18 +74,18 @@ func (ps SqlPluginStore) SaveOrUpdate(kv *model.PluginKeyValue) (*model.PluginKe
 
 		queryString, args, err := query.ToSql()
 		if err != nil {
-			return nil, model.NewAppError("SqlPluginStore.SaveOrUpdate", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "plugin_tosql")
 		}
 
 		if _, err := ps.GetMaster().Exec(queryString, args...); err != nil {
-			return nil, model.NewAppError("SqlPluginStore.SaveOrUpdate", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "failed to upsert PluginKeyValue")
 		}
 	}
 
 	return kv, nil
 }
 
-func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	if err := kv.IsValid(); err != nil {
 		return false, err
 	}
@@ -106,11 +106,11 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
 
 		queryString, args, err := query.ToSql()
 		if err != nil {
-			return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return false, errors.Wrap(err, "plugin_tosql")
 		}
 
 		if _, err := ps.GetMaster().Exec(queryString, args...); err != nil {
-			return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.delete.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return false, errors.Wrap(err, "failed to delete PluginKeyValue")
 		}
 
 		// Insert if oldValue is nil
@@ -121,7 +121,7 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
 			if IsUniqueConstraintError(err, []string{"PRIMARY", "PluginId", "Key", "PKey", "pkey"}) {
 				return false, nil
 			} else {
-				return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+				return false, errors.Wrap(err, "failed to insert PluginKeyValue")
 			}
 		}
 	} else {
@@ -142,17 +142,17 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
 
 		queryString, args, err := query.ToSql()
 		if err != nil {
-			return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return false, errors.Wrap(err, "plugin_tosql")
 		}
 
 		updateResult, err := ps.GetMaster().Exec(queryString, args...)
 		if err != nil {
-			return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return false, errors.Wrap(err, "failed to update PluginKeyValue")
 		}
 
 		if rowsAffected, err := updateResult.RowsAffected(); err != nil {
 			// Failed to update
-			return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return false, errors.Wrap(err, "unable to get rows affected")
 		} else if rowsAffected == 0 {
 			if ps.DriverName() == model.DATABASE_DRIVER_MYSQL && bytes.Equal(oldValue, kv.Value) {
 				// ROW_COUNT on MySQL is zero even if the row existed but no changes to the row were required.
@@ -173,12 +173,12 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
 
 				queryString, args, err := query.ToSql()
 				if err != nil {
-					return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql.build_query.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, err=%v", kv.PluginId, kv.Key, err.Error()), http.StatusInternalServerError)
+					return false, errors.Wrap(err, "plugin_tosql")
 				}
 
 				count, err := ps.GetReplica().SelectInt(queryString, args...)
 				if err != nil {
-					return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.compare_and_set.mysql_select.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, err=%v", kv.PluginId, kv.Key, err.Error()), http.StatusInternalServerError)
+					return false, errors.Wrapf(err, "failed to count PluginKeyValue with pluginId=%s and key=%s", kv.PluginId, kv.Key)
 				}
 
 				if count == 0 {
@@ -186,7 +186,7 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
 				} else if count == 1 {
 					return true, nil
 				} else {
-					return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.compare_and_set.too_many_rows.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, count=%d", kv.PluginId, kv.Key, count), http.StatusInternalServerError)
+					return false, errors.Wrapf(err, "got too many rows when counting PluginKeyValue with pluginId=%s, key=%s, rows=%d", kv.PluginId, kv.Key, count)
 				}
 			}
 
@@ -199,7 +199,7 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
 	return true, nil
 }
 
-func (ps SqlPluginStore) CompareAndDelete(kv *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (ps SqlPluginStore) CompareAndDelete(kv *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	if err := kv.IsValid(); err != nil {
 		return false, err
 	}
@@ -221,16 +221,16 @@ func (ps SqlPluginStore) CompareAndDelete(kv *model.PluginKeyValue, oldValue []b
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return false, model.NewAppError("SqlPluginStore.CompareAndDelete", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "plugin_tosql")
 	}
 
 	deleteResult, err := ps.GetMaster().Exec(queryString, args...)
 	if err != nil {
-		return false, model.NewAppError("SqlPluginStore.CompareAndDelete", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "failed to delete PluginKeyValue")
 	}
 
 	if rowsAffected, err := deleteResult.RowsAffected(); err != nil {
-		return false, model.NewAppError("SqlPluginStore.CompareAndDelete", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrap(err, "unable to get rows affected")
 	} else if rowsAffected == 0 {
 		return false, nil
 	}
@@ -238,7 +238,7 @@ func (ps SqlPluginStore) CompareAndDelete(kv *model.PluginKeyValue, oldValue []b
 	return true, nil
 }
 
-func (ps SqlPluginStore) SetWithOptions(pluginId string, key string, value []byte, opt model.PluginKVSetOptions) (bool, *model.AppError) {
+func (ps SqlPluginStore) SetWithOptions(pluginId string, key string, value []byte, opt model.PluginKVSetOptions) (bool, error) {
 	if err := opt.IsValid(); err != nil {
 		return false, err
 	}
@@ -252,27 +252,16 @@ func (ps SqlPluginStore) SetWithOptions(pluginId string, key string, value []byt
 		return ps.CompareAndSet(kv, opt.OldValue)
 	}
 
-	savedKv, err := ps.SaveOrUpdate(kv)
-	if err != nil {
-		return false, err
+	savedKv, nErr := ps.SaveOrUpdate(kv)
+	if nErr != nil {
+		return false, nErr
 	}
 
 	return savedKv != nil, nil
 }
 
-func (ps SqlPluginStore) Get(pluginId, key string) (*model.PluginKeyValue, *model.AppError) {
+func (ps SqlPluginStore) Get(pluginId, key string) (*model.PluginKeyValue, error) {
 	currentTime := model.GetMillis()
-
-	failure := func(err error, statusCode int) *model.AppError {
-		return model.NewAppError(
-			"SqlPluginStore.Get",
-			"store.sql_plugin_store.get.app_error",
-			nil,
-			fmt.Sprintf("plugin_id=%v, key=%v, err=%v", pluginId, key, err.Error()),
-			statusCode,
-		)
-	}
-
 	query := ps.getQueryBuilder().Select("PluginId, PKey, PValue, ExpireAt").
 		From("PluginKeyValueStore").
 		Where(sq.Eq{"PluginId": pluginId}).
@@ -280,21 +269,22 @@ func (ps SqlPluginStore) Get(pluginId, key string) (*model.PluginKeyValue, *mode
 		Where(sq.Or{sq.Eq{"ExpireAt": 0}, sq.Gt{"ExpireAt": currentTime}})
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return nil, failure(err, http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "plugin_tosql")
 	}
+
 	row := ps.GetReplica().Db.QueryRow(queryString, args...)
 	var kv model.PluginKeyValue
 	if err := row.Scan(&kv.PluginId, &kv.Key, &kv.Value, &kv.ExpireAt); err != nil {
 		if err == sql.ErrNoRows {
-			return nil, failure(err, http.StatusNotFound)
+			return nil, store.NewErrNotFound("PluginKeyValue", fmt.Sprintf("pluginId=%s, key=%s", pluginId, key))
 		}
-		return nil, failure(err, http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to get PluginKeyValue with pluginId=%s and key=%s", pluginId, key)
 	}
 
 	return &kv, nil
 }
 
-func (ps SqlPluginStore) Delete(pluginId, key string) *model.AppError {
+func (ps SqlPluginStore) Delete(pluginId, key string) error {
 	query := ps.getQueryBuilder().
 		Delete("PluginKeyValueStore").
 		Where(sq.Eq{"PluginId": pluginId}).
@@ -302,34 +292,33 @@ func (ps SqlPluginStore) Delete(pluginId, key string) *model.AppError {
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return model.NewAppError("SqlPluginStore.Delete", "store.sql.build_query.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, err=%v", pluginId, key, err.Error()), http.StatusInternalServerError)
+		return errors.Wrap(err, "plugin_tosql")
 	}
 
 	if _, err := ps.GetMaster().Exec(queryString, args...); err != nil {
-		return model.NewAppError("SqlPluginStore.Delete", "store.sql_plugin_store.delete.app_error", nil, fmt.Sprintf("plugin_id=%v, key=%v, err=%v", pluginId, key, err.Error()), http.StatusInternalServerError)
+		return errors.Wrapf(err, "failed to delete PluginKeyValue with pluginId=%s and key=%s", pluginId, key)
 	}
 	return nil
 }
 
-func (ps SqlPluginStore) DeleteAllForPlugin(pluginId string) *model.AppError {
+func (ps SqlPluginStore) DeleteAllForPlugin(pluginId string) error {
 	query := ps.getQueryBuilder().
 		Delete("PluginKeyValueStore").
 		Where(sq.Eq{"PluginId": pluginId})
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return model.NewAppError("SqlPluginStore.Delete", "store.sql.build_query.app_error", nil, fmt.Sprintf("plugin_id=%v, err=%v", pluginId, err.Error()), http.StatusInternalServerError)
+		return errors.Wrap(err, "plugin_tosql")
 	}
 
 	if _, err := ps.GetMaster().Exec(queryString, args...); err != nil {
-		return model.NewAppError("SqlPluginStore.Delete", "store.sql_plugin_store.delete.app_error", nil, fmt.Sprintf("plugin_id=%v, err=%v", pluginId, err.Error()), http.StatusInternalServerError)
+		return errors.Wrapf(err, "failed to get all PluginKeyValues with pluginId=%s ", pluginId)
 	}
 	return nil
 }
 
-func (ps SqlPluginStore) DeleteAllExpired() *model.AppError {
+func (ps SqlPluginStore) DeleteAllExpired() error {
 	currentTime := model.GetMillis()
-
 	query := ps.getQueryBuilder().
 		Delete("PluginKeyValueStore").
 		Where(sq.NotEq{"ExpireAt": 0}).
@@ -337,16 +326,16 @@ func (ps SqlPluginStore) DeleteAllExpired() *model.AppError {
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return model.NewAppError("SqlPluginStore.Delete", "store.sql.build_query.app_error", nil, fmt.Sprintf("current_time=%v, err=%v", currentTime, err.Error()), http.StatusInternalServerError)
+		return errors.Wrap(err, "plugin_tosql")
 	}
 
 	if _, err := ps.GetMaster().Exec(queryString, args...); err != nil {
-		return model.NewAppError("SqlPluginStore.Delete", "store.sql_plugin_store.delete.app_error", nil, fmt.Sprintf("current_time=%v, err=%v", currentTime, err.Error()), http.StatusInternalServerError)
+		return errors.Wrap(err, "failed to delete all expired PluginKeyValues")
 	}
 	return nil
 }
 
-func (ps SqlPluginStore) List(pluginId string, offset int, limit int) ([]string, *model.AppError) {
+func (ps SqlPluginStore) List(pluginId string, offset int, limit int) ([]string, error) {
 	if limit <= 0 {
 		limit = defaultPluginKeyFetchLimit
 	}
@@ -371,12 +360,12 @@ func (ps SqlPluginStore) List(pluginId string, offset int, limit int) ([]string,
 
 	queryString, args, err := query.ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlPluginStore.List", "store.sql.build_query.app_error", nil, fmt.Sprintf("plugin_id=%v, err=%v", pluginId, err.Error()), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "plugin_tosql")
 	}
 
 	_, err = ps.GetReplica().Select(&keys, queryString, args...)
 	if err != nil {
-		return nil, model.NewAppError("SqlPluginStore.List", "store.sql_plugin_store.list.app_error", nil, fmt.Sprintf("plugin_id=%v, err=%v", pluginId, err.Error()), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "failed to get PluginKeyValues with pluginId=%s", pluginId)
 	}
 
 	return keys, nil

store/store.go

@@ -587,15 +587,15 @@ type UserAccessTokenStore interface {
 }
 
 type PluginStore interface {
-	SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError)
-	CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError)
-	CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError)
-	SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, *model.AppError)
-	Get(pluginId, key string) (*model.PluginKeyValue, *model.AppError)
-	Delete(pluginId, key string) *model.AppError
-	DeleteAllForPlugin(PluginId string) *model.AppError
-	DeleteAllExpired() *model.AppError
-	List(pluginId string, page, perPage int) ([]string, *model.AppError)
+	SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, error)
+	CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error)
+	CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error)
+	SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, error)
+	Get(pluginId, key string) (*model.PluginKeyValue, error)
+	Delete(pluginId, key string) error
+	DeleteAllForPlugin(PluginId string) error
+	DeleteAllExpired() error
+	List(pluginId string, page, perPage int) ([]string, error)
 }
 
 type RoleStore interface {

store/storetest/mocks/PluginStore.go

@@ -15,7 +15,7 @@ type PluginStore struct {
 }
 
 // CompareAndDelete provides a mock function with given fields: keyVal, oldValue
-func (_m *PluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (_m *PluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	ret := _m.Called(keyVal, oldValue)
 
 	var r0 bool
@@ -25,20 +25,18 @@ func (_m *PluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue [
 		r0 = ret.Get(0).(bool)
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(*model.PluginKeyValue, []byte) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*model.PluginKeyValue, []byte) error); ok {
 		r1 = rf(keyVal, oldValue)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1
 }
 
 // CompareAndSet provides a mock function with given fields: keyVal, oldValue
-func (_m *PluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (_m *PluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	ret := _m.Called(keyVal, oldValue)
 
 	var r0 bool
@@ -48,68 +46,60 @@ func (_m *PluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []by
 		r0 = ret.Get(0).(bool)
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(*model.PluginKeyValue, []byte) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*model.PluginKeyValue, []byte) error); ok {
 		r1 = rf(keyVal, oldValue)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1
 }
 
 // Delete provides a mock function with given fields: pluginId, key
-func (_m *PluginStore) Delete(pluginId string, key string) *model.AppError {
+func (_m *PluginStore) Delete(pluginId string, key string) error {
 	ret := _m.Called(pluginId, key)
 
-	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok {
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, string) error); ok {
 		r0 = rf(pluginId, key)
 	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.AppError)
-		}
+		r0 = ret.Error(0)
 	}
 
 	return r0
 }
 
 // DeleteAllExpired provides a mock function with given fields:
-func (_m *PluginStore) DeleteAllExpired() *model.AppError {
+func (_m *PluginStore) DeleteAllExpired() error {
 	ret := _m.Called()
 
-	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func() *model.AppError); ok {
+	var r0 error
+	if rf, ok := ret.Get(0).(func() error); ok {
 		r0 = rf()
 	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.AppError)
-		}
+		r0 = ret.Error(0)
 	}
 
 	return r0
 }
 
 // DeleteAllForPlugin provides a mock function with given fields: PluginId
-func (_m *PluginStore) DeleteAllForPlugin(PluginId string) *model.AppError {
+func (_m *PluginStore) DeleteAllForPlugin(PluginId string) error {
 	ret := _m.Called(PluginId)
 
-	var r0 *model.AppError
-	if rf, ok := ret.Get(0).(func(string) *model.AppError); ok {
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string) error); ok {
 		r0 = rf(PluginId)
 	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.AppError)
-		}
+		r0 = ret.Error(0)
 	}
 
 	return r0
 }
 
 // Get provides a mock function with given fields: pluginId, key
-func (_m *PluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, *model.AppError) {
+func (_m *PluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, error) {
 	ret := _m.Called(pluginId, key)
 
 	var r0 *model.PluginKeyValue
@@ -121,20 +111,18 @@ func (_m *PluginStore) Get(pluginId string, key string) (*model.PluginKeyValue,
 		}
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, string) error); ok {
 		r1 = rf(pluginId, key)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1
 }
 
 // List provides a mock function with given fields: pluginId, page, perPage
-func (_m *PluginStore) List(pluginId string, page int, perPage int) ([]string, *model.AppError) {
+func (_m *PluginStore) List(pluginId string, page int, perPage int) ([]string, error) {
 	ret := _m.Called(pluginId, page, perPage)
 
 	var r0 []string
@@ -146,20 +134,18 @@ func (_m *PluginStore) List(pluginId string, page int, perPage int) ([]string, *
 		}
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, int, int) error); ok {
 		r1 = rf(pluginId, page, perPage)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1
 }
 
 // SaveOrUpdate provides a mock function with given fields: keyVal
-func (_m *PluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+func (_m *PluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 	ret := _m.Called(keyVal)
 
 	var r0 *model.PluginKeyValue
@@ -171,20 +157,18 @@ func (_m *PluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.Plugin
 		}
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(*model.PluginKeyValue) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*model.PluginKeyValue) error); ok {
 		r1 = rf(keyVal)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1
 }
 
 // SetWithOptions provides a mock function with given fields: pluginId, key, value, options
-func (_m *PluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, *model.AppError) {
+func (_m *PluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, error) {
 	ret := _m.Called(pluginId, key, value, options)
 
 	var r0 bool
@@ -194,13 +178,11 @@ func (_m *PluginStore) SetWithOptions(pluginId string, key string, value []byte,
 		r0 = ret.Get(0).(bool)
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(string, string, []byte, model.PluginKVSetOptions) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, string, []byte, model.PluginKVSetOptions) error); ok {
 		r1 = rf(pluginId, key, value, options)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1

store/storetest/plugin_store.go

@@ -4,7 +4,6 @@
 package storetest
 
 import (
-	"net/http"
 	"sort"
 	"testing"
 
@@ -64,7 +63,7 @@ func setupKVs(t *testing.T, ss store.Store) (string, func()) {
 	}
 }
 
-func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer func(kv *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError)) {
+func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer func(kv *model.PluginKeyValue) (*model.PluginKeyValue, error)) {
 	t.Run("invalid kv", func(t *testing.T) {
 		_, tearDown := setupKVs(t, ss)
 		defer tearDown()
@@ -78,7 +77,9 @@ func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer
 
 		kv, err := doer(kv)
 		require.NotNil(t, err)
-		require.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", err.Id)
+		appErr, ok := err.(*model.AppError)
+		require.True(t, ok)
+		require.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", appErr.Id)
 		assert.Nil(t, kv)
 	})
 
@@ -107,8 +108,8 @@ func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer
 		assert.Equal(t, []byte(value), kv.Value)
 		assert.Equal(t, expireAt, kv.ExpireAt)
 
-		actualKV, err := ss.Plugin().Get(pluginId, key)
-		require.Nil(t, err)
+		actualKV, nErr := ss.Plugin().Get(pluginId, key)
+		require.Nil(t, nErr)
 		assert.Equal(t, kv, actualKV)
 	})
 
@@ -137,9 +138,10 @@ func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer
 		assert.Nil(t, kv.Value)
 		assert.Equal(t, expireAt, kv.ExpireAt)
 
-		actualKV, err := ss.Plugin().Get(pluginId, key)
-		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		actualKV, nErr := ss.Plugin().Get(pluginId, key)
+		_, ok := nErr.(*store.ErrNotFound)
+		require.NotNil(t, nErr)
+		assert.True(t, ok)
 		assert.Nil(t, actualKV)
 	})
 
@@ -174,8 +176,8 @@ func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer
 		assert.Equal(t, []byte(newValue), kv.Value)
 		assert.Equal(t, expireAt, kv.ExpireAt)
 
-		actualKV, err := ss.Plugin().Get(pluginId, key)
-		require.Nil(t, err)
+		actualKV, nErr := ss.Plugin().Get(pluginId, key)
+		require.Nil(t, nErr)
 		assert.Equal(t, kv, actualKV)
 	})
 
@@ -209,22 +211,23 @@ func doTestPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier, doer
 		assert.Nil(t, kv.Value)
 		assert.Equal(t, expireAt, kv.ExpireAt)
 
-		actualKV, err := ss.Plugin().Get(pluginId, key)
-		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		actualKV, nErr := ss.Plugin().Get(pluginId, key)
+		_, ok := nErr.(*store.ErrNotFound)
+		require.NotNil(t, nErr)
+		assert.True(t, ok)
 		assert.Nil(t, actualKV)
 	})
 }
 
 func testPluginSaveOrUpdate(t *testing.T, ss store.Store, s SqlSupplier) {
-	doTestPluginSaveOrUpdate(t, ss, s, func(kv *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+	doTestPluginSaveOrUpdate(t, ss, s, func(kv *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 		return ss.Plugin().SaveOrUpdate(kv)
 	})
 }
 
 // doTestPluginCompareAndSet exercises the CompareAndSet functionality, but abstracts the actual
 // call to same to allow reuse with SetWithOptions
-func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, compareAndSet func(kv *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError)) {
+func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, compareAndSet func(kv *model.PluginKeyValue, oldValue []byte) (bool, error)) {
 	t.Run("invalid kv", func(t *testing.T) {
 		_, tearDown := setupKVs(t, ss)
 		defer tearDown()
@@ -238,8 +241,10 @@ func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, comp
 
 		ok, err := compareAndSet(kv, nil)
 		require.NotNil(t, err)
-		assert.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", err.Id)
 		assert.False(t, ok)
+		appErr, ok := err.(*model.AppError)
+		require.True(t, ok)
+		assert.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", appErr.Id)
 	})
 
 	// assertChanged verifies that CompareAndSet successfully changes to the given value.
@@ -250,8 +255,8 @@ func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, comp
 		require.Nil(t, err)
 		require.True(t, ok, "should have succeeded to CompareAndSet")
 
-		actualKV, err := ss.Plugin().Get(kv.PluginId, kv.Key)
-		require.Nil(t, err)
+		actualKV, nErr := ss.Plugin().Get(kv.PluginId, kv.Key)
+		require.Nil(t, nErr)
 
 		// When tested with KVSetWithOptions, a strict comparison can fail because that
 		// function accepts a relative time and makes its own call to model.GetMillis(),
@@ -275,13 +280,14 @@ func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, comp
 		require.Nil(t, err)
 		require.False(t, ok, "should have failed to CompareAndSet")
 
-		actualKV, err := ss.Plugin().Get(kv.PluginId, kv.Key)
+		actualKV, nErr := ss.Plugin().Get(kv.PluginId, kv.Key)
 		if existingKV == nil {
-			require.NotNil(t, err)
-			assert.Equal(t, err.StatusCode, http.StatusNotFound)
+			require.NotNil(t, nErr)
+			_, ok := nErr.(*store.ErrNotFound)
+			assert.True(t, ok)
 			assert.Nil(t, actualKV)
 		} else {
-			require.Nil(t, err)
+			require.Nil(t, nErr)
 			assert.Equal(t, existingKV, actualKV)
 		}
 	}
@@ -294,9 +300,10 @@ func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, comp
 		require.Nil(t, err)
 		require.True(t, ok, "should have succeeded to CompareAndSet")
 
-		actualKV, err := ss.Plugin().Get(kv.PluginId, kv.Key)
-		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		actualKV, nErr := ss.Plugin().Get(kv.PluginId, kv.Key)
+		_, ok = nErr.(*store.ErrNotFound)
+		require.NotNil(t, nErr)
+		assert.True(t, ok)
 		assert.Nil(t, actualKV)
 	}
 
@@ -518,7 +525,7 @@ func doTestPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier, comp
 }
 
 func testPluginCompareAndSet(t *testing.T, ss store.Store, s SqlSupplier) {
-	doTestPluginCompareAndSet(t, ss, s, func(kv *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+	doTestPluginCompareAndSet(t, ss, s, func(kv *model.PluginKeyValue, oldValue []byte) (bool, error) {
 		return ss.Plugin().CompareAndSet(kv, oldValue)
 	})
 }
@@ -537,8 +544,10 @@ func testPluginCompareAndDelete(t *testing.T, ss store.Store, s SqlSupplier) {
 
 		ok, err := ss.Plugin().CompareAndDelete(kv, nil)
 		require.NotNil(t, err)
-		assert.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", err.Id)
 		assert.False(t, ok)
+		appErr, ok := err.(*model.AppError)
+		require.True(t, ok)
+		assert.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", appErr.Id)
 	})
 
 	t.Run("non-existent key should fail", func(t *testing.T) {
@@ -666,8 +675,10 @@ func testPluginSetWithOptions(t *testing.T, ss store.Store, s SqlSupplier) {
 
 		ok, err := ss.Plugin().SetWithOptions(pluginId, key, []byte(value), options)
 		require.NotNil(t, err)
-		require.Equal(t, "model.plugin_kvset_options.is_valid.old_value.app_error", err.Id)
 		assert.False(t, ok)
+		appErr, ok := err.(*model.AppError)
+		require.True(t, ok)
+		require.Equal(t, "model.plugin_kvset_options.is_valid.old_value.app_error", appErr.Id)
 	})
 
 	t.Run("invalid kv", func(t *testing.T) {
@@ -681,12 +692,14 @@ func testPluginSetWithOptions(t *testing.T, ss store.Store, s SqlSupplier) {
 
 		ok, err := ss.Plugin().SetWithOptions(pluginId, key, []byte(value), options)
 		require.NotNil(t, err)
-		require.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", err.Id)
 		assert.False(t, ok)
+		appErr, ok := err.(*model.AppError)
+		require.True(t, ok)
+		require.Equal(t, "model.plugin_key_value.is_valid.plugin_id.app_error", appErr.Id)
 	})
 
 	t.Run("atomic", func(t *testing.T) {
-		doTestPluginCompareAndSet(t, ss, s, func(kv *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+		doTestPluginCompareAndSet(t, ss, s, func(kv *model.PluginKeyValue, oldValue []byte) (bool, error) {
 			now := model.GetMillis()
 			options := model.PluginKVSetOptions{
 				Atomic:   true,
@@ -702,7 +715,7 @@ func testPluginSetWithOptions(t *testing.T, ss store.Store, s SqlSupplier) {
 	})
 
 	t.Run("non-atomic", func(t *testing.T) {
-		doTestPluginSaveOrUpdate(t, ss, s, func(kv *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+		doTestPluginSaveOrUpdate(t, ss, s, func(kv *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 			now := model.GetMillis()
 			options := model.PluginKVSetOptions{
 				Atomic: false,
@@ -712,11 +725,11 @@ func testPluginSetWithOptions(t *testing.T, ss store.Store, s SqlSupplier) {
 				options.ExpireInSeconds = (kv.ExpireAt - now) / 1000
 			}
 
-			ok, appErr := ss.Plugin().SetWithOptions(kv.PluginId, kv.Key, kv.Value, options)
+			ok, err := ss.Plugin().SetWithOptions(kv.PluginId, kv.Key, kv.Value, options)
 			if !ok {
-				return nil, appErr
+				return nil, err
 			} else {
-				return kv, appErr
+				return kv, err
 			}
 		})
 	})
@@ -727,9 +740,10 @@ func testPluginGet(t *testing.T, ss store.Store) {
 		pluginId := model.NewId()
 		key := model.NewId()
 
-		kv, err := ss.Plugin().Get(pluginId, key)
-		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		kv, nErr := ss.Plugin().Get(pluginId, key)
+		_, ok := nErr.(*store.ErrNotFound)
+		require.NotNil(t, nErr)
+		assert.True(t, ok)
 		assert.Nil(t, kv)
 	})
 
@@ -750,8 +764,9 @@ func testPluginGet(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 
 		kv, err = ss.Plugin().Get(model.NewId(), key)
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, kv)
 	})
 
@@ -772,8 +787,9 @@ func testPluginGet(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 
 		kv, err = ss.Plugin().Get(pluginId, model.NewId())
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, kv)
 	})
 
@@ -794,8 +810,9 @@ func testPluginGet(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 
 		kv, err = ss.Plugin().Get(pluginId, model.NewId())
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, kv)
 	})
 
@@ -816,8 +833,9 @@ func testPluginGet(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 
 		kv, err = ss.Plugin().Get(pluginId, model.NewId())
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, kv)
 	})
 
@@ -875,8 +893,9 @@ func testPluginDelete(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 
 		kv, err := ss.Plugin().Get(pluginId, key)
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, kv)
 	})
 
@@ -921,8 +940,9 @@ func testPluginDelete(t *testing.T, ss store.Store) {
 			require.Nil(t, err)
 
 			kv, err = ss.Plugin().Get(pluginId, key)
+			_, ok := err.(*store.ErrNotFound)
 			require.NotNil(t, err)
-			assert.Equal(t, err.StatusCode, http.StatusNotFound)
+			assert.True(t, ok)
 			assert.Nil(t, kv)
 		})
 	}
@@ -985,12 +1005,14 @@ func testPluginDeleteAllForPlugin(t *testing.T, ss store.Store) {
 		require.Nil(t, err)
 
 		_, err = ss.Plugin().Get(kv.PluginId, kv.Key)
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 
 		_, err = ss.Plugin().Get(kv.PluginId, kv2.Key)
+		_, ok = err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 	})
 }
 
@@ -1168,8 +1190,9 @@ func testPluginDeleteAllExpired(t *testing.T, ss store.Store) {
 		assert.Equal(t, kvA1, actualKVA1)
 
 		actualKVA2, err := ss.Plugin().Get(pluginIdA, expiredKVA2.Key)
+		_, ok := err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, actualKVA2)
 
 		actualKVB1, err := ss.Plugin().Get(pluginIdB, kvB1.Key)
@@ -1177,8 +1200,9 @@ func testPluginDeleteAllExpired(t *testing.T, ss store.Store) {
 		assert.Equal(t, kvB1, actualKVB1)
 
 		actualKVB2, err := ss.Plugin().Get(pluginIdB, expiredKVB2.Key)
+		_, ok = err.(*store.ErrNotFound)
 		require.NotNil(t, err)
-		assert.Equal(t, err.StatusCode, http.StatusNotFound)
+		assert.True(t, ok)
 		assert.Nil(t, actualKVB2)
 	})
 }

store/timerlayer/timerlayer.go

@@ -4095,7 +4095,7 @@ func (s *TimerLayerOAuthStore) UpdateApp(app *model.OAuthApp) (*model.OAuthApp,
 	return result, err
 }
 
-func (s *TimerLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (s *TimerLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	start := timemodule.Now()
 
 	result, err := s.PluginStore.CompareAndDelete(keyVal, oldValue)
@@ -4111,7 +4111,7 @@ func (s *TimerLayerPluginStore) CompareAndDelete(keyVal *model.PluginKeyValue, o
 	return result, err
 }
 
-func (s *TimerLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, *model.AppError) {
+func (s *TimerLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldValue []byte) (bool, error) {
 	start := timemodule.Now()
 
 	result, err := s.PluginStore.CompareAndSet(keyVal, oldValue)
@@ -4127,7 +4127,7 @@ func (s *TimerLayerPluginStore) CompareAndSet(keyVal *model.PluginKeyValue, oldV
 	return result, err
 }
 
-func (s *TimerLayerPluginStore) Delete(pluginId string, key string) *model.AppError {
+func (s *TimerLayerPluginStore) Delete(pluginId string, key string) error {
 	start := timemodule.Now()
 
 	err := s.PluginStore.Delete(pluginId, key)
@@ -4143,7 +4143,7 @@ func (s *TimerLayerPluginStore) Delete(pluginId string, key string) *model.AppEr
 	return err
 }
 
-func (s *TimerLayerPluginStore) DeleteAllExpired() *model.AppError {
+func (s *TimerLayerPluginStore) DeleteAllExpired() error {
 	start := timemodule.Now()
 
 	err := s.PluginStore.DeleteAllExpired()
@@ -4159,7 +4159,7 @@ func (s *TimerLayerPluginStore) DeleteAllExpired() *model.AppError {
 	return err
 }
 
-func (s *TimerLayerPluginStore) DeleteAllForPlugin(PluginId string) *model.AppError {
+func (s *TimerLayerPluginStore) DeleteAllForPlugin(PluginId string) error {
 	start := timemodule.Now()
 
 	err := s.PluginStore.DeleteAllForPlugin(PluginId)
@@ -4175,7 +4175,7 @@ func (s *TimerLayerPluginStore) DeleteAllForPlugin(PluginId string) *model.AppEr
 	return err
 }
 
-func (s *TimerLayerPluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, *model.AppError) {
+func (s *TimerLayerPluginStore) Get(pluginId string, key string) (*model.PluginKeyValue, error) {
 	start := timemodule.Now()
 
 	result, err := s.PluginStore.Get(pluginId, key)
@@ -4191,7 +4191,7 @@ func (s *TimerLayerPluginStore) Get(pluginId string, key string) (*model.PluginK
 	return result, err
 }
 
-func (s *TimerLayerPluginStore) List(pluginId string, page int, perPage int) ([]string, *model.AppError) {
+func (s *TimerLayerPluginStore) List(pluginId string, page int, perPage int) ([]string, error) {
 	start := timemodule.Now()
 
 	result, err := s.PluginStore.List(pluginId, page, perPage)
@@ -4207,7 +4207,7 @@ func (s *TimerLayerPluginStore) List(pluginId string, page int, perPage int) ([]
 	return result, err
 }
 
-func (s *TimerLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, *model.AppError) {
+func (s *TimerLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*model.PluginKeyValue, error) {
 	start := timemodule.Now()
 
 	result, err := s.PluginStore.SaveOrUpdate(keyVal)
@@ -4223,7 +4223,7 @@ func (s *TimerLayerPluginStore) SaveOrUpdate(keyVal *model.PluginKeyValue) (*mod
 	return result, err
 }
 
-func (s *TimerLayerPluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, *model.AppError) {
+func (s *TimerLayerPluginStore) SetWithOptions(pluginId string, key string, value []byte, options model.PluginKVSetOptions) (bool, error) {
 	start := timemodule.Now()
 
 	result, err := s.PluginStore.SetWithOptions(pluginId, key, value, options)