From 8898d7aab9565c48162c5cf16bfdf2f6f74cdb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Mon, 24 Sep 2018 09:22:29 +0200 Subject: Idiomatic error handling for app/e*.go (#9426) --- app/elasticsearch.go | 21 ++++---- app/email.go | 8 +-- app/email_batching.go | 42 ++++++++-------- app/emoji.go | 136 +++++++++++++++++++++++++++----------------------- 4 files changed, 109 insertions(+), 98 deletions(-) diff --git a/app/elasticsearch.go b/app/elasticsearch.go index c3b558bce..2e731f5d7 100644 --- a/app/elasticsearch.go +++ b/app/elasticsearch.go @@ -18,27 +18,28 @@ func (a *App) TestElasticsearch(cfg *model.Config) *model.AppError { } } - if esI := a.Elasticsearch; esI != nil { - if err := esI.TestConfig(cfg); err != nil { - return err - } - } else { + esI := a.Elasticsearch + if esI == nil { err := model.NewAppError("TestElasticsearch", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) return err } + if err := esI.TestConfig(cfg); err != nil { + return err + } return nil } func (a *App) PurgeElasticsearchIndexes() *model.AppError { - if esI := a.Elasticsearch; esI != nil { - if err := esI.PurgeIndexes(); err != nil { - return err - } - } else { + esI := a.Elasticsearch + if esI == nil { err := model.NewAppError("PurgeElasticsearchIndexes", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) return err } + if err := esI.PurgeIndexes(); err != nil { + return err + } + return nil } diff --git a/app/email.go b/app/email.go index eefe83a81..143d4a052 100644 --- a/app/email.go +++ b/app/email.go @@ -291,6 +291,11 @@ func (a *App) SendInviteEmails(team *model.Team, senderName string, senderUserId return } rateLimited, result, err := a.EmailRateLimiter.RateLimit(senderUserId, len(invites)) + if err != nil { + a.Log.Error("Error rate limiting invite email.", mlog.String("user_id", senderUserId), mlog.String("team_id", team.Id), mlog.Err(err)) + return + } + if rateLimited { a.Log.Error("Invite emails rate limited.", mlog.String("user_id", senderUserId), @@ -298,9 +303,6 @@ func (a *App) SendInviteEmails(team *model.Team, senderName string, senderUserId mlog.String("retry_after", result.RetryAfter.String()), mlog.Err(err)) return - } else if err != nil { - a.Log.Error("Error rate limiting invite email.", mlog.String("user_id", senderUserId), mlog.String("team_id", team.Id), mlog.Err(err)) - return } for _, invite := range invites { diff --git a/app/email_batching.go b/app/email_batching.go index aad1eb8cb..ceee47f32 100644 --- a/app/email_batching.go +++ b/app/email_batching.go @@ -139,21 +139,26 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu if inspectedTeamNames[notification.teamName] != "" { continue } - tchan := job.app.Srv.Store.Team().GetByName(notifications[0].teamName) - if result := <-tchan; result.Err != nil { + + result := <-job.app.Srv.Store.Team().GetByName(notifications[0].teamName) + if result.Err != nil { mlog.Error(fmt.Sprint("Unable to find Team id for notification", result.Err)) continue - } else if team, ok := result.Data.(*model.Team); ok { + } + + if team, ok := result.Data.(*model.Team); ok { inspectedTeamNames[notification.teamName] = team.Id } // if the user has viewed any channels in this team since the notification was queued, delete // all queued notifications - mchan := job.app.Srv.Store.Channel().GetMembersForUser(inspectedTeamNames[notification.teamName], userId) - if result := <-mchan; result.Err != nil { + result = <-job.app.Srv.Store.Channel().GetMembersForUser(inspectedTeamNames[notification.teamName], userId) + if result.Err != nil { mlog.Error(fmt.Sprint("Unable to find ChannelMembers for user", result.Err)) continue - } else if channelMembers, ok := result.Data.(*model.ChannelMembers); ok { + } + + if channelMembers, ok := result.Data.(*model.ChannelMembers); ok { for _, channelMember := range *channelMembers { if channelMember.LastViewedAt >= batchStartTime { mlog.Debug(fmt.Sprintf("Deleted notifications for user %s", userId), mlog.String("user_id", userId)) @@ -194,38 +199,31 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu } func (a *App) sendBatchedEmailNotification(userId string, notifications []*batchedNotification) { - uchan := a.Srv.Store.User().Get(userId) - - var user *model.User - if result := <-uchan; result.Err != nil { + result := <-a.Srv.Store.User().Get(userId) + if result.Err != nil { mlog.Warn("Unable to find recipient for batched email notification") return - } else { - user = result.Data.(*model.User) } + user := result.Data.(*model.User) translateFunc := utils.GetUserTranslations(user.Locale) displayNameFormat := *a.Config().TeamSettings.TeammateNameDisplay var contents string for _, notification := range notifications { - var sender *model.User - schan := a.Srv.Store.User().Get(notification.post.UserId) - if result := <-schan; result.Err != nil { + result := <-a.Srv.Store.User().Get(notification.post.UserId) + if result.Err != nil { mlog.Warn("Unable to find sender of post for batched email notification") continue - } else { - sender = result.Data.(*model.User) } + sender := result.Data.(*model.User) - var channel *model.Channel - cchan := a.Srv.Store.Channel().Get(notification.post.ChannelId, true) - if result := <-cchan; result.Err != nil { + result = <-a.Srv.Store.Channel().Get(notification.post.ChannelId, true) + if result.Err != nil { mlog.Warn("Unable to find channel of post for batched email notification") continue - } else { - channel = result.Data.(*model.Channel) } + channel := result.Data.(*model.Channel) emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL if license := a.License(); license != nil && *license.Features.EmailNotificationContents { diff --git a/app/emoji.go b/app/emoji.go index 864dc31bf..c0eda18a1 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -49,29 +49,33 @@ func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartIma return nil, model.NewAppError("createEmoji", "api.emoji.create.duplicate.app_error", nil, "", http.StatusBadRequest) } - if imageData := multiPartImageData.File["image"]; len(imageData) == 0 { + imageData := multiPartImageData.File["image"] + if len(imageData) == 0 { err := model.NewAppError("Context", "api.context.invalid_body_param.app_error", map[string]interface{}{"Name": "createEmoji"}, "", http.StatusBadRequest) return nil, err - } else if err := a.UploadEmojiImage(emoji.Id, imageData[0]); err != nil { + } + + if err := a.UploadEmojiImage(emoji.Id, imageData[0]); err != nil { return nil, err } - if result := <-a.Srv.Store.Emoji().Save(emoji); result.Err != nil { + result := <-a.Srv.Store.Emoji().Save(emoji) + if result.Err != nil { return nil, result.Err - } else { - message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_EMOJI_ADDED, "", "", "", nil) - message.Add("emoji", emoji.ToJson()) - a.Publish(message) - return result.Data.(*model.Emoji), nil } + + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_EMOJI_ADDED, "", "", "", nil) + message.Add("emoji", emoji.ToJson()) + a.Publish(message) + return result.Data.(*model.Emoji), nil } func (a *App) GetEmojiList(page, perPage int, sort string) ([]*model.Emoji, *model.AppError) { - if result := <-a.Srv.Store.Emoji().GetList(page*perPage, perPage, sort); result.Err != nil { + result := <-a.Srv.Store.Emoji().GetList(page*perPage, perPage, sort) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([]*model.Emoji), nil } + return result.Data.([]*model.Emoji), nil } func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *model.AppError { @@ -85,41 +89,52 @@ func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *mode io.Copy(buf, file) // make sure the file is an image and is within the required dimensions - if config, _, err := image.DecodeConfig(bytes.NewReader(buf.Bytes())); err != nil { + config, _, err := image.DecodeConfig(bytes.NewReader(buf.Bytes())) + if err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.image.app_error", nil, "", http.StatusBadRequest) - } else if config.Width > MaxEmojiOriginalWidth || config.Height > MaxEmojiOriginalHeight { + } + + if config.Width > MaxEmojiOriginalWidth || config.Height > MaxEmojiOriginalHeight { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.too_large.app_error", map[string]interface{}{ "MaxWidth": MaxEmojiOriginalWidth, "MaxHeight": MaxEmojiOriginalHeight, }, "", http.StatusBadRequest) - } else if config.Width > MaxEmojiWidth || config.Height > MaxEmojiHeight { + } + + if config.Width > MaxEmojiWidth || config.Height > MaxEmojiHeight { data := buf.Bytes() newbuf := bytes.NewBuffer(nil) - if info, err := model.GetInfoForBytes(imageData.Filename, data); err != nil { + info, err := model.GetInfoForBytes(imageData.Filename, data) + if err != nil { return err - } else if info.MimeType == "image/gif" { - if gif_data, err := gif.DecodeAll(bytes.NewReader(data)); err != nil { + } + + if info.MimeType == "image/gif" { + gif_data, err := gif.DecodeAll(bytes.NewReader(data)) + if err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_decode_error", nil, "", http.StatusBadRequest) - } else { - resized_gif := resizeEmojiGif(gif_data) - if err := gif.EncodeAll(newbuf, resized_gif); err != nil { - return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest) - } - if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil { - return err - } + } + + resized_gif := resizeEmojiGif(gif_data) + if err := gif.EncodeAll(newbuf, resized_gif); err != nil { + return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest) + } + + if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil { + return err } } else { - if img, _, err := image.Decode(bytes.NewReader(data)); err != nil { + img, _, err := image.Decode(bytes.NewReader(data)) + if err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.decode_error", nil, "", http.StatusBadRequest) - } else { - resized_image := resizeEmoji(img, config.Width, config.Height) - if err := png.Encode(newbuf, resized_image); err != nil { - return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest) - } - if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil { - return err - } + } + + resized_image := resizeEmoji(img, config.Width, config.Height) + if err := png.Encode(newbuf, resized_image); err != nil { + return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest) + } + if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil { + return err } } } @@ -147,11 +162,11 @@ func (a *App) GetEmoji(emojiId string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - if result := <-a.Srv.Store.Emoji().Get(emojiId, false); result.Err != nil { + result := <-a.Srv.Store.Emoji().Get(emojiId, false) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.Emoji), nil } + return result.Data.(*model.Emoji), nil } func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { @@ -163,32 +178,30 @@ func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - if result := <-a.Srv.Store.Emoji().GetByName(emojiName); result.Err != nil { + result := <-a.Srv.Store.Emoji().GetByName(emojiName) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.(*model.Emoji), nil } + return result.Data.(*model.Emoji), nil } -func (a *App) GetEmojiImage(emojiId string) (imageByte []byte, imageType string, err *model.AppError) { - if result := <-a.Srv.Store.Emoji().Get(emojiId, true); result.Err != nil { +func (a *App) GetEmojiImage(emojiId string) ([]byte, string, *model.AppError) { + result := <-a.Srv.Store.Emoji().Get(emojiId, true) + if result.Err != nil { return nil, "", result.Err - } else { - var img []byte - - if data, err := a.ReadFile(getEmojiImagePath(emojiId)); err != nil { - return nil, "", model.NewAppError("getEmojiImage", "api.emoji.get_image.read.app_error", nil, err.Error(), http.StatusNotFound) - } else { - img = data - } + } - _, imageType, err := image.DecodeConfig(bytes.NewReader(img)) - if err != nil { - return nil, "", model.NewAppError("getEmojiImage", "api.emoji.get_image.decode.app_error", nil, err.Error(), http.StatusInternalServerError) - } + img, appErr := a.ReadFile(getEmojiImagePath(emojiId)) + if appErr != nil { + return nil, "", model.NewAppError("getEmojiImage", "api.emoji.get_image.read.app_error", nil, appErr.Error(), http.StatusNotFound) + } - return img, imageType, nil + _, imageType, err := image.DecodeConfig(bytes.NewReader(img)) + if err != nil { + return nil, "", model.NewAppError("getEmojiImage", "api.emoji.get_image.decode.app_error", nil, err.Error(), http.StatusInternalServerError) } + + return img, imageType, nil } func (a *App) SearchEmoji(name string, prefixOnly bool, limit int) ([]*model.Emoji, *model.AppError) { @@ -196,11 +209,11 @@ func (a *App) SearchEmoji(name string, prefixOnly bool, limit int) ([]*model.Emo return nil, model.NewAppError("SearchEmoji", "api.emoji.disabled.app_error", nil, "", http.StatusNotImplemented) } - if result := <-a.Srv.Store.Emoji().Search(name, prefixOnly, limit); result.Err != nil { + result := <-a.Srv.Store.Emoji().Search(name, prefixOnly, limit) + if result.Err != nil { return nil, result.Err - } else { - return result.Data.([]*model.Emoji), nil } + return result.Data.([]*model.Emoji), nil } func resizeEmojiGif(gifImg *gif.GIF) *gif.GIF { @@ -231,13 +244,10 @@ func resizeEmoji(img image.Image, width int, height int) image.Image { emojiWidth := float64(width) emojiHeight := float64(height) - var emoji image.Image if emojiHeight <= MaxEmojiHeight && emojiWidth <= MaxEmojiWidth { - emoji = img - } else { - emoji = imaging.Fit(img, MaxEmojiWidth, MaxEmojiHeight, imaging.Lanczos) + return img } - return emoji + return imaging.Fit(img, MaxEmojiWidth, MaxEmojiHeight, imaging.Lanczos) } func imageToPaletted(img image.Image) *image.Paletted { -- cgit v1.2.3-1-g7c22