Use appropriate log levels, wrap errors coming from msgraph for loggi…

…ng (#101)

Automatic Merge

server/api/post_action.go

@@ -50,7 +50,7 @@ func (api *api) postActionAccept(w http.ResponseWriter, req *http.Request) {
 	}
 	err := mscalendar.AcceptEvent(user, eventID)
 	if err != nil {
-		api.Logger.Warnf(err.Error())
+		api.Logger.Warnf("Failed to accept event. err=%v", err)
 		utils.SlackAttachmentError(w, "Error: Failed to accept event: "+err.Error())
 		return
 	}

server/command/command.go

@@ -100,7 +100,7 @@ func (c *Command) isValid() (subcommand string, parameters []string, err error)
 	split := strings.Fields(c.Args.Command)
 	command := split[0]
 	if command != "/"+config.CommandTrigger {
-		return "", nil, errors.Errorf("%q is not a supported command. Please contact your system administrator.", command)
+		return "", nil, fmt.Errorf("%q is not a supported command. Please contact your system administrator.", command)
 	}
 
 	parameters = []string{}

server/command/create_event.go

@@ -6,10 +6,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/mattermost/mattermost-plugin-mscalendar/server/remote"
-	"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
 	"github.com/pkg/errors"
 	flag "github.com/spf13/pflag"
+
+	"github.com/mattermost/mattermost-plugin-mscalendar/server/remote"
+	"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
 )
 
 func getCreateEventFlagSet() *flag.FlagSet {
@@ -82,13 +83,13 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 		})
 	for _, req := range requiredFlags {
 		if !flags[req] {
-			return nil, errors.Errorf("Missing required flag: `--%s` ", req)
+			return nil, fmt.Errorf("Missing required flag: `--%s` ", req)
 		}
 	}
 
 	help, err := createFlagSet.GetBool("help")
 	if help == true {
-		return nil, errors.Errorf(getCreateEventFlagSet().FlagUsages())
+		return nil, errors.New(getCreateEventFlagSet().FlagUsages())
 	}
 
 	subject, err := createFlagSet.GetString("test-subject")
@@ -97,7 +98,7 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 	}
 	// check that next arg is not a flag "--"
 	if strings.HasPrefix(subject, "--") {
-		return nil, errors.Errorf("test-subject flag requires an argument")
+		return nil, errors.New("test-subject flag requires an argument")
 	}
 	event.Subject = subject
 
@@ -107,7 +108,7 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 	}
 	// check that next arg is not a flag "--"
 	if strings.HasPrefix(body, "--") {
-		return nil, errors.Errorf("body flag requires an argument")
+		return nil, errors.New("body flag requires an argument")
 	}
 	event.Body = &remote.ItemBody{
 		Content: body,
@@ -118,7 +119,7 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 		return nil, err
 	}
 	if strings.HasPrefix(startTime, "--") {
-		return nil, errors.Errorf("starttime flag requires an argument")
+		return nil, errors.New("starttime flag requires an argument")
 	}
 	event.Start = &remote.DateTime{
 		DateTime: startTime,
@@ -130,7 +131,7 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 		return nil, err
 	}
 	if strings.HasPrefix(endTime, "--") {
-		return nil, errors.Errorf("endtime flag requires an argument")
+		return nil, errors.New("endtime flag requires an argument")
 	}
 	event.End = &remote.DateTime{
 		DateTime: endTime,
@@ -148,7 +149,7 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 		return nil, err
 	}
 	if strings.HasPrefix(strconv.Itoa(int(reminder)), "--") {
-		return nil, errors.Errorf("reminder flag requires an argument")
+		return nil, errors.New("reminder flag requires an argument")
 	}
 	event.ReminderMinutesBeforeStart = reminder
 
