Migrating AppError to error for mailservice (#16810)

* Migrating AppError to error for mailservice

* Updating i18n strings

* Fixing shadow variable problem

* Addressing PR review comments

* fixing test
Этот коммит содержится в:
Jesús Espino
2021-02-09 12:28:42 +01:00
коммит произвёл GitHub
родитель 572f861675
Коммит fbe0294e86
8 изменённых файлов: 61 добавлений и 95 удалений

Просмотреть файл

@@ -292,7 +292,7 @@ func (es *EmailService) SendPasswordResetEmail(email string, token *model.Token,
bodyPage.Props["Button"] = T("api.templates.reset_body.button")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendPasswordReset", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendPasswordReset", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -462,8 +462,8 @@ func (es *EmailService) sendGuestInviteEmails(team *model.Team, channels []*mode
}
}
if err := es.sendMailWithEmbeddedFiles(invite, subject, bodyPage.Render(), embeddedFiles); err != nil {
mlog.Error("Failed to send invite email successfully", mlog.Err(err))
if nErr := es.sendMailWithEmbeddedFiles(invite, subject, bodyPage.Render(), embeddedFiles); nErr != nil {
mlog.Error("Failed to send invite email successfully", mlog.Err(nErr))
}
}
}
@@ -545,23 +545,23 @@ func (es *EmailService) SendRemoveExpiredLicenseEmail(email string, locale, site
return nil
}
func (es *EmailService) sendNotificationMail(to, subject, htmlBody string) *model.AppError {
func (es *EmailService) sendNotificationMail(to, subject, htmlBody string) error {
if !*es.srv.Config().EmailSettings.SendEmailNotifications {
return nil
}
return es.sendMail(to, subject, htmlBody)
}
func (es *EmailService) sendMail(to, subject, htmlBody string) *model.AppError {
func (es *EmailService) sendMail(to, subject, htmlBody string) error {
return es.sendMailWithCC(to, subject, htmlBody, "")
}
func (es *EmailService) sendMailWithCC(to, subject, htmlBody string, ccMail string) *model.AppError {
func (es *EmailService) sendMailWithCC(to, subject, htmlBody string, ccMail string) error {
license := es.srv.License()
return mailservice.SendMailUsingConfig(to, subject, htmlBody, es.srv.Config(), license != nil && *license.Features.Compliance, ccMail)
}
func (es *EmailService) sendMailWithEmbeddedFiles(to, subject, htmlBody string, embeddedFiles map[string]io.Reader) *model.AppError {
func (es *EmailService) sendMailWithEmbeddedFiles(to, subject, htmlBody string, embeddedFiles map[string]io.Reader) error {
license := es.srv.License()
config := es.srv.Config()
@@ -613,7 +613,7 @@ func (es *EmailService) SendAtUserLimitWarningEmail(email string, locale string,
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendAtUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendAtUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -635,7 +635,7 @@ func (es *EmailService) SendOverUserLimitWarningEmail(email string, locale strin
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -660,7 +660,7 @@ func (es *EmailService) SendOverUserLimitThirtyDayWarningEmail(email string, loc
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -684,7 +684,7 @@ func (es *EmailService) SendOverUserLimitNinetyDayWarningEmail(email string, loc
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -706,7 +706,7 @@ func (es *EmailService) SendOverUserLimitWorkspaceSuspendedWarningEmail(email st
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -727,7 +727,7 @@ func (es *EmailService) SendOverUserFourteenDayWarningEmail(email string, locale
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -748,7 +748,7 @@ func (es *EmailService) SendOverUserSevenDayWarningEmail(email string, locale st
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -767,7 +767,7 @@ func (es *EmailService) SendSuspensionEmailToSupport(email string, installationI
bodyPage.Props["UserCount"] = userCount
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendOverUserLimitWarningEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -792,7 +792,7 @@ func (es *EmailService) SendPaymentFailedEmail(email string, locale string, fail
bodyPage.Props["FailedReason"] = failedPayment.FailureMessage
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return false, model.NewAppError("SendPaymentFailedEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return false, model.NewAppError("SendPaymentFailedEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return true, nil
@@ -814,7 +814,7 @@ func (es *EmailService) SendNoCardPaymentFailedEmail(email string, locale string
bodyPage.Props["Footer"] = T("api.templates.copyright")
if err := es.sendMail(email, subject, bodyPage.Render()); err != nil {
return model.NewAppError("SendPaymentFailedEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Message, http.StatusInternalServerError)
return model.NewAppError("SendPaymentFailedEmail", "api.user.send_password_reset.send.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
}
return nil

Просмотреть файл

@@ -239,8 +239,8 @@ func (es *EmailService) sendBatchedEmailNotification(userID string, notification
body.Props["Posts"] = template.HTML(contents)
body.Props["BodyText"] = translateFunc("api.email_batching.send_batched_email_notification.body_text", len(notifications))
if err := es.sendNotificationMail(user.Email, subject, body.Render()); err != nil {
mlog.Warn("Unable to send batched email notification", mlog.String("email", user.Email), mlog.Err(err))
if nErr := es.sendNotificationMail(user.Email, subject, body.Render()); nErr != nil {
mlog.Warn("Unable to send batched email notification", mlog.String("email", user.Email), mlog.Err(nErr))
}
}

Просмотреть файл

@@ -101,8 +101,8 @@ func (a *App) sendNotificationEmail(notification *PostNotification, user *model.
var bodyText = a.getNotificationEmailBody(user, post, channel, channelName, senderName, team.Name, landingURL, emailNotificationContentsType, useMilitaryTime, translateFunc)
a.Srv().Go(func() {
if err := a.Srv().EmailService.sendNotificationMail(user.Email, html.UnescapeString(subjectText), bodyText); err != nil {
mlog.Error("Error while sending the email", mlog.String("user_email", user.Email), mlog.Err(err))
if nErr := a.Srv().EmailService.sendNotificationMail(user.Email, html.UnescapeString(subjectText), bodyText); nErr != nil {
mlog.Error("Error while sending the email", mlog.String("user_email", user.Email), mlog.Err(nErr))
}
})

Просмотреть файл

@@ -731,7 +731,11 @@ func (api *PluginAPI) SendMail(to, subject, htmlBody string) *model.AppError {
return model.NewAppError("SendMail", "plugin_api.send_mail.missing_htmlbody", nil, "", http.StatusBadRequest)
}
return api.app.Srv().EmailService.sendNotificationMail(to, subject, htmlBody)
if err := api.app.Srv().EmailService.sendNotificationMail(to, subject, htmlBody); err != nil {
return model.NewAppError("SendMail", "plugin_api.send_mail.missing_htmlbody", nil, err.Error(), http.StatusInternalServerError)
}
return nil
}
// Plugin Section

Просмотреть файл

@@ -473,8 +473,8 @@ func NewServer(options ...Option) (*Server, error) {
}
s.WebSocketRouter.app = fakeApp
if appErr := mailservice.TestConnection(s.Config()); appErr != nil {
mlog.Error("Mail server connection test is failed: " + appErr.Message)
if nErr := mailservice.TestConnection(s.Config()); nErr != nil {
mlog.Error("Mail server connection test is failed", mlog.Err(nErr))
}
if _, err = url.ParseRequestURI(*s.Config().ServiceSettings.SiteURL); err != nil {
@@ -485,8 +485,7 @@ func NewServer(options ...Option) (*Server, error) {
if appErr != nil {
mlog.Error("Problem with file storage settings", mlog.Err(appErr))
} else {
nErr := backend.TestConnection()
if nErr != nil {
if nErr := backend.TestConnection(); nErr != nil {
mlog.Error("Problem with file storage settings", mlog.Err(nErr))
}
}

Просмотреть файл

@@ -6670,6 +6670,10 @@
"id": "ent.message_export.global_relay_export.deliver.to_address.app_error",
"translation": "Unable to set the email To address."
},
{
"id": "ent.message_export.global_relay_export.deliver.unable_to_connect_smtp_server.app_error",
"translation": "Unable to connect to the smtp server"
},
{
"id": "ent.message_export.global_relay_export.deliver.unable_to_get_file_info.app_error",
"translation": "Unable to get the information of the export temporary file."
@@ -8394,46 +8398,6 @@
"id": "system.message.name",
"translation": "System"
},
{
"id": "utils.mail.connect_smtp.helo.app_error",
"translation": "Failed to set HELO."
},
{
"id": "utils.mail.connect_smtp.open.app_error",
"translation": "Failed to open connection."
},
{
"id": "utils.mail.connect_smtp.open_tls.app_error",
"translation": "Failed to open TLS connection."
},
{
"id": "utils.mail.new_client.auth.app_error",
"translation": "Failed to authenticate on SMTP server."
},
{
"id": "utils.mail.sendMail.attachments.write_error",
"translation": "Failed to write attachment to email"
},
{
"id": "utils.mail.send_mail.close.app_error",
"translation": "Failed to close connection to SMTP server."
},
{
"id": "utils.mail.send_mail.from_address.app_error",
"translation": "Error setting \"From Address\""
},
{
"id": "utils.mail.send_mail.msg.app_error",
"translation": "Failed to write email message."
},
{
"id": "utils.mail.send_mail.msg_data.app_error",
"translation": "Failed to add email message data."
},
{
"id": "utils.mail.send_mail.to_address.app_error",
"translation": "Error setting \"To Address\"."
},
{
"id": "web.command_webhook.command.app_error",
"translation": "Couldn't find the command."

Просмотреть файл

@@ -6,16 +6,15 @@ package mailservice
import (
"context"
"crypto/tls"
"errors"
"io"
"mime"
"net"
"net/http"
"net/mail"
"net/smtp"
"time"
"github.com/jaytaylor/html2text"
"github.com/pkg/errors"
gomail "gopkg.in/mail.v2"
"github.com/mattermost/mattermost-server/v5/mlog"
@@ -112,7 +111,7 @@ func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) {
return nil, nil
}
func ConnectToSMTPServerAdvanced(connectionInfo *SmtpConnectionInfo) (net.Conn, *model.AppError) {
func ConnectToSMTPServerAdvanced(connectionInfo *SmtpConnectionInfo) (net.Conn, error) {
var conn net.Conn
var err error
@@ -129,19 +128,19 @@ func ConnectToSMTPServerAdvanced(connectionInfo *SmtpConnectionInfo) (net.Conn,
conn, err = tls.DialWithDialer(dialer, "tcp", smtpAddress, tlsconfig)
if err != nil {
return nil, model.NewAppError("SendMail", "utils.mail.connect_smtp.open_tls.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "unable to connect to the SMTP server through TLS")
}
} else {
conn, err = dialer.Dial("tcp", smtpAddress)
if err != nil {
return nil, model.NewAppError("SendMail", "utils.mail.connect_smtp.open.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "unable to connect to the SMTP server")
}
}
return conn, nil
}
func ConnectToSMTPServer(config *model.Config) (net.Conn, *model.AppError) {
func ConnectToSMTPServer(config *model.Config) (net.Conn, error) {
return ConnectToSMTPServerAdvanced(
&SmtpConnectionInfo{
ConnectionSecurity: *config.EmailSettings.ConnectionSecurity,
@@ -154,7 +153,7 @@ func ConnectToSMTPServer(config *model.Config) (net.Conn, *model.AppError) {
)
}
func NewSMTPClientAdvanced(ctx context.Context, conn net.Conn, hostname string, connectionInfo *SmtpConnectionInfo) (*smtp.Client, *model.AppError) {
func NewSMTPClientAdvanced(ctx context.Context, conn net.Conn, hostname string, connectionInfo *SmtpConnectionInfo) (*smtp.Client, error) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
@@ -174,16 +173,16 @@ func NewSMTPClientAdvanced(ctx context.Context, conn net.Conn, hostname string,
case <-ctx.Done():
err := ctx.Err()
if err != nil && err.Error() != "context canceled" {
return nil, model.NewAppError("SendMail", "utils.mail.connect_smtp.open_tls.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "unable to connect to the SMTP server")
}
case err := <-ec:
return nil, model.NewAppError("SendMail", "utils.mail.connect_smtp.open_tls.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "unable to connect to the SMTP server")
}
if hostname != "" {
err := c.Hello(hostname)
if err != nil {
return nil, model.NewAppError("SendMail", "utils.mail.connect_smtp.helo.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "unable to send hello message")
}
}
@@ -197,13 +196,13 @@ func NewSMTPClientAdvanced(ctx context.Context, conn net.Conn, hostname string,
if connectionInfo.Auth {
if err := c.Auth(&authChooser{connectionInfo: connectionInfo}); err != nil {
return nil, model.NewAppError("SendMail", "utils.mail.new_client.auth.app_error", nil, err.Error(), http.StatusInternalServerError)
return nil, errors.Wrap(err, "authentication failed")
}
}
return c, nil
}
func NewSMTPClient(ctx context.Context, conn net.Conn, config *model.Config) (*smtp.Client, *model.AppError) {
func NewSMTPClient(ctx context.Context, conn net.Conn, config *model.Config) (*smtp.Client, error) {
return NewSMTPClientAdvanced(
ctx,
conn,
@@ -222,14 +221,14 @@ func NewSMTPClient(ctx context.Context, conn net.Conn, config *model.Config) (*s
)
}
func TestConnection(config *model.Config) *model.AppError {
func TestConnection(config *model.Config) error {
if !*config.EmailSettings.SendEmailNotifications {
return &model.AppError{Message: "SendEmailNotifications is not true"}
return errors.New("SendEmailNotifications is not true")
}
conn, err := ConnectToSMTPServer(config)
if err != nil {
return &model.AppError{Message: "Could not connect to SMTP server, check SMTP server settings.", DetailedError: err.DetailedError}
return errors.Wrap(err, "unable to connect")
}
defer conn.Close()
@@ -241,7 +240,7 @@ func TestConnection(config *model.Config) *model.AppError {
c, err := NewSMTPClient(ctx, conn, config)
if err != nil {
return &model.AppError{Message: "Could not connect to SMTP server, check SMTP server settings."}
return errors.Wrap(err, "unable to connect")
}
c.Close()
c.Quit()
@@ -249,7 +248,7 @@ func TestConnection(config *model.Config) *model.AppError {
return nil
}
func SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody string, embeddedFiles map[string]io.Reader, config *model.Config, enableComplianceFeatures bool, ccMail string) *model.AppError {
func SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody string, embeddedFiles map[string]io.Reader, config *model.Config, enableComplianceFeatures bool, ccMail string) error {
fromMail := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.FeedbackEmail}
replyTo := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.ReplyToAddress}
@@ -267,12 +266,12 @@ func SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody string, embedded
return sendMailUsingConfigAdvanced(mail, config, enableComplianceFeatures)
}
func SendMailUsingConfig(to, subject, htmlBody string, config *model.Config, enableComplianceFeatures bool, ccMail string) *model.AppError {
func SendMailUsingConfig(to, subject, htmlBody string, config *model.Config, enableComplianceFeatures bool, ccMail string) error {
return SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody, nil, config, enableComplianceFeatures, ccMail)
}
// allows for sending an email with attachments and differing MIME/SMTP recipients
func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComplianceFeatures bool) *model.AppError {
func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComplianceFeatures bool) error {
if *config.EmailSettings.SMTPServer == "" {
return nil
}
@@ -298,13 +297,13 @@ func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComp
fileBackend, nErr := filesstore.NewFileBackend(&config.FileSettings, enableComplianceFeatures)
if nErr != nil {
return model.NewAppError("sendMailUsingConfigAdvanced", "api.file.no_driver.app_error", nil, nErr.Error(), http.StatusInternalServerError)
return errors.Wrap(nErr, "unable to initialize file backend")
}
return SendMail(c, mail, fileBackend, time.Now())
}
func SendMail(c smtpClient, mail mailData, fileBackend filesstore.FileBackend, date time.Time) *model.AppError {
func SendMail(c smtpClient, mail mailData, fileBackend filesstore.FileBackend, date time.Time) error {
mlog.Debug("sending mail", mlog.String("to", mail.smtpTo), mlog.String("subject", mail.subject))
htmlMessage := "\r\n<html><body>" + mail.htmlBody + "</body></html>"
@@ -349,37 +348,37 @@ func SendMail(c smtpClient, mail mailData, fileBackend filesstore.FileBackend, d
for _, fileInfo := range mail.attachments {
bytes, nErr := fileBackend.ReadFile(fileInfo.Path)
if nErr != nil {
return model.NewAppError("SendMail", "api.file.read_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to read attachment")
}
m.Attach(fileInfo.Name, gomail.SetCopyFunc(func(writer io.Writer) error {
if _, nErr = writer.Write(bytes); nErr != nil {
return model.NewAppError("SendMail", "utils.mail.sendMail.attachments.write_error", nil, nErr.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to write attachment to email")
}
return nil
}))
}
if err = c.Mail(mail.from.Address); err != nil {
return model.NewAppError("SendMail", "utils.mail.send_mail.from_address.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to set the from address")
}
if err = c.Rcpt(mail.smtpTo); err != nil {
return model.NewAppError("SendMail", "utils.mail.send_mail.to_address.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to set the to address")
}
w, err := c.Data()
if err != nil {
return model.NewAppError("SendMail", "utils.mail.send_mail.msg_data.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to add email message data")
}
_, err = m.WriteTo(w)
if err != nil {
return model.NewAppError("SendMail", "utils.mail.send_mail.msg.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to write the email message")
}
err = w.Close()
if err != nil {
return model.NewAppError("SendMail", "utils.mail.send_mail.close.app_error", nil, err.Error(), http.StatusInternalServerError)
return errors.Wrap(err, "failed to close connection to the SMTP server")
}
return nil

Просмотреть файл

@@ -110,7 +110,7 @@ func TestMailConnectionAdvanced(t *testing.T) {
connInfo,
)
require.NotNil(t, err4, "Should get a timeout get while creating a new SMTP client")
assert.Equal(t, err4.Id, "utils.mail.connect_smtp.open_tls.app_error")
assert.Contains(t, err4.Error(), "unable to connect to the SMTP server")
_, err5 := ConnectToSMTPServerAdvanced(
&SmtpConnectionInfo{