Remove usages of AppError on filesstore service (#15841)

* Remove usages of AppError on filesstore service

* Fixing a golint error

* Fixing shadowed variable

* Adding err.Error() to the NewAppError calls

* Fixing tests

* Adding missed translations

* Fix error handling and updating the translation that affects it

* Fixing two typos

api4/system.go

@@ -19,7 +19,6 @@ import (
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/services/cache"
-	"github.com/mattermost/mattermost-server/v5/services/filesstore"
 	"github.com/mattermost/mattermost-server/v5/services/upgrader"
 )
 
@@ -117,16 +116,8 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) {
 
 		filestoreStatusKey := "filestore_status"
 		s[filestoreStatusKey] = model.STATUS_OK
-		license := c.App.Srv().License()
+		appErr := c.App.TestFilesStoreConnection()
-		backend, appErr := filesstore.NewFileBackend(&c.App.Config().FileSettings, license != nil && *license.Features.Compliance)
+		if appErr != nil {
-		if appErr == nil {
-			appErr = backend.TestConnection()
-			if appErr != nil {
-				s[filestoreStatusKey] = model.STATUS_UNHEALTHY
-				s[model.STATUS] = model.STATUS_UNHEALTHY
-			}
-		} else {
-			mlog.Debug("Unable to get filestore for ping status.", mlog.Err(appErr))
 			s[filestoreStatusKey] = model.STATUS_UNHEALTHY
 			s[model.STATUS] = model.STATUS_UNHEALTHY
 		}
@@ -393,7 +384,7 @@ func testS3(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	err := filesstore.CheckMandatoryS3Fields(&cfg.FileSettings)
+	err := c.App.CheckMandatoryS3Fields(&cfg.FileSettings)
 	if err != nil {
 		c.Err = err
 		return
@@ -403,11 +394,7 @@ func testS3(c *Context, w http.ResponseWriter, r *http.Request) {
 		cfg.FileSettings.AmazonS3SecretAccessKey = c.App.Config().FileSettings.AmazonS3SecretAccessKey
 	}
 
-	license := c.App.Srv().License()
+	appErr := c.App.TestFilesStoreConnectionWithConfig(&cfg.FileSettings)
-	backend, appErr := filesstore.NewFileBackend(&cfg.FileSettings, license != nil && *license.Features.Compliance)
-	if appErr == nil {
-		appErr = backend.TestConnection()
-	}
 	if appErr != nil {
 		c.Err = appErr
 		return

api4/system_test.go

@@ -478,7 +478,7 @@ func TestS3TestConnection(t *testing.T) {
 		config.FileSettings.AmazonS3Bucket = model.NewString("Wrong_bucket")
 		_, resp = th.SystemAdminClient.TestS3Connection(&config)
 		CheckInternalErrorStatus(t, resp)
-		assert.Equal(t, "Unable to create bucket.", resp.Error.Message)
+		assert.Equal(t, "api.file.test_connection.app_error", resp.Error.Id)
 
 		*config.FileSettings.AmazonS3Bucket = "shouldcreatenewbucket"
 		_, resp = th.SystemAdminClient.TestS3Connection(&config)

app/app_iface.go

@@ -393,6 +393,7 @@ type AppIface interface {
 	ChannelMembersToRemove(teamID *string) ([]*model.ChannelMember, *model.AppError)
 	CheckAndSendUserLimitWarningEmails() *model.AppError
 	CheckForClientSideCert(r *http.Request) (string, string, string)
+	CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError
 	CheckPasswordAndAllCriteria(user *model.User, password string, mfaToken string) *model.AppError
 	CheckRolesExist(roleNames []string) *model.AppError
 	CheckUserAllAuthenticationCriteria(user *model.User, mfaToken string) *model.AppError
@@ -834,6 +835,7 @@ type AppIface interface {
 	ReloadConfig() error
 	RemoveAllDeactivatedMembersFromChannel(channel *model.Channel) *model.AppError
 	RemoveConfigListener(id string)
+	RemoveDirectory(path string) *model.AppError
 	RemoveFile(path string) *model.AppError
 	RemoveLdapPrivateCertificate() *model.AppError
 	RemoveLdapPublicCertificate() *model.AppError
@@ -957,6 +959,8 @@ type AppIface interface {
 	TelemetryId() string
 	TestElasticsearch(cfg *model.Config) *model.AppError
 	TestEmail(userId string, cfg *model.Config) *model.AppError
+	TestFilesStoreConnection() *model.AppError
+	TestFilesStoreConnectionWithConfig(cfg *model.FileSettings) *model.AppError
 	TestLdap() *model.AppError
 	TestSiteURL(siteURL string) *model.AppError
 	Timezones() *timezones.Timezones

app/file.go

@@ -76,12 +76,49 @@ func (a *App) FileBackend() (filesstore.FileBackend, *model.AppError) {
 	return a.Srv().FileBackend()
 }
 
+func (a *App) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError {
+	err := filesstore.CheckMandatoryS3Fields(settings)
+	if err != nil {
+		return model.NewAppError("CheckMandatoryS3Fields", "api.admin.test_s3.missing_s3_bucket", nil, err.Error(), http.StatusBadRequest)
+	}
+	return nil
+}
+
+func (a *App) TestFilesStoreConnection() *model.AppError {
+	backend, err := a.FileBackend()
+	if err != nil {
+		return err
+	}
+	nErr := backend.TestConnection()
+	if nErr != nil {
+		return model.NewAppError("TestConnection", "api.file.test_connection.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return nil
+}
+
+func (a *App) TestFilesStoreConnectionWithConfig(cfg *model.FileSettings) *model.AppError {
+	license := a.Srv().License()
+	backend, err := filesstore.NewFileBackend(cfg, license != nil && *license.Features.Compliance)
+	if err != nil {
+		return model.NewAppError("FileBackend", "api.file.no_driver.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+	nErr := backend.TestConnection()
+	if nErr != nil {
+		return model.NewAppError("TestConnection", "api.file.test_connection.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return nil
+}
+
 func (a *App) ReadFile(path string) ([]byte, *model.AppError) {
 	backend, err := a.FileBackend()
 	if err != nil {
 		return nil, err
 	}
-	return backend.ReadFile(path)
+	result, nErr := backend.ReadFile(path)
+	if nErr != nil {
+		return nil, model.NewAppError("ReadFile", "api.file.read_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return result, nil
 }
 
 // Caller must close the first return value
@@ -90,7 +127,11 @@ func (a *App) FileReader(path string) (filesstore.ReadCloseSeeker, *model.AppErr
 	if err != nil {
 		return nil, err
 	}
-	return backend.Reader(path)
+	result, nErr := backend.Reader(path)
+	if nErr != nil {
+		return nil, model.NewAppError("FileReader", "api.file.file_reader.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return result, nil
 }
 
 func (a *App) FileExists(path string) (bool, *model.AppError) {
@@ -98,7 +139,11 @@ func (a *App) FileExists(path string) (bool, *model.AppError) {
 	if err != nil {
 		return false, err
 	}
-	return backend.FileExists(path)
+	result, nErr := backend.FileExists(path)
+	if nErr != nil {
+		return false, model.NewAppError("FileExists", "api.file.file_exists.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return result, nil
 }
 
 func (a *App) FileSize(path string) (int64, *model.AppError) {
@@ -106,7 +151,11 @@ func (a *App) FileSize(path string) (int64, *model.AppError) {
 	if err != nil {
 		return 0, err
 	}
-	return backend.FileSize(path)
+	size, nErr := backend.FileSize(path)
+	if nErr != nil {
+		return 0, model.NewAppError("FileSize", "api.file.file_size.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return size, nil
 }
 
 func (a *App) MoveFile(oldPath, newPath string) *model.AppError {
@@ -114,7 +163,11 @@ func (a *App) MoveFile(oldPath, newPath string) *model.AppError {
 	if err != nil {
 		return err
 	}
-	return backend.MoveFile(oldPath, newPath)
+	nErr := backend.MoveFile(oldPath, newPath)
+	if nErr != nil {
+		return model.NewAppError("MoveFile", "api.file.move_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return nil
 }
 
 func (a *App) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
@@ -123,7 +176,11 @@ func (a *App) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
 		return 0, err
 	}
 
-	return backend.WriteFile(fr, path)
+	result, nErr := backend.WriteFile(fr, path)
+	if nErr != nil {
+		return result, model.NewAppError("WriteFile", "api.file.write_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return result, nil
 }
 
 func (a *App) AppendFile(fr io.Reader, path string) (int64, *model.AppError) {
@@ -132,7 +189,11 @@ func (a *App) AppendFile(fr io.Reader, path string) (int64, *model.AppError) {
 		return 0, err
 	}
 
-	return backend.AppendFile(fr, path)
+	result, nErr := backend.AppendFile(fr, path)
+	if nErr != nil {
+		return result, model.NewAppError("AppendFile", "api.file.append_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return result, nil
 }
 
 func (a *App) RemoveFile(path string) *model.AppError {
@@ -140,7 +201,11 @@ func (a *App) RemoveFile(path string) *model.AppError {
 	if err != nil {
 		return err
 	}
-	return backend.RemoveFile(path)
+	nErr := backend.RemoveFile(path)
+	if nErr != nil {
+		return model.NewAppError("RemoveFile", "api.file.remove_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+	return nil
 }
 
 func (a *App) ListDirectory(path string) ([]string, *model.AppError) {
@@ -148,14 +213,27 @@ func (a *App) ListDirectory(path string) ([]string, *model.AppError) {
 	if err != nil {
 		return nil, err
 	}
-	paths, err := backend.ListDirectory(path)
+	paths, nErr := backend.ListDirectory(path)
-	if err != nil {
+	if nErr != nil {
-		return nil, err
+		return nil, model.NewAppError("ListDirectory", "api.file.list_directory.app_error", nil, nErr.Error(), http.StatusInternalServerError)
 	}
 
 	return *paths, nil
 }
 
+func (a *App) RemoveDirectory(path string) *model.AppError {
+	backend, err := a.FileBackend()
+	if err != nil {
+		return err
+	}
+	nErr := backend.RemoveDirectory(path)
+	if nErr != nil {
+		return model.NewAppError("RemoveDirectory", "api.file.remove_directory.app_error", nil, nErr.Error(), http.StatusInternalServerError)
+	}
+
+	return nil
+}
+
 func (a *App) getInfoForFilename(post *model.Post, teamId, channelId, userId, oldId, filename string) *model.FileInfo {
 	name, _ := url.QueryUnescape(filename)
 	pathPrefix := fmt.Sprintf("teams/%s/channels/%s/users/%s/%s/", teamId, channelId, userId, oldId)

app/opentracing/opentracing_layer.go

@@ -1071,6 +1071,28 @@ func (a *OpenTracingAppLayer) CheckForClientSideCert(r *http.Request) (string, s
 	return resultVar0, resultVar1, resultVar2
 }
 
+func (a *OpenTracingAppLayer) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CheckMandatoryS3Fields")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.CheckMandatoryS3Fields(settings)
+
+	if resultVar0 != nil {
+		span.LogFields(spanlog.Error(resultVar0))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) CheckPasswordAndAllCriteria(user *model.User, password string, mfaToken string) *model.AppError {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CheckPasswordAndAllCriteria")
@@ -11709,6 +11731,28 @@ func (a *OpenTracingAppLayer) RemoveConfigListener(id string) {
 	a.app.RemoveConfigListener(id)
 }
 
+func (a *OpenTracingAppLayer) RemoveDirectory(path string) *model.AppError {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveDirectory")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.RemoveDirectory(path)
+
+	if resultVar0 != nil {
+		span.LogFields(spanlog.Error(resultVar0))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) RemoveFile(path string) *model.AppError {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RemoveFile")
@@ -14300,6 +14344,50 @@ func (a *OpenTracingAppLayer) TestEmail(userId string, cfg *model.Config) *model
 	return resultVar0
 }
 
+func (a *OpenTracingAppLayer) TestFilesStoreConnection() *model.AppError {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.TestFilesStoreConnection")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.TestFilesStoreConnection()
+
+	if resultVar0 != nil {
+		span.LogFields(spanlog.Error(resultVar0))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0
+}
+
+func (a *OpenTracingAppLayer) TestFilesStoreConnectionWithConfig(cfg *model.FileSettings) *model.AppError {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.TestFilesStoreConnectionWithConfig")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.TestFilesStoreConnectionWithConfig(cfg)
+
+	if resultVar0 != nil {
+		span.LogFields(spanlog.Error(resultVar0))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) TestLdap() *model.AppError {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.TestLdap")

app/server.go

@@ -465,11 +465,13 @@ func NewServer(options ...Option) (*Server, error) {
 	}
 
 	backend, appErr := s.FileBackend()
-	if appErr == nil {
-		appErr = backend.TestConnection()
-	}
 	if appErr != nil {
 		mlog.Error("Problem with file storage settings", mlog.Err(appErr))
+	} else {
+		nErr := backend.TestConnection()
+		if nErr != nil {
+			mlog.Error("Problem with file storage settings", mlog.Err(nErr))
+		}
 	}
 
 	s.timezones = timezones.New()
@@ -1531,7 +1533,11 @@ func (s *Server) stopSearchEngine() {
 
 func (s *Server) FileBackend() (filesstore.FileBackend, *model.AppError) {
 	license := s.License()
-	return filesstore.NewFileBackend(&s.Config().FileSettings, license != nil && *license.Features.Compliance)
+	backend, err := filesstore.NewFileBackend(&s.Config().FileSettings, license != nil && *license.Features.Compliance)
+	if err != nil {
+		return nil, model.NewAppError("FileBackend", "api.file.no_driver.app_error", nil, err.Error(), http.StatusInternalServerError)
+	}
+	return backend, nil
 }
 
 func (s *Server) TotalWebsocketConnections() int {

i18n/en.json

@@ -1317,39 +1317,23 @@
     "translation": "Unable to create the emoji. An error occurred when trying to open the attached image."
   },
   {
-    "id": "api.file.append_file.no_exist.app_error",
+    "id": "api.file.append_file.app_error",
-    "translation": "File does not exist."
+    "translation": "Unable to append data to the file."
-  },
-  {
-    "id": "api.file.append_file.opening.app_error",
-    "translation": "Encountered an error opening the file."
-  },
-  {
-    "id": "api.file.append_file.s3.app_error",
-    "translation": "Encountered an error appending to a file in S3."
-  },
-  {
-    "id": "api.file.append_file.writing.app_error",
-    "translation": "Encountered an error writing to S3."
   },
   {
     "id": "api.file.attachments.disabled.app_error",
     "translation": "File attachments have been disabled on this server."
   },
   {
-    "id": "api.file.file_exists.exists_local.app_error",
+    "id": "api.file.file_exists.app_error",
     "translation": "Unable to check if the file exists."
   },
   {
-    "id": "api.file.file_exists.s3.app_error",
+    "id": "api.file.file_reader.app_error",
-    "translation": "Unable to check if the file exists."
+    "translation": "Unable to get a file reader."
   },
   {
-    "id": "api.file.file_size.local.app_error",
+    "id": "api.file.file_size.app_error",
-    "translation": "Unable to get the file size."
-  },
-  {
-    "id": "api.file.file_size.s3.app_error",
     "translation": "Unable to get the file size."
   },
   {
@@ -1373,56 +1357,36 @@
     "translation": "Unable to get public link for file. File must be attached to a post that can be read by the current user."
   },
   {
-    "id": "api.file.move_file.copy_within_s3.app_error",
+    "id": "api.file.list_directory.app_error",
-    "translation": "Unable to copy file within S3."
+    "translation": "Unable to list directory."
   },
   {
-    "id": "api.file.move_file.delete_from_s3.app_error",
+    "id": "api.file.move_file.app_error",
-    "translation": "Unable to delete file from S3."
+    "translation": "Unable to move file."
-  },
-  {
-    "id": "api.file.move_file.rename.app_error",
-    "translation": "Unable to move file locally."
-  },
-  {
-    "id": "api.file.new_backend.s3.app_error",
-    "translation": "Encountered an error opening a connection to S3."
   },
   {
     "id": "api.file.no_driver.app_error",
     "translation": "No file driver selected."
   },
+  {
+    "id": "api.file.read_file.app_error",
+    "translation": "Unable to read the file."
+  },
   {
     "id": "api.file.read_file.reading_local.app_error",
     "translation": "Encountered an error reading from local server file storage."
   },
   {
-    "id": "api.file.read_file.s3.app_error",
+    "id": "api.file.remove_directory.app_error",
-    "translation": "Encountered an error reading from S3 storage."
+    "translation": "Unable to remove the directory."
   },
   {
-    "id": "api.file.reader.reading_local.app_error",
+    "id": "api.file.remove_file.app_error",
-    "translation": "Encountered an error opening a reader from local server file storage."
+    "translation": "Unable to remove the file."
   },
   {
-    "id": "api.file.reader.s3.app_error",
+    "id": "api.file.test_connection.app_error",
-    "translation": "Encountered an error opening a reader from S3 storage."
+    "translation": "Unable to access the file storage."
-  },
-  {
-    "id": "api.file.test_connection.local.connection.app_error",
-    "translation": "Don't have permissions to write to local path specified or other error."
-  },
-  {
-    "id": "api.file.test_connection.s3.bucket_create.app_error",
-    "translation": "Unable to create bucket."
-  },
-  {
-    "id": "api.file.test_connection.s3.bucket_exists.app_error",
-    "translation": "Error checking if bucket exists."
-  },
-  {
-    "id": "api.file.test_connection.s3.list_objects.app_error",
-    "translation": "Error trying to list objects."
   },
   {
     "id": "api.file.upload_file.incorrect_channelId.app_error",
@@ -1465,16 +1429,8 @@
     "translation": "Unable to upload file {{.Filename}}. {{.Length}} bytes exceeds the maximum allowed {{.Limit}} bytes."
   },
   {
-    "id": "api.file.write_file.s3.app_error",
+    "id": "api.file.write_file.app_error",
-    "translation": "Encountered an error writing to S3."
+    "translation": "Unable to write the file."
-  },
-  {
-    "id": "api.file.write_file_locally.create_dir.app_error",
-    "translation": "Encountered an error creating the directory for the new file."
-  },
-  {
-    "id": "api.file.write_file_locally.writing.app_error",
-    "translation": "Encountered an error writing to local server storage."
   },
   {
     "id": "api.image.get.app_error",
@@ -6026,6 +5982,10 @@
     "id": "ent.actiance.export.marshalToXml.appError",
     "translation": "Unable to convert export to XML."
   },
+  {
+    "id": "ent.actiance.export.write_file.appError",
+    "translation": "Unable to write the export file."
+  },
   {
     "id": "ent.api.post.send_notifications_and_forget.push_image_only",
     "translation": " attached a file."
@@ -6106,6 +6066,10 @@
     "id": "ent.compliance.csv.warning.appError",
     "translation": "Unable to create the warning file."
   },
+  {
+    "id": "ent.compliance.csv.write_file.appError",
+    "translation": "Unable to write the csv file."
+  },
   {
     "id": "ent.compliance.csv.zip.creation.appError",
     "translation": "Unable to create the zip export file."
@@ -6122,6 +6086,10 @@
     "id": "ent.compliance.global_relay.rewind_temporary_file.appError",
     "translation": "Unable to re-read the Global Relay temporary export file."
   },
+  {
+    "id": "ent.compliance.global_relay.write_file.appError",
+    "translation": "Unable to write the global relay file."
+  },
   {
     "id": "ent.compliance.licence_disable.app_error",
     "translation": "Compliance functionality disabled by current license. Please contact your system administrator about upgrading your enterprise license."
@@ -8246,30 +8214,6 @@
     "id": "system.message.name",
     "translation": "System"
   },
-  {
-    "id": "utils.file.list_directory.local.app_error",
-    "translation": "Encountered an error listing directory from local server file storage."
-  },
-  {
-    "id": "utils.file.list_directory.s3.app_error",
-    "translation": "Encountered an error listing directory from S3."
-  },
-  {
-    "id": "utils.file.remove_directory.local.app_error",
-    "translation": "Encountered an error removing directory from local server file storage."
-  },
-  {
-    "id": "utils.file.remove_directory.s3.app_error",
-    "translation": "Encountered an error removing directory from S3."
-  },
-  {
-    "id": "utils.file.remove_file.local.app_error",
-    "translation": "Encountered an error removing file from local server file storage."
-  },
-  {
-    "id": "utils.file.remove_file.s3.app_error",
-    "translation": "Encountered an error removing file from S3."
-  },
   {
     "id": "utils.mail.connect_smtp.helo.app_error",
     "translation": "Failed to set HELO."

services/filesstore/filesstore.go

@@ -5,7 +5,8 @@ package filesstore
 
 import (
 	"io"
-	"net/http"
+
+	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost-server/v5/model"
 )
@@ -16,28 +17,28 @@ type ReadCloseSeeker interface {
 }
 
 type FileBackend interface {
-	TestConnection() *model.AppError
+	TestConnection() error
 
-	Reader(path string) (ReadCloseSeeker, *model.AppError)
+	Reader(path string) (ReadCloseSeeker, error)
-	ReadFile(path string) ([]byte, *model.AppError)
+	ReadFile(path string) ([]byte, error)
-	FileExists(path string) (bool, *model.AppError)
+	FileExists(path string) (bool, error)
-	FileSize(path string) (int64, *model.AppError)
+	FileSize(path string) (int64, error)
-	CopyFile(oldPath, newPath string) *model.AppError
+	CopyFile(oldPath, newPath string) error
-	MoveFile(oldPath, newPath string) *model.AppError
+	MoveFile(oldPath, newPath string) error
-	WriteFile(fr io.Reader, path string) (int64, *model.AppError)
+	WriteFile(fr io.Reader, path string) (int64, error)
-	AppendFile(fr io.Reader, path string) (int64, *model.AppError)
+	AppendFile(fr io.Reader, path string) (int64, error)
-	RemoveFile(path string) *model.AppError
+	RemoveFile(path string) error
 
-	ListDirectory(path string) (*[]string, *model.AppError)
+	ListDirectory(path string) (*[]string, error)
-	RemoveDirectory(path string) *model.AppError
+	RemoveDirectory(path string) error
 }
 
-func NewFileBackend(settings *model.FileSettings, enableComplianceFeatures bool) (FileBackend, *model.AppError) {
+func NewFileBackend(settings *model.FileSettings, enableComplianceFeatures bool) (FileBackend, error) {
 	switch *settings.DriverName {
 	case model.IMAGE_DRIVER_S3:
 		backend, err := NewS3FileBackend(settings, enableComplianceFeatures)
 		if err != nil {
-			return nil, model.NewAppError("NewFileBackend", "api.file.new_backend.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrap(err, "unable to connect to the s3 backend")
 		}
 		return backend, nil
 	case model.IMAGE_DRIVER_LOCAL:
@@ -45,5 +46,5 @@ func NewFileBackend(settings *model.FileSettings, enableComplianceFeatures bool)
 			directory: *settings.Directory,
 		}, nil
 	}
-	return nil, model.NewAppError("NewFileBackend", "api.file.no_driver.app_error", nil, "", http.StatusInternalServerError)
+	return nil, errors.New("no valid filestorage driver found")
 }

services/filesstore/localstore.go

@@ -7,12 +7,12 @@ import (
 	"bytes"
 	"io"
 	"io/ioutil"
-	"net/http"
 	"os"
 	"path/filepath"
 
+	"github.com/pkg/errors"
+
 	"github.com/mattermost/mattermost-server/v5/mlog"
-	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/utils"
 )
 
@@ -24,33 +24,33 @@ type LocalFileBackend struct {
 	directory string
 }
 
-func (b *LocalFileBackend) TestConnection() *model.AppError {
+func (b *LocalFileBackend) TestConnection() error {
 	f := bytes.NewReader([]byte("testingwrite"))
 	if _, err := writeFileLocally(f, filepath.Join(b.directory, TEST_FILE_PATH)); err != nil {
-		return model.NewAppError("TestFileConnection", "api.file.test_connection.local.connection.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrap(err, "unable to write to the local filesystem storage")
 	}
 	os.Remove(filepath.Join(b.directory, TEST_FILE_PATH))
 	mlog.Debug("Able to write files to local storage.")
 	return nil
 }
 
-func (b *LocalFileBackend) Reader(path string) (ReadCloseSeeker, *model.AppError) {
+func (b *LocalFileBackend) Reader(path string) (ReadCloseSeeker, error) {
 	f, err := os.Open(filepath.Join(b.directory, path))
 	if err != nil {
-		return nil, model.NewAppError("Reader", "api.file.reader.reading_local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "unable to open file %s", path)
 	}
 	return f, nil
 }
 
-func (b *LocalFileBackend) ReadFile(path string) ([]byte, *model.AppError) {
+func (b *LocalFileBackend) ReadFile(path string) ([]byte, error) {
 	f, err := ioutil.ReadFile(filepath.Join(b.directory, path))
 	if err != nil {
-		return nil, model.NewAppError("ReadFile", "api.file.read_file.reading_local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "unable to read file %s", path)
 	}
 	return f, nil
 }
 
-func (b *LocalFileBackend) FileExists(path string) (bool, *model.AppError) {
+func (b *LocalFileBackend) FileExists(path string) (bool, error) {
 	_, err := os.Stat(filepath.Join(b.directory, path))
 
 	if os.IsNotExist(err) {
@@ -58,91 +58,91 @@ func (b *LocalFileBackend) FileExists(path string) (bool, *model.AppError) {
 	}
 
 	if err != nil {
-		return false, model.NewAppError("ReadFile", "api.file.file_exists.exists_local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return false, errors.Wrapf(err, "unable to know if file %s exists", path)
 	}
 	return true, nil
 }
 
-func (b *LocalFileBackend) FileSize(path string) (int64, *model.AppError) {
+func (b *LocalFileBackend) FileSize(path string) (int64, error) {
 	info, err := os.Stat(filepath.Join(b.directory, path))
 	if err != nil {
-		return 0, model.NewAppError("FileSize", "api.file.file_size.local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to get file size for %s", path)
 	}
 	return info.Size(), nil
 }
 
-func (b *LocalFileBackend) CopyFile(oldPath, newPath string) *model.AppError {
+func (b *LocalFileBackend) CopyFile(oldPath, newPath string) error {
 	if err := utils.CopyFile(filepath.Join(b.directory, oldPath), filepath.Join(b.directory, newPath)); err != nil {
-		return model.NewAppError("copyFile", "api.file.move_file.rename.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to copy file from %s to %s", oldPath, newPath)
 	}
 	return nil
 }
 
-func (b *LocalFileBackend) MoveFile(oldPath, newPath string) *model.AppError {
+func (b *LocalFileBackend) MoveFile(oldPath, newPath string) error {
 	if err := os.MkdirAll(filepath.Dir(filepath.Join(b.directory, newPath)), 0750); err != nil {
-		return model.NewAppError("moveFile", "api.file.move_file.rename.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to create the new destination directory %s", filepath.Dir(newPath))
 	}
 
 	if err := os.Rename(filepath.Join(b.directory, oldPath), filepath.Join(b.directory, newPath)); err != nil {
-		return model.NewAppError("moveFile", "api.file.move_file.rename.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to move the file to %s to the destination directory", newPath)
 	}
 
 	return nil
 }
 
-func (b *LocalFileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
+func (b *LocalFileBackend) WriteFile(fr io.Reader, path string) (int64, error) {
 	return writeFileLocally(fr, filepath.Join(b.directory, path))
 }
 
-func writeFileLocally(fr io.Reader, path string) (int64, *model.AppError) {
+func writeFileLocally(fr io.Reader, path string) (int64, error) {
 	if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil {
 		directory, _ := filepath.Abs(filepath.Dir(path))
-		return 0, model.NewAppError("WriteFile", "api.file.write_file_locally.create_dir.app_error", nil, "directory="+directory+", err="+err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to create the directory %s for the file %s", directory, path)
 	}
 	fw, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
 	if err != nil {
-		return 0, model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to open the file %s to write the data", path)
 	}
 	defer fw.Close()
 	written, err := io.Copy(fw, fr)
 	if err != nil {
-		return written, model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return written, errors.Wrapf(err, "unable write the data in the file %s", path)
 	}
 	return written, nil
 }
 
-func (b *LocalFileBackend) AppendFile(fr io.Reader, path string) (int64, *model.AppError) {
+func (b *LocalFileBackend) AppendFile(fr io.Reader, path string) (int64, error) {
 	fp := filepath.Join(b.directory, path)
 	if _, err := os.Stat(fp); err != nil {
-		return 0, model.NewAppError("AppendFile", "api.file.append_file.no_exist.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to find the file %s to append the data", path)
 	}
 	fw, err := os.OpenFile(fp, os.O_WRONLY|os.O_APPEND, 0600)
 	if err != nil {
-		return 0, model.NewAppError("AppendFile", "api.file.append_file.opening.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to open the file %s to append the data", path)
 	}
 	defer fw.Close()
 	written, err := io.Copy(fw, fr)
 	if err != nil {
-		return written, model.NewAppError("AppendFile", "api.file.append_file.writing.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return written, errors.Wrapf(err, "unable append the data in the file %s", path)
 	}
 	return written, nil
 }
 
-func (b *LocalFileBackend) RemoveFile(path string) *model.AppError {
+func (b *LocalFileBackend) RemoveFile(path string) error {
 	if err := os.Remove(filepath.Join(b.directory, path)); err != nil {
-		return model.NewAppError("RemoveFile", "utils.file.remove_file.local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to remove the file %s", path)
 	}
 	return nil
 }
 
-func (b *LocalFileBackend) ListDirectory(path string) (*[]string, *model.AppError) {
+func (b *LocalFileBackend) ListDirectory(path string) (*[]string, error) {
 	var paths []string
 	fileInfos, err := ioutil.ReadDir(filepath.Join(b.directory, path))
 	if err != nil {
 		if os.IsNotExist(err) {
 			return &paths, nil
 		}
-		return nil, model.NewAppError("ListDirectory", "utils.file.list_directory.local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "unable to list the directory %s", path)
 	}
 	for _, fileInfo := range fileInfos {
 		paths = append(paths, filepath.Join(path, fileInfo.Name()))
@@ -150,9 +150,9 @@ func (b *LocalFileBackend) ListDirectory(path string) (*[]string, *model.AppErro
 	return &paths, nil
 }
 
-func (b *LocalFileBackend) RemoveDirectory(path string) *model.AppError {
+func (b *LocalFileBackend) RemoveDirectory(path string) error {
 	if err := os.RemoveAll(filepath.Join(b.directory, path)); err != nil {
-		return model.NewAppError("RemoveDirectory", "utils.file.remove_directory.local.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to remove the directory %s", path)
 	}
 	return nil
 }

services/filesstore/mocks/FileBackend.go

@@ -10,8 +10,6 @@ import (
 	filesstore "github.com/mattermost/mattermost-server/v5/services/filesstore"
 
 	mock "github.com/stretchr/testify/mock"
-
-	model "github.com/mattermost/mattermost-server/v5/model"
 )
 
 // FileBackend is an autogenerated mock type for the FileBackend type
@@ -20,7 +18,7 @@ type FileBackend struct {
 }
 
 // AppendFile provides a mock function with given fields: fr, path
-func (_m *FileBackend) AppendFile(fr io.Reader, path string) (int64, *model.AppError) {
+func (_m *FileBackend) AppendFile(fr io.Reader, path string) (int64, error) {
 	ret := _m.Called(fr, path)
 
 	var r0 int64
@@ -30,36 +28,32 @@ func (_m *FileBackend) AppendFile(fr io.Reader, path string) (int64, *model.AppE
 		r0 = ret.Get(0).(int64)
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(io.Reader, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(io.Reader, string) error); ok {
 		r1 = rf(fr, path)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // CopyFile provides a mock function with given fields: oldPath, newPath
-func (_m *FileBackend) CopyFile(oldPath string, newPath string) *model.AppError {
+func (_m *FileBackend) CopyFile(oldPath string, newPath string) error {
 	ret := _m.Called(oldPath, newPath)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string, string) error); ok {
 		r0 = rf(oldPath, newPath)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
 }
 
 // FileExists provides a mock function with given fields: path
-func (_m *FileBackend) FileExists(path string) (bool, *model.AppError) {
+func (_m *FileBackend) FileExists(path string) (bool, error) {
 	ret := _m.Called(path)
 
 	var r0 bool
@@ -69,20 +63,18 @@ func (_m *FileBackend) FileExists(path string) (bool, *model.AppError) {
 		r0 = ret.Get(0).(bool)
 	}
 
-	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(path)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // ListDirectory provides a mock function with given fields: path
-func (_m *FileBackend) ListDirectory(path string) (*[]string, *model.AppError) {
+func (_m *FileBackend) ListDirectory(path string) (*[]string, error) {
 	ret := _m.Called(path)
 
 	var r0 *[]string
@@ -94,36 +86,32 @@ func (_m *FileBackend) ListDirectory(path string) (*[]string, *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(path)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // MoveFile provides a mock function with given fields: oldPath, newPath
-func (_m *FileBackend) MoveFile(oldPath string, newPath string) *model.AppError {
+func (_m *FileBackend) MoveFile(oldPath string, newPath string) error {
 	ret := _m.Called(oldPath, newPath)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string, string) error); ok {
 		r0 = rf(oldPath, newPath)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
 }
 
 // ReadFile provides a mock function with given fields: path
-func (_m *FileBackend) ReadFile(path string) ([]byte, *model.AppError) {
+func (_m *FileBackend) ReadFile(path string) ([]byte, error) {
 	ret := _m.Called(path)
 
 	var r0 []byte
@@ -135,20 +123,18 @@ func (_m *FileBackend) ReadFile(path string) ([]byte, *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(path)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // Reader provides a mock function with given fields: path
-func (_m *FileBackend) Reader(path string) (filesstore.ReadCloseSeeker, *model.AppError) {
+func (_m *FileBackend) Reader(path string) (filesstore.ReadCloseSeeker, error) {
 	ret := _m.Called(path)
 
 	var r0 filesstore.ReadCloseSeeker
@@ -160,68 +146,60 @@ func (_m *FileBackend) Reader(path string) (filesstore.ReadCloseSeeker, *model.A
 		}
 	}
 
-	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(path)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1
 }
 
 // RemoveDirectory provides a mock function with given fields: path
-func (_m *FileBackend) RemoveDirectory(path string) *model.AppError {
+func (_m *FileBackend) RemoveDirectory(path string) error {
 	ret := _m.Called(path)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string) error); ok {
 		r0 = rf(path)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
 }
 
 // RemoveFile provides a mock function with given fields: path
-func (_m *FileBackend) RemoveFile(path string) *model.AppError {
+func (_m *FileBackend) RemoveFile(path string) error {
 	ret := _m.Called(path)
 
-	var r0 *model.AppError
+	var r0 error
-	if rf, ok := ret.Get(0).(func(string) *model.AppError); ok {
+	if rf, ok := ret.Get(0).(func(string) error); ok {
 		r0 = rf(path)
 	} else {
-		if ret.Get(0) != nil {
+		r0 = ret.Error(0)
-			r0 = ret.Get(0).(*model.AppError)
-		}
 	}
 
 	return r0
 }
 
 // TestConnection provides a mock function with given fields:
-func (_m *FileBackend) TestConnection() *model.AppError {
+func (_m *FileBackend) TestConnection() 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
 }
 
 // WriteFile provides a mock function with given fields: fr, path
-func (_m *FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
+func (_m *FileBackend) WriteFile(fr io.Reader, path string) (int64, error) {
 	ret := _m.Called(fr, path)
 
 	var r0 int64
@@ -231,13 +209,11 @@ func (_m *FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppEr
 		r0 = ret.Get(0).(int64)
 	}
 
-	var r1 *model.AppError
+	var r1 error
-	if rf, ok := ret.Get(1).(func(io.Reader, string) *model.AppError); ok {
+	if rf, ok := ret.Get(1).(func(io.Reader, string) error); ok {
 		r1 = rf(fr, path)
 	} else {
-		if ret.Get(1) != nil {
+		r1 = ret.Error(1)
-			r1 = ret.Get(1).(*model.AppError)
-		}
 	}
 
 	return r0, r1

services/filesstore/s3store.go

@@ -5,10 +5,8 @@ package filesstore
 
 import (
 	"context"
-	"errors"
 	"io"
 	"io/ioutil"
-	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -16,6 +14,7 @@ import (
 	s3 "github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
 	"github.com/minio/minio-go/v7/pkg/encrypt"
+	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
@@ -118,7 +117,7 @@ func (b *S3FileBackend) s3New() (*s3.Client, error) {
 	return s3Clnt, nil
 }
 
-func (b *S3FileBackend) TestConnection() *model.AppError {
+func (b *S3FileBackend) TestConnection() error {
 	exists := true
 	var err error
 	// If a path prefix is present, we attempt to test the bucket by listing objects under the path
@@ -129,14 +128,14 @@ func (b *S3FileBackend) TestConnection() *model.AppError {
 		if obj.Err != nil {
 			typedErr := s3.ToErrorResponse(obj.Err)
 			if typedErr.Code != bucketNotFound {
-				return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.list_objects.app_error", nil, obj.Err.Error(), http.StatusInternalServerError)
+				return errors.Wrap(err, "unable to list objects in the s3 bucket")
 			}
 			exists = false
 		}
 	} else {
 		exists, err = b.client.BucketExists(context.Background(), b.bucket)
 		if err != nil {
-			return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.bucket_exists.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return errors.Wrap(err, "unable to check if the s3 bucket exists")
 		}
 	}
 
@@ -146,7 +145,7 @@ func (b *S3FileBackend) TestConnection() *model.AppError {
 		mlog.Warn("Bucket specified does not exist. Attempting to create...")
 		err := b.client.MakeBucket(context.Background(), b.bucket, s3.MakeBucketOptions{Region: b.region})
 		if err != nil {
-			return model.NewAppError("TestFileConnection", "api.file.test_connection.s3.bucket_create.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return errors.Wrap(err, "unable to create the s3 bucket")
 		}
 	}
 
@@ -154,32 +153,32 @@ func (b *S3FileBackend) TestConnection() *model.AppError {
 }
 
 // Caller must close the first return value
-func (b *S3FileBackend) Reader(path string) (ReadCloseSeeker, *model.AppError) {
+func (b *S3FileBackend) Reader(path string) (ReadCloseSeeker, error) {
 	path = filepath.Join(b.pathPrefix, path)
 	minioObject, err := b.client.GetObject(context.Background(), b.bucket, path, s3.GetObjectOptions{})
 	if err != nil {
-		return nil, model.NewAppError("Reader", "api.file.reader.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "unable to open file %s", path)
 	}
 
 	return minioObject, nil
 }
 
-func (b *S3FileBackend) ReadFile(path string) ([]byte, *model.AppError) {
+func (b *S3FileBackend) ReadFile(path string) ([]byte, error) {
 	path = filepath.Join(b.pathPrefix, path)
 	minioObject, err := b.client.GetObject(context.Background(), b.bucket, path, s3.GetObjectOptions{})
 	if err != nil {
-		return nil, model.NewAppError("ReadFile", "api.file.read_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "unable to open file %s", path)
 	}
 
 	defer minioObject.Close()
 	if f, err := ioutil.ReadAll(minioObject); err != nil {
-		return nil, model.NewAppError("ReadFile", "api.file.read_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "unable to read file %s", path)
 	} else {
 		return f, nil
 	}
 }
 
-func (b *S3FileBackend) FileExists(path string) (bool, *model.AppError) {
+func (b *S3FileBackend) FileExists(path string) (bool, error) {
 	path = filepath.Join(b.pathPrefix, path)
 
 	_, err := b.client.StatObject(context.Background(), b.bucket, path, s3.StatObjectOptions{})
@@ -192,21 +191,21 @@ func (b *S3FileBackend) FileExists(path string) (bool, *model.AppError) {
 		return false, nil
 	}
 
-	return false, model.NewAppError("FileExists", "api.file.file_exists.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+	return false, errors.Wrapf(err, "unable to know if file %s exists", path)
 }
 
-func (b *S3FileBackend) FileSize(path string) (int64, *model.AppError) {
+func (b *S3FileBackend) FileSize(path string) (int64, error) {
 	path = filepath.Join(b.pathPrefix, path)
 
 	info, err := b.client.StatObject(context.Background(), b.bucket, path, s3.StatObjectOptions{})
 	if err != nil {
-		return 0, model.NewAppError("FileSize", "api.file.file_size.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to get file size for %s", path)
 	}
 
 	return info.Size, nil
 }
 
-func (b *S3FileBackend) CopyFile(oldPath, newPath string) *model.AppError {
+func (b *S3FileBackend) CopyFile(oldPath, newPath string) error {
 	oldPath = filepath.Join(b.pathPrefix, oldPath)
 	newPath = filepath.Join(b.pathPrefix, newPath)
 	srcOpts := s3.CopySrcOptions{
@@ -220,12 +219,12 @@ func (b *S3FileBackend) CopyFile(oldPath, newPath string) *model.AppError {
 		Encryption: encrypt.NewSSE(),
 	}
 	if _, err := b.client.CopyObject(context.Background(), dstOpts, srcOpts); err != nil {
-		return model.NewAppError("copyFile", "api.file.move_file.copy_within_s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to copy file from %s to %s", oldPath, newPath)
 	}
 	return nil
 }
 
-func (b *S3FileBackend) MoveFile(oldPath, newPath string) *model.AppError {
+func (b *S3FileBackend) MoveFile(oldPath, newPath string) error {
 	oldPath = filepath.Join(b.pathPrefix, oldPath)
 	newPath = filepath.Join(b.pathPrefix, newPath)
 	srcOpts := s3.CopySrcOptions{
@@ -240,17 +239,17 @@ func (b *S3FileBackend) MoveFile(oldPath, newPath string) *model.AppError {
 	}
 
 	if _, err := b.client.CopyObject(context.Background(), dstOpts, srcOpts); err != nil {
-		return model.NewAppError("moveFile", "api.file.move_file.copy_within_s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to copy the file to %s to the new destionation", newPath)
 	}
 
 	if err := b.client.RemoveObject(context.Background(), b.bucket, oldPath, s3.RemoveObjectOptions{}); err != nil {
-		return model.NewAppError("moveFile", "api.file.move_file.delete_from_s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to remove the file old file %s", oldPath)
 	}
 
 	return nil
 }
 
-func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
+func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, error) {
 	var contentType string
 	path = filepath.Join(b.pathPrefix, path)
 	if ext := filepath.Ext(path); model.IsFileExtImage(ext) {
@@ -262,16 +261,16 @@ func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppE
 	options := s3PutOptions(b.encrypt, contentType)
 	info, err := b.client.PutObject(context.Background(), b.bucket, path, fr, -1, options)
 	if err != nil {
-		return info.Size, model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return info.Size, errors.Wrapf(err, "unable write the data in the file %s", path)
 	}
 
 	return info.Size, nil
 }
 
-func (b *S3FileBackend) AppendFile(fr io.Reader, path string) (int64, *model.AppError) {
+func (b *S3FileBackend) AppendFile(fr io.Reader, path string) (int64, error) {
 	fp := filepath.Join(b.pathPrefix, path)
 	if _, err := b.client.StatObject(context.Background(), b.bucket, fp, s3.StatObjectOptions{}); err != nil {
-		return 0, model.NewAppError("AppendFile", "api.file.append_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return 0, errors.Wrapf(err, "unable to find the file %s to append the data", path)
 	}
 
 	var contentType string
@@ -302,23 +301,18 @@ func (b *S3FileBackend) AppendFile(fr io.Reader, path string) (int64, *model.App
 		}
 		_, err = b.client.ComposeObject(context.Background(), dstOpts, src1Opts, src2Opts)
 		if err != nil {
-			return 0, model.NewAppError("AppendFile", "api.file.append_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+			return 0, errors.Wrapf(err, "unable append the data in the file %s", path)
 		}
 		return info.Size, nil
 	}
 
-	var errString string
+	return 0, errors.Wrapf(err, "unable append the data in the file %s", path)
-	if err != nil {
-		errString = err.Error()
-	}
-
-	return 0, model.NewAppError("AppendFile", "api.file.append_file.s3.app_error", nil, errString, http.StatusInternalServerError)
 }
 
-func (b *S3FileBackend) RemoveFile(path string) *model.AppError {
+func (b *S3FileBackend) RemoveFile(path string) error {
 	path = filepath.Join(b.pathPrefix, path)
 	if err := b.client.RemoveObject(context.Background(), b.bucket, path, s3.RemoveObjectOptions{}); err != nil {
-		return model.NewAppError("RemoveFile", "utils.file.remove_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return errors.Wrapf(err, "unable to remove the file %s", path)
 	}
 
 	return nil
@@ -344,9 +338,7 @@ func getPathsFromObjectInfos(in <-chan s3.ObjectInfo) <-chan s3.ObjectInfo {
 	return out
 }
 
-func (b *S3FileBackend) ListDirectory(path string) (*[]string, *model.AppError) {
+func (b *S3FileBackend) ListDirectory(path string) (*[]string, error) {
-	var paths []string
-
 	path = filepath.Join(b.pathPrefix, path)
 	if !strings.HasSuffix(path, "/") && len(path) > 0 {
 		// s3Clnt returns only the path itself when "/" is not present
@@ -357,9 +349,10 @@ func (b *S3FileBackend) ListDirectory(path string) (*[]string, *model.AppError)
 	opts := s3.ListObjectsOptions{
 		Prefix: path,
 	}
+	var paths []string
 	for object := range b.client.ListObjects(context.Background(), b.bucket, opts) {
 		if object.Err != nil {
-			return nil, model.NewAppError("ListDirectory", "utils.file.list_directory.s3.app_error", nil, object.Err.Error(), http.StatusInternalServerError)
+			return nil, errors.Wrapf(object.Err, "unable to list the directory %s", path)
 		}
 		// We strip the path prefix that gets applied,
 		// so that it remains transparent to the application.
@@ -373,7 +366,7 @@ func (b *S3FileBackend) ListDirectory(path string) (*[]string, *model.AppError)
 	return &paths, nil
 }
 
-func (b *S3FileBackend) RemoveDirectory(path string) *model.AppError {
+func (b *S3FileBackend) RemoveDirectory(path string) error {
 	opts := s3.ListObjectsOptions{
 		Prefix:    filepath.Join(b.pathPrefix, path),
 		Recursive: true,
@@ -382,7 +375,7 @@ func (b *S3FileBackend) RemoveDirectory(path string) *model.AppError {
 	objectsCh := b.client.RemoveObjects(context.Background(), b.bucket, getPathsFromObjectInfos(list), s3.RemoveObjectsOptions{})
 	for err := range objectsCh {
 		if err.Err != nil {
-			return model.NewAppError("RemoveDirectory", "utils.file.remove_directory.s3.app_error", nil, err.Err.Error(), http.StatusInternalServerError)
+			return errors.Wrapf(err.Err, "unable to remove the directory %s", path)
 		}
 	}
 
@@ -402,9 +395,9 @@ func s3PutOptions(encrypted bool, contentType string) s3.PutObjectOptions {
 	return options
 }
 
-func CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError {
+func CheckMandatoryS3Fields(settings *model.FileSettings) error {
 	if settings.AmazonS3Bucket == nil || len(*settings.AmazonS3Bucket) == 0 {
-		return model.NewAppError("S3File", "api.admin.test_s3.missing_s3_bucket", nil, "", http.StatusBadRequest)
+		return errors.New("missing s3 bucket settings")
 	}
 
 	// if S3 endpoint is not set call the set defaults to set that

services/filesstore/s3store_test.go

@@ -15,7 +15,7 @@ func TestCheckMandatoryS3Fields(t *testing.T) {
 
 	err := CheckMandatoryS3Fields(&cfg)
 	require.NotNil(t, err)
-	require.Equal(t, err.Message, "api.admin.test_s3.missing_s3_bucket", "should've failed with missing s3 bucket")
+	require.Equal(t, err.Error(), "missing s3 bucket settings", "should've failed with missing s3 bucket")
 
 	cfg.AmazonS3Bucket = model.NewString("test-mm")
 	err = CheckMandatoryS3Fields(&cfg)

services/mailservice/mail.go

@@ -298,9 +298,9 @@ func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComp
 	defer c.Quit()
 	defer c.Close()
 
-	fileBackend, err := filesstore.NewFileBackend(&config.FileSettings, enableComplianceFeatures)
+	fileBackend, nErr := filesstore.NewFileBackend(&config.FileSettings, enableComplianceFeatures)
-	if err != nil {
+	if nErr != nil {
-		return err
+		return model.NewAppError("sendMailUsingConfigAdvanced", "api.file.no_driver.app_error", nil, nErr.Error(), http.StatusInternalServerError)
 	}
 
 	return SendMail(c, mail, fileBackend, time.Now())
@@ -349,14 +349,14 @@ func SendMail(c smtpClient, mail mailData, fileBackend filesstore.FileBackend, d
 	}
 
 	for _, fileInfo := range mail.attachments {
-		bytes, err := fileBackend.ReadFile(fileInfo.Path)
+		bytes, nErr := fileBackend.ReadFile(fileInfo.Path)
-		if err != nil {
+		if nErr != nil {
-			return err
+			return model.NewAppError("SendMail", "api.file.read_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
 		}
 
 		m.Attach(fileInfo.Name, gomail.SetCopyFunc(func(writer io.Writer) error {
-			if _, err := writer.Write(bytes); err != nil {
+			if _, nErr = writer.Write(bytes); nErr != nil {
-				return model.NewAppError("SendMail", "utils.mail.sendMail.attachments.write_error", nil, err.Error(), http.StatusInternalServerError)
+				return model.NewAppError("SendMail", "utils.mail.sendMail.attachments.write_error", nil, nErr.Error(), http.StatusInternalServerError)
 			}
 			return nil
 		}))