@@ -158,7 +159,7 @@ func parseCreateArgs(args []string, timeZone string) (*remote.Event, error) {
 	}
 	if len(location) != 0 {
 		if len(location) != 6 {
-			return nil, errors.Errorf("test-location flag requires 6 parameters, including a comma for empty values")
+			return nil, errors.New("test-location flag requires 6 parameters, including a comma for empty values")
 		}
 		event.Location = &remote.Location{
 			LocationType: "default",

server/jobs/daily_summary_job.go

@@ -30,7 +30,7 @@ func runDailySummaryJob(env mscalendar.Env) {
 
 	err := mscalendar.New(env, "").ProcessAllDailySummary(time.Now())
 	if err != nil {
-		env.Logger.Errorf("Error during daily summary job: %v", err)
+		env.Logger.Errorf("Error during daily summary job. err=%v", err)
 	}
 
 	env.Logger.Debugf("Daily summary job finished")

server/jobs/job_manager.go

@@ -5,13 +5,13 @@ package jobs
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"sync"
 	"time"
 
 	"github.com/mattermost/mattermost-plugin-api/cluster"
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar"
-	"github.com/pkg/errors"
 )
 
 type JobManager struct {
@@ -56,10 +56,8 @@ func NewJobManager(papi cluster.JobPluginAPI, env mscalendar.Env) *JobManager {
 }
 
 // AddJob accepts a RegisteredJob, stores it, and activates it if enabled.
-func (jm *JobManager) AddJob(job RegisteredJob) error {
+func (jm *JobManager) AddJob(job RegisteredJob) {
 	jm.registeredJobs.Store(job.id, job)
-
-	return nil
 }
 
 // OnConfigurationChange activates/deactivates jobs based on their current state, and the current plugin config.
@@ -77,15 +75,15 @@ func (jm *JobManager) OnConfigurationChange(env mscalendar.Env) error {
 		if enabled && !active {
 			err := jm.activateJob(job)
 			if err != nil {
-				jm.env.Logger.Errorf("Error activating %s job: %v", job.id, err)
+				jm.env.Logger.Warnf("Error activating %s job. err=%v", job.id, err)
 			}
 		}
 
 		// Config is set to disable. Job is active, so deactivate the job.
 		if !enabled && active {
 			err := jm.deactivateJob(job)
 			if err != nil {
-				jm.env.Logger.Errorf("Error deactivating %s job: %v", job.id, err)
+				jm.env.Logger.Warnf("Error deactivating %s job. err=%v", job.id, err)
 			}
 		}
 
@@ -125,7 +123,7 @@ func (jm *JobManager) activateJob(job RegisteredJob) error {
 func (jm *JobManager) deactivateJob(job RegisteredJob) error {
 	v, ok := jm.activeJobs.Load(job.id)
 	if !ok {
-		return errors.Errorf("Attempted to deactivate a non-active job %s", job.id)
+		return fmt.Errorf("Attempted to deactivate a non-active job %s", job.id)
 	}
 
 	scheduledJob := v.(*activeJob)

server/jobs/renew_job.go

@@ -23,7 +23,7 @@ func NewRenewJob() RegisteredJob {
 func runRenewJob(env mscalendar.Env) {
 	uindex, err := env.Store.LoadUserIndex()
 	if err != nil {
-		env.Logger.Errorf("Renew job failed to load user index: %s", err.Error())
+		env.Logger.Errorf("Renew job failed to load user index. err=%v", err)
 		return
 	}
 	env.Logger.Debugf("Renew job: %v users", len(uindex))
@@ -33,7 +33,7 @@ func runRenewJob(env mscalendar.Env) {
 		env.Logger.Debugf("Renewing for user: %s", u.MattermostUserID)
 		_, err = asUser.RenewMyEventSubscription()
 		if err != nil {
-			env.Logger.Errorf("Error renewing subscription: %s", err.Error())
+			env.Logger.Errorf("Error renewing subscription. err=%v", err)
 		}
 
 		time.Sleep(ditherRenew)

server/jobs/status_sync_job.go

@@ -26,7 +26,7 @@ func runSyncJob(env mscalendar.Env) {
 
 	_, err := mscalendar.New(env, "").SyncAll()
 	if err != nil {
-		env.Logger.Errorf("Error during user status sync job: %v", err)
+		env.Logger.Errorf("Error during user status sync job. err=%v", err)
 	}
 
 	env.Logger.Debugf("User status sync job finished")

server/mscalendar/availability.go

@@ -113,7 +113,7 @@ func (m *mscalendar) deliverReminders(users []*store.User, calendarViews []*remo
 			continue
 		}
 		if view.Error != nil {
-			m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, view.Error.Message)
+			m.Logger.Warnf("Error getting availability for %s. err=%s", user.Remote.Mail, view.Error.Message)
 			continue
 		}
 
@@ -156,7 +156,7 @@ func (m *mscalendar) setUserStatuses(users []*store.User, calendarViews []*remot
 			continue
 		}
 		if view.Error != nil {
-			m.Logger.Warnf("Error getting availability for %s: %s", user.Remote.Mail, view.Error.Message)
+			m.Logger.Warnf("Error getting availability for %s. err=%s", user.Remote.Mail, view.Error.Message)
 			continue
 		}
 
@@ -169,7 +169,7 @@ func (m *mscalendar) setUserStatuses(users []*store.User, calendarViews []*remot
 		var err error
 		res, err = m.setStatusFromCalendarView(user, status, view)
 		if err != nil {
-			m.Logger.Warnf("Error setting user %s status. %s", user.Remote.Mail, err.Error())
+			m.Logger.Warnf("Error setting user %s status. err=%v", user.Remote.Mail, err)
 		}
 	}
 	if res != "" {
@@ -321,19 +321,19 @@ func (m *mscalendar) notifyUpcomingEvents(mattermostUserID string, events []*rem
 			if timezone == "" {
 				timezone, err = m.GetTimezoneByID(mattermostUserID)
 				if err != nil {
-					m.Logger.Warnf("notifyUpcomingEvents error getting timezone, err=%s", err.Error())
+					m.Logger.Warnf("notifyUpcomingEvents error getting timezone. err=%v", err)
 					return
 				}
 			}
 
 			message, err := views.RenderUpcomingEvent(event, timezone)
 			if err != nil {
-				m.Logger.Warnf("notifyUpcomingEvent error rendering schedule item, err=", err.Error())
+				m.Logger.Warnf("notifyUpcomingEvent error rendering schedule item. err=%v", err)
 				continue
 			}
 			_, err = m.Poster.DM(mattermostUserID, message)
 			if err != nil {
-				m.Logger.Warnf("notifyUpcomingEvents error creating DM, err=", err.Error())
+				m.Logger.Warnf("notifyUpcomingEvents error creating DM. err=%v", err)
 				continue
 			}
 		}

server/mscalendar/availability_test.go

@@ -132,7 +132,7 @@ func TestSyncStatusAll(t *testing.T) {
 			}
 
 			if tc.shouldLogError {
-				logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", tc.apiError.Message).Times(1)
+				logger.EXPECT().Warnf("Error getting availability for %s. err=%s", "user_email@example.com", tc.apiError.Message).Times(1)
 			} else {
 				logger.EXPECT().Warnf(gomock.Any()).Times(0)
 			}
@@ -291,7 +291,7 @@ func TestReminders(t *testing.T) {
 			}
 
 			if tc.shouldLogError {
-				logger.EXPECT().Warnf("Error getting availability for %s: %s", "user_email@example.com", tc.apiError.Message).Times(1)
+				logger.EXPECT().Warnf("Error getting availability for %s. err=%s", "user_email@example.com", tc.apiError.Message).Times(1)
 			} else {
 				logger.EXPECT().Warnf(gomock.Any()).Times(0)
 			}

server/mscalendar/daily_summary.go

@@ -4,6 +4,7 @@
 package mscalendar
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/mattermost/mattermost-plugin-mscalendar/server/mscalendar/views"
@@ -50,7 +51,7 @@ func (m *mscalendar) SetDailySummaryPostTime(user *User, timeStr string) (*store
 	}
 
 	if t.Minute()%int(DailySummaryJobInterval/time.Minute) != 0 {
-		return nil, errors.Errorf("Time must be a multiple of %d minutes.", DailySummaryJobInterval/time.Minute)
+		return nil, fmt.Errorf("Time must be a multiple of %d minutes.", DailySummaryJobInterval/time.Minute)
 	}
 
 	timezone, err := m.GetTimezone(user)
@@ -119,7 +120,7 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error {
 	for _, dsum := range dsumIndex {
 		shouldPost, shouldPostErr := shouldPostDailySummary(dsum, now)
 		if shouldPostErr != nil {
-			m.Logger.Errorf("Error posting daily summary for user %s: %v", dsum.MattermostUserID, shouldPostErr)
+			m.Logger.Warnf("Error posting daily summary for user %s. err=%v", dsum.MattermostUserID, shouldPostErr)
 			continue
 		}
 		if !shouldPost {
@@ -145,11 +146,11 @@ func (m *mscalendar) ProcessAllDailySummary(now time.Time) error {
 	for _, res := range responses {
 		dsum := byRemoteID[res.RemoteUserID]
 		if res.Error != nil {
-			m.Logger.Errorf("Error rendering user %s calendar: %s %s", dsum.MattermostUserID, res.Error.Code, res.Error.Message)
+			m.Logger.Warnf("Error rendering user %s calendar. err=%s %s", dsum.MattermostUserID, res.Error.Code, res.Error.Message)
 		}
 		postStr, err := views.RenderCalendarView(res.Events, dsum.Timezone)
 		if err != nil {
-			m.Logger.Errorf("Error rendering user %s calendar: %v", dsum.MattermostUserID, err)
+			m.Logger.Warnf("Error rendering user %s calendar. err=%v", dsum.MattermostUserID, err)
 		}
 
 		m.Poster.DM(dsum.MattermostUserID, postStr)

server/mscalendar/notification.go

@@ -87,7 +87,7 @@ func (processor *notificationProcessor) Enqueue(notifications ...*remote.Notific
 		select {
 		case processor.queue <- n:
 		default:
-			return errors.Errorf("webhook notification: queue full, dropped notification")
+			return fmt.Errorf("webhook notification: queue full, dropped notification")
 		}
 	}
 	return nil

server/mscalendar/oauth2.go

@@ -36,7 +36,7 @@ func NewOAuth2App(env Env) oauth2connect.App {
 func (app *oauth2App) InitOAuth2(mattermostUserID string) (url string, err error) {
 	user, err := app.Store.LoadUser(mattermostUserID)
 	if err == nil {
-		return "", errors.Errorf("User is already connected to %s", user.Remote.Mail)
+		return "", fmt.Errorf("User is already connected to %s", user.Remote.Mail)
 	}
 
 	conf := app.Remote.NewOAuth2Config()
@@ -83,11 +83,11 @@ func (app *oauth2App) CompleteOAuth2(authedUserID, code, state string) error {
 		user, userErr := app.PluginAPI.GetMattermostUser(uid)
 		if userErr == nil {
 			app.Poster.DM(authedUserID, RemoteUserAlreadyConnected, config.ApplicationName, me.Mail, config.CommandTrigger, user.Username)
-			return errors.Errorf(RemoteUserAlreadyConnected, config.ApplicationName, me.Mail, config.CommandTrigger, user.Username)
+			return fmt.Errorf(RemoteUserAlreadyConnected, config.ApplicationName, me.Mail, config.CommandTrigger, user.Username)
 		} else {
 			// Couldn't fetch connected MM account. Reject connect attempt.
 			app.Poster.DM(authedUserID, RemoteUserAlreadyConnectedNotFound, config.ApplicationName, me.Mail)
-			return errors.Errorf(RemoteUserAlreadyConnectedNotFound, config.ApplicationName, me.Mail)
+			return fmt.Errorf(RemoteUserAlreadyConnectedNotFound, config.ApplicationName, me.Mail)
 		}
 	}
 

server/mscalendar/settings.go

@@ -18,7 +18,7 @@ func (c *mscalendar) PrintSettings(userID string) {
 func (c *mscalendar) ClearSettingsPosts(userID string) {
 	err := c.SettingsPanel.Clear(userID)
 	if err != nil {
-		c.Logger.Warnf("error clearing settings posts, " + err.Error())
+		c.Logger.Warnf("Error clearing settings posts. err=%v", err)
 	}
 }
 

server/mscalendar/settings_daily_summary.go

@@ -134,7 +134,7 @@ func (s *dailySummarySetting) GetSlackAttachments(userID, settingHandler string,
 
 	timezone, err := s.getTimezone(userID)
 	if err != nil {
-		return nil, errors.New("could not load the timezone from Microsoft, err= " + err.Error())
+		return nil, fmt.Errorf("could not load the timezone from Microsoft. err=%v", err)
 	}
 	fullTime = fullTime + " " + timezone
 

server/mscalendar/user.go

@@ -138,7 +138,7 @@ func (m *mscalendar) DisconnectUser(mattermostUserID string) error {
 
 		err = m.client.DeleteSubscription(eventSubscriptionID)
 		if err != nil {
-			m.Logger.Errorf("failed to delete remote subscription %s: %v", eventSubscriptionID, err)
+			m.Logger.Warnf("failed to delete remote subscription %s. err=%v", eventSubscriptionID, err)
 		}
 	}
 

server/mscalendar/welcomer.go

@@ -74,7 +74,7 @@ func (bot *mscBot) Welcome(userID string) error {
 func (bot *mscBot) AfterSuccessfullyConnect(userID, userLogin string) error {
 	postID, err := bot.Store.DeleteUserWelcomePost(userID)
 	if err != nil {
-		bot.Errorf("error deleting user welcom post id, err=" + err.Error())
+		bot.Errorf("error deleting user's welcome post id, err=%v", err)
 	}
 	if postID != "" {
 		post := &model.Post{
@@ -137,7 +137,7 @@ func (bot *mscBot) cleanWelcomePost(mattermostUserID string) error {
 	if postID != "" {
 		err = bot.DeletePost(postID)
 		if err != nil {
-			bot.Errorf(err.Error())
+			bot.Errorf("Error deleting post. err=%v", err)
 		}
 	}
 	return nil