pkg/services: Check errors (#19712)

* pkg/services: Check errors
* pkg/services: Don't treat context.Canceled|context.DeadlineExceeded as error

pkg/services/alerting/conditions/query_test.go

@@ -23,7 +23,8 @@ func TestQueryCondition(t *testing.T) {
 			ctx.evaluator = `{"type": "gt", "params": [100]}`
 
 			Convey("Can read query condition from json model", func() {
-				ctx.exec()
+				_, err := ctx.exec()
+				So(err, ShouldBeNil)
 
 				So(ctx.condition.Query.From, ShouldEqual, "5m")
 				So(ctx.condition.Query.To, ShouldEqual, "now")

pkg/services/alerting/engine.go

@@ -15,6 +15,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/rendering"
 	"github.com/grafana/grafana/pkg/setting"
 	"golang.org/x/sync/errgroup"
+	"golang.org/x/xerrors"
 )
 
 // AlertEngine is the background process that
@@ -210,7 +211,16 @@ func (e *AlertEngine) processJob(attemptID int, attemptChan chan int, cancelChan
 		// dont reuse the evalContext and get its own context.
 		evalContext.Ctx = resultHandleCtx
 		evalContext.Rule.State = evalContext.GetNewState()
-		e.resultHandler.handle(evalContext)
+		if err := e.resultHandler.handle(evalContext); err != nil {
+			if xerrors.Is(err, context.Canceled) {
+				e.log.Debug("Result handler returned context.Canceled")
+			} else if xerrors.Is(err, context.DeadlineExceeded) {
+				e.log.Debug("Result handler returned context.DeadlineExceeded")
+			} else {
+				e.log.Error("Failed to handle result", "err", err)
+			}
+		}
+
 		span.Finish()
 		e.log.Debug("Job Execution completed", "timeMs", evalContext.GetDurationMs(), "alertId", evalContext.Rule.ID, "name", evalContext.Rule.Name, "firing", evalContext.Firing, "attemptID", attemptID)
 		close(attemptChan)

pkg/services/alerting/engine_integration_test.go

@@ -18,7 +18,8 @@ import (
 func TestEngineTimeouts(t *testing.T) {
 	Convey("Alerting engine timeout tests", t, func() {
 		engine := &AlertEngine{}
-		engine.Init()
+		err := engine.Init()
+		So(err, ShouldBeNil)
 		setting.AlertingNotificationTimeout = 30 * time.Second
 		setting.AlertingMaxAttempts = 3
 		engine.resultHandler = &FakeResultHandler{}
@@ -36,7 +37,8 @@ func TestEngineTimeouts(t *testing.T) {
 				engine.evalHandler = evalHandler
 				engine.resultHandler = resultHandler
 
-				engine.processJobWithRetry(context.TODO(), job)
+				err := engine.processJobWithRetry(context.TODO(), job)
+				So(err, ShouldBeNil)
 
 				So(evalHandler.EvalSucceed, ShouldEqual, true)
 				So(resultHandler.ResultHandleSucceed, ShouldEqual, true)

pkg/services/alerting/engine_test.go

@@ -40,7 +40,8 @@ func (handler *FakeResultHandler) handle(evalContext *EvalContext) error {
 func TestEngineProcessJob(t *testing.T) {
 	Convey("Alerting engine job processing", t, func() {
 		engine := &AlertEngine{}
-		engine.Init()
+		err := engine.Init()
+		So(err, ShouldBeNil)
 		setting.AlertingEvaluationTimeout = 30 * time.Second
 		setting.AlertingNotificationTimeout = 30 * time.Second
 		setting.AlertingMaxAttempts = 3
@@ -99,7 +100,8 @@ func TestEngineProcessJob(t *testing.T) {
 				evalHandler := NewFakeEvalHandler(0)
 				engine.evalHandler = evalHandler
 
-				engine.processJobWithRetry(context.TODO(), job)
+				err := engine.processJobWithRetry(context.TODO(), job)
+				So(err, ShouldBeNil)
 				So(evalHandler.CallNb, ShouldEqual, expectedAttempts)
 			})
 
@@ -108,7 +110,8 @@ func TestEngineProcessJob(t *testing.T) {
 				evalHandler := NewFakeEvalHandler(1)
 				engine.evalHandler = evalHandler
 
-				engine.processJobWithRetry(context.TODO(), job)
+				err := engine.processJobWithRetry(context.TODO(), job)
+				So(err, ShouldBeNil)
 				So(evalHandler.CallNb, ShouldEqual, expectedAttempts)
 			})
 
@@ -117,7 +120,8 @@ func TestEngineProcessJob(t *testing.T) {
 				evalHandler := NewFakeEvalHandler(expectedAttempts)
 				engine.evalHandler = evalHandler
 
-				engine.processJobWithRetry(context.TODO(), job)
+				err := engine.processJobWithRetry(context.TODO(), job)
+				So(err, ShouldBeNil)
 				So(evalHandler.CallNb, ShouldEqual, expectedAttempts)
 			})
 		})

pkg/services/alerting/notifiers/telegram.go

@@ -89,19 +89,20 @@ func NewTelegramNotifier(model *models.AlertNotification) (alerting.Notifier, er
 	}, nil
 }
 
-func (tn *TelegramNotifier) buildMessage(evalContext *alerting.EvalContext, sendImageInline bool) *models.SendWebhookSync {
+func (tn *TelegramNotifier) buildMessage(evalContext *alerting.EvalContext, sendImageInline bool) (*models.SendWebhookSync, error) {
 	if sendImageInline {
 		cmd, err := tn.buildMessageInlineImage(evalContext)
 		if err == nil {
-			return cmd
+			return cmd, nil
 		}
+
 		tn.log.Error("Could not generate Telegram message with inline image.", "err", err)
 	}
 
 	return tn.buildMessageLinkedImage(evalContext)
 }
 
-func (tn *TelegramNotifier) buildMessageLinkedImage(evalContext *alerting.EvalContext) *models.SendWebhookSync {
+func (tn *TelegramNotifier) buildMessageLinkedImage(evalContext *alerting.EvalContext) (*models.SendWebhookSync, error) {
 	message := fmt.Sprintf("<b>%s</b>\nState: %s\nMessage: %s\n", evalContext.GetNotificationTitle(), evalContext.Rule.Name, evalContext.Rule.Message)
 
 	ruleURL, err := evalContext.GetRuleURL()
@@ -118,11 +119,17 @@ func (tn *TelegramNotifier) buildMessageLinkedImage(evalContext *alerting.EvalCo
 		message = message + fmt.Sprintf("\n<i>Metrics:</i>%s", metrics)
 	}
 
-	cmd := tn.generateTelegramCmd(message, "text", "sendMessage", func(w *multipart.Writer) {
-		fw, _ := w.CreateFormField("parse_mode")
-		fw.Write([]byte("html"))
+	return tn.generateTelegramCmd(message, "text", "sendMessage", func(w *multipart.Writer) {
+		fw, err := w.CreateFormField("parse_mode")
+		if err != nil {
+			tn.log.Error("Failed to create form file", "err", err)
+			return
+		}
+
+		if _, err := fw.Write([]byte("html")); err != nil {
+			tn.log.Error("Failed to write to form field", "err", err)
+		}
 	})
-	return cmd
 }
 
 func (tn *TelegramNotifier) buildMessageInlineImage(evalContext *alerting.EvalContext) (*models.SendWebhookSync, error) {
@@ -149,22 +156,38 @@ func (tn *TelegramNotifier) buildMessageInlineImage(evalContext *alerting.EvalCo
 	metrics := generateMetricsMessage(evalContext)
 	message := generateImageCaption(evalContext, ruleURL, metrics)
 
-	cmd := tn.generateTelegramCmd(message, "caption", "sendPhoto", func(w *multipart.Writer) {
-		fw, _ := w.CreateFormFile("photo", evalContext.ImageOnDiskPath)
-		io.Copy(fw, imageFile)
+	return tn.generateTelegramCmd(message, "caption", "sendPhoto", func(w *multipart.Writer) {
+		fw, err := w.CreateFormFile("photo", evalContext.ImageOnDiskPath)
+		if err != nil {
+			tn.log.Error("Failed to create form file", "err", err)
+			return
+		}
+
+		if _, err := io.Copy(fw, imageFile); err != nil {
+			tn.log.Error("Failed to write to form file", "err", err)
+		}
 	})
-	return cmd, nil
 }
 
-func (tn *TelegramNotifier) generateTelegramCmd(message string, messageField string, apiAction string, extraConf func(writer *multipart.Writer)) *models.SendWebhookSync {
+func (tn *TelegramNotifier) generateTelegramCmd(message string, messageField string, apiAction string, extraConf func(writer *multipart.Writer)) (*models.SendWebhookSync, error) {
 	var body bytes.Buffer
 	w := multipart.NewWriter(&body)
 
-	fw, _ := w.CreateFormField("chat_id")
-	fw.Write([]byte(tn.ChatID))
+	fw, err := w.CreateFormField("chat_id")
+	if err != nil {
+		return nil, err
+	}
+	if _, err := fw.Write([]byte(tn.ChatID)); err != nil {
+		return nil, err
+	}
 
-	fw, _ = w.CreateFormField(messageField)
-	fw.Write([]byte(message))
+	fw, err = w.CreateFormField(messageField)
+	if err != nil {
+		return nil, err
+	}
+	if _, err := fw.Write([]byte(message)); err != nil {
+		return nil, err
+	}
 
 	extraConf(w)
 
@@ -181,7 +204,7 @@ func (tn *TelegramNotifier) generateTelegramCmd(message string, messageField str
 			"Content-Type": w.FormDataContentType(),
 		},
 	}
-	return cmd
+	return cmd, nil
 }
 
 func generateMetricsMessage(evalContext *alerting.EvalContext) string {
@@ -232,10 +255,14 @@ func appendIfPossible(message string, extra string, sizeLimit int) string {
 // Notify send an alert notification to Telegram.
 func (tn *TelegramNotifier) Notify(evalContext *alerting.EvalContext) error {
 	var cmd *models.SendWebhookSync
+	var err error
 	if evalContext.ImagePublicURL == "" && tn.UploadImage {
-		cmd = tn.buildMessage(evalContext, true)
+		cmd, err = tn.buildMessage(evalContext, true)
 	} else {
-		cmd = tn.buildMessage(evalContext, false)
+		cmd, err = tn.buildMessage(evalContext, false)
+	}
+	if err != nil {
+		return err
 	}
 
 	if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil {

pkg/services/alerting/result_handler.go

@@ -1,6 +1,7 @@
 package alerting
 
 import (
+	"context"
 	"time"
 
 	"github.com/grafana/grafana/pkg/bus"
@@ -8,6 +9,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/models"
+	"golang.org/x/xerrors"
 
 	"github.com/grafana/grafana/pkg/services/annotations"
 	"github.com/grafana/grafana/pkg/services/rendering"
@@ -98,6 +100,15 @@ func (handler *defaultResultHandler) handle(evalContext *EvalContext) error {
 		}
 	}
 
-	handler.notifier.SendIfNeeded(evalContext)
+	if err := handler.notifier.SendIfNeeded(evalContext); err != nil {
+		if xerrors.Is(err, context.Canceled) {
+			handler.log.Debug("handler.notifier.SendIfNeeded returned context.Canceled")
+		} else if xerrors.Is(err, context.DeadlineExceeded) {
+			handler.log.Debug("handler.notifier.SendIfNeeded returned context.DeadlineExceeded")
+		} else {
+			handler.log.Error("handler.notifier.SendIfNeeded failed", "err", err)
+		}
+	}
+
 	return nil
 }

pkg/services/auth/auth_token.go

@@ -201,7 +201,10 @@ func (s *UserAuthTokenService) TryRotateToken(ctx context.Context, token *models
 		return false, nil
 	}
 
-	model := userAuthTokenFromUserToken(token)
+	model, err := userAuthTokenFromUserToken(token)
+	if err != nil {
+		return false, err
+	}
 
 	now := getTime()
 
@@ -262,7 +265,9 @@ func (s *UserAuthTokenService) TryRotateToken(ctx context.Context, token *models
 	s.log.Debug("auth token rotated", "affected", affected, "auth_token_id", model.Id, "userId", model.UserId)
 	if affected > 0 {
 		model.UnhashedToken = newToken
-		model.toUserToken(token)
+		if err := model.toUserToken(token); err != nil {
+			return false, err
+		}
 		return true, nil
 	}
 
@@ -274,10 +279,12 @@ func (s *UserAuthTokenService) RevokeToken(ctx context.Context, token *models.Us
 		return models.ErrUserTokenNotFound
 	}
 
-	model := userAuthTokenFromUserToken(token)
+	model, err := userAuthTokenFromUserToken(token)
+	if err != nil {
+		return err
+	}
 
 	var rowsAffected int64
-	var err error
 	err = s.SQLStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
 		rowsAffected, err = dbSession.Delete(model)
 		return err
@@ -347,7 +354,6 @@ func (s *UserAuthTokenService) BatchRevokeAllUserTokens(ctx context.Context, use
 }
 
 func (s *UserAuthTokenService) GetUserToken(ctx context.Context, userId, userTokenId int64) (*models.UserToken, error) {
-
 	var result models.UserToken
 	err := s.SQLStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
 		var token userAuthToken
@@ -360,8 +366,7 @@ func (s *UserAuthTokenService) GetUserToken(ctx context.Context, userId, userTok
 			return models.ErrUserTokenNotFound
 		}
 
-		token.toUserToken(&result)
-		return nil
+		return token.toUserToken(&result)
 	})
 
 	return &result, err
@@ -384,7 +389,9 @@ func (s *UserAuthTokenService) GetUserTokens(ctx context.Context, userId int64)
 
 		for _, token := range tokens {
 			var userToken models.UserToken
-			token.toUserToken(&userToken)
+			if err := token.toUserToken(&userToken); err != nil {
+				return err
+			}
 			result = append(result, &userToken)
 		}
 

pkg/services/auth/auth_token_test.go

@@ -124,7 +124,8 @@ func TestUserAuthToken(t *testing.T) {
 					for i := 0; i < 3; i++ {
 						userId := userID + int64(i+1)
 						userIds = append(userIds, userId)
-						userAuthTokenService.CreateToken(context.Background(), userId, "192.168.10.11:1234", "some user agent")
+						_, err := userAuthTokenService.CreateToken(context.Background(), userId, "192.168.10.11:1234", "some user agent")
+						So(err, ShouldBeNil)
 					}
 
 					err := userAuthTokenService.BatchRevokeAllUserTokens(context.Background(), userIds)
@@ -441,7 +442,8 @@ func TestUserAuthToken(t *testing.T) {
 			utMap := utJSON.MustMap()
 
 			var uat userAuthToken
-			uat.fromUserToken(&ut)
+			err = uat.fromUserToken(&ut)
+			So(err, ShouldBeNil)
 			uatBytes, err := json.Marshal(uat)
 			So(err, ShouldBeNil)
 			uatJSON, err := simplejson.NewJson(uatBytes)

pkg/services/auth/model.go

@@ -21,10 +21,10 @@ type userAuthToken struct {
 	UnhashedToken string `xorm:"-"`
 }
 
-func userAuthTokenFromUserToken(ut *models.UserToken) *userAuthToken {
+func userAuthTokenFromUserToken(ut *models.UserToken) (*userAuthToken, error) {
 	var uat userAuthToken
-	uat.fromUserToken(ut)
-	return &uat
+	err := uat.fromUserToken(ut)
+	return &uat, err
 }
 
 func (uat *userAuthToken) fromUserToken(ut *models.UserToken) error {

pkg/services/auth/token_cleanup.go

@@ -13,9 +13,10 @@ func (srv *UserAuthTokenService) Run(ctx context.Context) error {
 	maxLifetime := time.Duration(srv.Cfg.LoginMaxLifetimeDays) * 24 * time.Hour
 
 	err := srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() {
-		srv.deleteExpiredTokens(ctx, maxInactiveLifetime, maxLifetime)
+		if _, err := srv.deleteExpiredTokens(ctx, maxInactiveLifetime, maxLifetime); err != nil {
+			srv.log.Error("An error occurred while deleting expired tokens", "err", err)
+		}
 	})
-
 	if err != nil {
 		srv.log.Error("failed to lock and execute cleanup of expired auth token", "error", err)
 	}
@@ -24,9 +25,10 @@ func (srv *UserAuthTokenService) Run(ctx context.Context) error {
 		select {
 		case <-ticker.C:
 			err := srv.ServerLockService.LockAndExecute(ctx, "cleanup expired auth tokens", time.Hour*12, func() {
-				srv.deleteExpiredTokens(ctx, maxInactiveLifetime, maxLifetime)
+				if _, err := srv.deleteExpiredTokens(ctx, maxInactiveLifetime, maxLifetime); err != nil {
+					srv.log.Error("An error occurred while deleting expired tokens", "err", err)
+				}
 			})
-
 			if err != nil {
 				srv.log.Error("failed to lock and execute cleanup of expired auth token", "error", err)
 			}

pkg/services/cleanup/cleanup.go

@@ -40,10 +40,13 @@ func (srv *CleanUpService) Run(ctx context.Context) error {
 			srv.cleanUpTmpFiles()
 			srv.deleteExpiredSnapshots()
 			srv.deleteExpiredDashboardVersions()
-			srv.ServerLockService.LockAndExecute(ctx, "delete old login attempts", time.Minute*10, func() {
-				srv.deleteOldLoginAttempts()
-			})
-
+			err := srv.ServerLockService.LockAndExecute(ctx, "delete old login attempts",
+				time.Minute*10, func() {
+					srv.deleteOldLoginAttempts()
+				})
+			if err != nil {
+				srv.log.Error("failed to lock and execute cleanup of old login attempts", "error", err)
+			}
 		case <-ctx.Done():
 			return ctx.Err()
 		}

pkg/services/notifications/codes.go

@@ -16,7 +16,7 @@ const timeLimitCodeLength = 12 + 6 + 40
 
 // create a time limit code
 // code format: 12 length date time string + 6 minutes string + 40 sha1 encoded string
-func createTimeLimitCode(data string, minutes int, startInf interface{}) string {
+func createTimeLimitCode(data string, minutes int, startInf interface{}) (string, error) {
 	format := "200601021504"
 
 	var start, end time.Time
@@ -38,17 +38,20 @@ func createTimeLimitCode(data string, minutes int, startInf interface{}) string
 
 	// create sha1 encode string
 	sh := sha1.New()
-	sh.Write([]byte(data + setting.SecretKey + startStr + endStr + com.ToStr(minutes)))
+	if _, err := sh.Write([]byte(data + setting.SecretKey + startStr + endStr +
+		com.ToStr(minutes))); err != nil {
+		return "", err
+	}
 	encoded := hex.EncodeToString(sh.Sum(nil))
 
 	code := fmt.Sprintf("%s%06d%s", startStr, minutes, encoded)
-	return code
+	return code, nil
 }
 
 // verify time limit code
-func validateUserEmailCode(user *m.User, code string) bool {
+func validateUserEmailCode(user *m.User, code string) (bool, error) {
 	if len(code) <= 18 {
-		return false
+		return false, nil
 	}
 
 	minutes := setting.EmailCodeValidMinutes
@@ -63,18 +66,21 @@ func validateUserEmailCode(user *m.User, code string) bool {
 
 	// right active code
 	data := com.ToStr(user.Id) + user.Email + user.Login + user.Password + user.Rands
-	retCode := createTimeLimitCode(data, minutes, start)
+	retCode, err := createTimeLimitCode(data, minutes, start)
+	if err != nil {
+		return false, err
+	}
 	fmt.Printf("code : %s\ncode2: %s", retCode, code)
 	if retCode == code && minutes > 0 {
 		// check time is expired or not
 		before, _ := time.ParseInLocation("200601021504", start, time.Local)
 		now := time.Now()
 		if before.Add(time.Minute*time.Duration(minutes)).Unix() > now.Unix() {
-			return true
+			return true, nil
 		}
 	}
 
-	return false
+	return false, nil
 }
 
 func getLoginForEmailCode(code string) string {
@@ -88,12 +94,15 @@ func getLoginForEmailCode(code string) string {
 	return string(b)
 }
 
-func createUserEmailCode(u *m.User, startInf interface{}) string {
+func createUserEmailCode(u *m.User, startInf interface{}) (string, error) {
 	minutes := setting.EmailCodeValidMinutes
 	data := com.ToStr(u.Id) + u.Email + u.Login + u.Password + u.Rands
-	code := createTimeLimitCode(data, minutes, startInf)
+	code, err := createTimeLimitCode(data, minutes, startInf)
+	if err != nil {
+		return "", err
+	}
 
 	// add tail hex username
 	code += hex.EncodeToString([]byte(u.Login))
-	return code
+	return code, nil
 }

pkg/services/notifications/codes_test.go

@@ -14,7 +14,8 @@ func TestEmailCodes(t *testing.T) {
 		setting.EmailCodeValidMinutes = 120
 
 		user := &m.User{Id: 10, Email: "t@a.com", Login: "asd", Password: "1", Rands: "2"}
-		code := createUserEmailCode(user, nil)
+		code, err := createUserEmailCode(user, nil)
+		So(err, ShouldBeNil)
 
 		Convey("getLoginForCode should return login", func() {
 			login := getLoginForEmailCode(code)
@@ -22,12 +23,16 @@ func TestEmailCodes(t *testing.T) {
 		})
 
 		Convey("Can verify valid code", func() {
-			So(validateUserEmailCode(user, code), ShouldBeTrue)
+			isValid, err := validateUserEmailCode(user, code)
+			So(err, ShouldBeNil)
+			So(isValid, ShouldBeTrue)
 		})
 
 		Convey("Cannot verify in-valid code", func() {
 			code = "ASD"
-			So(validateUserEmailCode(user, code), ShouldBeFalse)
+			isValid, err := validateUserEmailCode(user, code)
+			So(err, ShouldBeNil)
+			So(isValid, ShouldBeFalse)
 		})
 
 	})

pkg/services/notifications/notifications.go

@@ -147,11 +147,15 @@ func (ns *NotificationService) sendEmailCommandHandler(cmd *m.SendEmailCommand)
 }
 
 func (ns *NotificationService) sendResetPasswordEmail(cmd *m.SendResetPasswordEmailCommand) error {
+	code, err := createUserEmailCode(cmd.User, nil)
+	if err != nil {
+		return err
+	}
 	return ns.sendEmailCommandHandler(&m.SendEmailCommand{
 		To:       []string{cmd.User.Email},
 		Template: tmplResetPassword,
 		Data: map[string]interface{}{
-			"Code": createUserEmailCode(cmd.User, nil),
+			"Code": code,
 			"Name": cmd.User.NameOrFallback(),
 		},
 	})
@@ -168,7 +172,11 @@ func (ns *NotificationService) validateResetPasswordCode(query *m.ValidateResetP
 		return err
 	}
 
-	if !validateUserEmailCode(userQuery.Result, query.Code) {
+	validEmailCode, err := validateUserEmailCode(userQuery.Result, query.Code)
+	if err != nil {
+		return err
+	}
+	if !validEmailCode {
 		return m.ErrInvalidEmailCode
 	}
 

pkg/services/notifications/send_email_integration_test.go

@@ -61,7 +61,8 @@ func TestEmailIntegrationTest(t *testing.T) {
 			sentMsg := <-ns.mailQueue
 			So(sentMsg.From, ShouldEqual, "Grafana Admin <from@address.com>")
 			So(sentMsg.To[0], ShouldEqual, "asdf@asdf.com")
-			ioutil.WriteFile("../../../tmp/test_email.html", []byte(sentMsg.Body), 0777)
+			err = ioutil.WriteFile("../../../tmp/test_email.html", []byte(sentMsg.Body), 0777)
+			So(err, ShouldBeNil)
 		})
 	})
 }

pkg/services/notifications/webhook.go

@@ -77,7 +77,9 @@ func (ns *NotificationService) sendWebRequestSync(ctx context.Context, webhook *
 
 	if resp.StatusCode/100 == 2 {
 		// flushing the body enables the transport to reuse the same connection
-		io.Copy(ioutil.Discard, resp.Body)
+		if _, err := io.Copy(ioutil.Discard, resp.Body); err != nil {
+			ns.log.Error("Failed to copy resp.Body to ioutil.Discard", "err", err)
+		}
 		return nil
 	}
 

pkg/services/sqlstore/alert.go

@@ -151,10 +151,16 @@ func HandleAlertsQuery(query *m.GetAlertsQuery) error {
 
 func deleteAlertDefinition(dashboardId int64, sess *DBSession) error {
 	alerts := make([]*m.Alert, 0)
-	sess.Where("dashboard_id = ?", dashboardId).Find(&alerts)
+	if err := sess.Where("dashboard_id = ?", dashboardId).Find(&alerts); err != nil {
+		return err
+	}
 
 	for _, alert := range alerts {
-		deleteAlertByIdInternal(alert.Id, "Dashboard deleted", sess)
+		if err := deleteAlertByIdInternal(alert.Id, "Dashboard deleted", sess); err != nil {
+			// If we return an error, the current transaction gets rolled back, so no use
+			// trying to delete more
+			return err
+		}
 	}
 
 	return nil
@@ -251,7 +257,11 @@ func deleteMissingAlerts(alerts []*m.Alert, cmd *m.SaveAlertsCommand, sess *DBSe
 		}
 
 		if missing {
-			deleteAlertByIdInternal(missingAlert.Id, "Removed from dashboard", sess)
+			if err := deleteAlertByIdInternal(missingAlert.Id, "Removed from dashboard", sess); err != nil {
+				// No use trying to delete more, since we're in a transaction and it will be
+				// rolled back on error.
+				return err
+			}
 		}
 	}
 

pkg/services/sqlstore/alert_notification.go

@@ -392,10 +392,11 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetA
 	return inTransactionCtx(ctx, func(sess *DBSession) error {
 		version := cmd.Version
 		var current m.AlertNotificationState
-		sess.ID(cmd.Id).Get(&current)
+		if _, err := sess.ID(cmd.Id).Get(&current); err != nil {
+			return err
+		}
 
 		newVersion := cmd.Version + 1
-
 		sql := `UPDATE alert_notification_state SET
 			state = ?,
 			version = ?,
@@ -404,7 +405,6 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetA
 			id = ?`
 
 		_, err := sess.Exec(sql, m.AlertNotificationStateCompleted, newVersion, timeNow().Unix(), cmd.Id)
-
 		if err != nil {
 			return err
 		}

pkg/services/sqlstore/alert_test.go

@@ -73,7 +73,8 @@ func TestAlertingDataAccess(t *testing.T) {
 			stateDateBeforePause := alert.NewStateDate
 
 			Convey("can pause all alerts", func() {
-				pauseAllAlerts(true)
+				err := pauseAllAlerts(true)
+				So(err, ShouldBeNil)
 
 				Convey("cannot updated paused alert", func() {
 					cmd := &m.SetAlertStateCommand{
@@ -98,7 +99,8 @@ func TestAlertingDataAccess(t *testing.T) {
 				})
 
 				Convey("unpausing alerts should update their NewStateDate again", func() {
-					pauseAllAlerts(false)
+					err := pauseAllAlerts(false)
+					So(err, ShouldBeNil)
 					alert, _ = getAlertById(1)
 					stateDateAfterUnpause := alert.NewStateDate
 					So(stateDateBeforePause, ShouldHappenBefore, stateDateAfterUnpause)
@@ -241,13 +243,13 @@ func TestAlertingDataAccess(t *testing.T) {
 				UserId:      1,
 			}
 
-			SaveAlerts(&cmd)
+			err = SaveAlerts(&cmd)
+			So(err, ShouldBeNil)
 
 			err = DeleteDashboard(&m.DeleteDashboardCommand{
 				OrgId: 1,
 				Id:    testDash.Id,
 			})
-
 			So(err, ShouldBeNil)
 
 			Convey("Alerts should be removed", func() {
@@ -275,10 +277,12 @@ func TestPausingAlerts(t *testing.T) {
 		stateDateBeforePause := alert.NewStateDate
 		stateDateAfterPause := stateDateBeforePause
 		Convey("when paused", func() {
-			pauseAlert(testDash.OrgId, 1, true)
+			_, err := pauseAlert(testDash.OrgId, 1, true)
+			So(err, ShouldBeNil)
 
 			Convey("the NewStateDate should be updated", func() {
-				alert, _ := getAlertById(1)
+				alert, err := getAlertById(1)
+				So(err, ShouldBeNil)
 
 				stateDateAfterPause = alert.NewStateDate
 				So(stateDateBeforePause, ShouldHappenBefore, stateDateAfterPause)
@@ -286,10 +290,12 @@ func TestPausingAlerts(t *testing.T) {
 		})
 
 		Convey("when unpaused", func() {
-			pauseAlert(testDash.OrgId, 1, false)
+			_, err := pauseAlert(testDash.OrgId, 1, false)
+			So(err, ShouldBeNil)
 
 			Convey("the NewStateDate should be updated again", func() {
-				alert, _ := getAlertById(1)
+				alert, err := getAlertById(1)
+				So(err, ShouldBeNil)
 
 				stateDateAfterUnpause := alert.NewStateDate
 				So(stateDateAfterPause, ShouldHappenBefore, stateDateAfterUnpause)

pkg/services/sqlstore/dashboard_folder_test.go

@@ -38,7 +38,13 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 
 			Convey("and acl is set for dashboard folder", func() {
 				var otherUser int64 = 999
-				testHelperUpdateDashboardAcl(folder.Id, models.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT})
+				err := testHelperUpdateDashboardAcl(folder.Id, models.DashboardAcl{
+					DashboardId: folder.Id,
+					OrgId:       1,
+					UserId:      otherUser,
+					Permission:  models.PERMISSION_EDIT,
+				})
+				So(err, ShouldBeNil)
 
 				Convey("should not return folder", func() {
 					query := &search.FindPersistedDashboardsQuery{
@@ -46,14 +52,17 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 						OrgId:        1, DashboardIds: []int64{folder.Id, dashInRoot.Id},
 					}
 					err := SearchDashboards(query)
-
 					So(err, ShouldBeNil)
+
 					So(len(query.Result), ShouldEqual, 1)
 					So(query.Result[0].Id, ShouldEqual, dashInRoot.Id)
 				})
 
 				Convey("when the user is given permission", func() {
-					testHelperUpdateDashboardAcl(folder.Id, models.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: currentUser.Id, Permission: models.PERMISSION_EDIT})
+					err := testHelperUpdateDashboardAcl(folder.Id, models.DashboardAcl{
+						DashboardId: folder.Id, OrgId: 1, UserId: currentUser.Id, Permission: models.PERMISSION_EDIT,
+					})
+					So(err, ShouldBeNil)
 
 					Convey("should be able to access folder", func() {
 						query := &search.FindPersistedDashboardsQuery{
@@ -91,11 +100,17 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 
 			Convey("and acl is set for dashboard child and folder has all permissions removed", func() {
 				var otherUser int64 = 999
-				testHelperUpdateDashboardAcl(folder.Id)
-				testHelperUpdateDashboardAcl(childDash.Id, models.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT})
+				err := testHelperUpdateDashboardAcl(folder.Id)
+				So(err, ShouldBeNil)
+				err = testHelperUpdateDashboardAcl(childDash.Id, models.DashboardAcl{
+					DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT,
+				})
+				So(err, ShouldBeNil)
 
 				Convey("should not return folder or child", func() {
-					query := &search.FindPersistedDashboardsQuery{SignedInUser: &models.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: models.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}}
+					query := &search.FindPersistedDashboardsQuery{
+						SignedInUser: &models.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: models.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id},
+					}
 					err := SearchDashboards(query)
 					So(err, ShouldBeNil)
 					So(len(query.Result), ShouldEqual, 1)
@@ -103,7 +118,8 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("when the user is given permission to child", func() {
-					testHelperUpdateDashboardAcl(childDash.Id, models.DashboardAcl{DashboardId: childDash.Id, OrgId: 1, UserId: currentUser.Id, Permission: models.PERMISSION_EDIT})
+					err := testHelperUpdateDashboardAcl(childDash.Id, models.DashboardAcl{DashboardId: childDash.Id, OrgId: 1, UserId: currentUser.Id, Permission: models.PERMISSION_EDIT})
+					So(err, ShouldBeNil)
 
 					Convey("should be able to search for child dashboard but not folder", func() {
 						query := &search.FindPersistedDashboardsQuery{SignedInUser: &models.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: models.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}}
@@ -162,7 +178,10 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 
 			Convey("and acl is set for one dashboard folder", func() {
 				var otherUser int64 = 999
-				testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT})
+				err := testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{
+					DashboardId: folder1.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT,
+				})
+				So(err, ShouldBeNil)
 
 				Convey("and a dashboard is moved from folder without acl to the folder with an acl", func() {
 					moveDashboard(1, childDash2.Data, folder1.Id)
@@ -199,7 +218,11 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("and a dashboard with an acl is moved to the folder without an acl", func() {
-					testHelperUpdateDashboardAcl(childDash1.Id, models.DashboardAcl{DashboardId: childDash1.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT})
+					err := testHelperUpdateDashboardAcl(childDash1.Id, models.DashboardAcl{
+						DashboardId: childDash1.Id, OrgId: 1, UserId: otherUser, Permission: models.PERMISSION_EDIT,
+					})
+					So(err, ShouldBeNil)
+
 					moveDashboard(1, childDash1.Data, folder2.Id)
 
 					Convey("should return folder without acl but not the dashboard with acl", func() {
@@ -318,9 +341,12 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("Should have write access to one dashboard folder if default role changed to view for one folder", func() {
-					testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: editorUser.Id, Permission: models.PERMISSION_VIEW})
+					err := testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{
+						DashboardId: folder1.Id, OrgId: 1, UserId: editorUser.Id, Permission: models.PERMISSION_VIEW,
+					})
+					So(err, ShouldBeNil)
 
-					err := SearchDashboards(&query)
+					err = SearchDashboards(&query)
 					So(err, ShouldBeNil)
 
 					So(len(query.Result), ShouldEqual, 1)
@@ -379,9 +405,12 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("Should be able to get one dashboard folder if default role changed to edit for one folder", func() {
-					testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: models.PERMISSION_EDIT})
+					err := testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{
+						DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: models.PERMISSION_EDIT,
+					})
+					So(err, ShouldBeNil)
 
-					err := SearchDashboards(&query)
+					err = SearchDashboards(&query)
 					So(err, ShouldBeNil)
 
 					So(len(query.Result), ShouldEqual, 1)
@@ -407,7 +436,10 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("and admin permission is given for user with org role viewer in one dashboard folder", func() {
-					testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: models.PERMISSION_ADMIN})
+					err := testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{
+						DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: models.PERMISSION_ADMIN,
+					})
+					So(err, ShouldBeNil)
 
 					Convey("should have edit permission in folders", func() {
 						query := &models.HasEditPermissionInFoldersQuery{
@@ -420,7 +452,10 @@ func TestDashboardFolderDataAccess(t *testing.T) {
 				})
 
 				Convey("and edit permission is given for user with org role viewer in one dashboard folder", func() {
-					testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: models.PERMISSION_EDIT})
+					err := testHelperUpdateDashboardAcl(folder1.Id, models.DashboardAcl{
+						DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: models.PERMISSION_EDIT,
+					})
+					So(err, ShouldBeNil)
 
 					Convey("should have edit permission in folders", func() {
 						query := &models.HasEditPermissionInFoldersQuery{

pkg/services/sqlstore/dashboard_snapshot_test.go

@@ -138,7 +138,8 @@ func TestDeleteExpiredSnapshots(t *testing.T) {
 			OrgId:        1,
 			SignedInUser: &m.SignedInUser{OrgRole: m.ROLE_ADMIN},
 		}
-		SearchDashboardSnapshots(&query)
+		err = SearchDashboardSnapshots(&query)
+		So(err, ShouldBeNil)
 
 		So(len(query.Result), ShouldEqual, 1)
 		So(query.Result[0].Key, ShouldEqual, notExpiredsnapshot.Key)

pkg/services/sqlstore/dashboard_test.go

@@ -345,15 +345,17 @@ func TestDashboardDataAccess(t *testing.T) {
 
 			Convey("Given two dashboards, one is starred dashboard by user 10, other starred by user 1", func() {
 				starredDash := insertTestDashboard("starred dash", 1, 0, false)
-				StarDashboard(&m.StarDashboardCommand{
+				err := StarDashboard(&m.StarDashboardCommand{
 					DashboardId: starredDash.Id,
 					UserId:      10,
 				})
+				So(err, ShouldBeNil)
 
-				StarDashboard(&m.StarDashboardCommand{
+				err = StarDashboard(&m.StarDashboardCommand{
 					DashboardId: savedDash.Id,
 					UserId:      1,
 				})
+				So(err, ShouldBeNil)
 
 				Convey("Should be able to search for starred dashboards", func() {
 					query := search.FindPersistedDashboardsQuery{
@@ -439,7 +441,8 @@ func createUser(name string, role string, isAdmin bool) m.User {
 	So(err, ShouldBeNil)
 
 	q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id}
-	GetUserOrgList(&q1)
+	err = GetUserOrgList(&q1)
+	So(err, ShouldBeNil)
 	So(q1.Result[0].Role, ShouldEqual, role)
 
 	return currentUserCmd.Result

pkg/services/sqlstore/dashboard_version_test.go

@@ -122,7 +122,8 @@ func TestDeleteExpiredVersions(t *testing.T) {
 			So(err, ShouldBeNil)
 
 			query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1}
-			GetDashboardVersions(&query)
+			err = GetDashboardVersions(&query)
+			So(err, ShouldBeNil)
 
 			So(len(query.Result), ShouldEqual, versionsToKeep)
 			// Ensure latest versions were kept
@@ -137,7 +138,8 @@ func TestDeleteExpiredVersions(t *testing.T) {
 			So(err, ShouldBeNil)
 
 			query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1, Limit: versionsToWrite}
-			GetDashboardVersions(&query)
+			err = GetDashboardVersions(&query)
+			So(err, ShouldBeNil)
 
 			So(len(query.Result), ShouldEqual, versionsToWrite)
 		})
@@ -154,7 +156,8 @@ func TestDeleteExpiredVersions(t *testing.T) {
 			So(err, ShouldBeNil)
 
 			query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1, Limit: versionsToWriteBigNumber}
-			GetDashboardVersions(&query)
+			err = GetDashboardVersions(&query)
+			So(err, ShouldBeNil)
 
 			// Ensure we have at least versionsToKeep versions
 			So(len(query.Result), ShouldBeGreaterThanOrEqualTo, versionsToKeep)

pkg/services/sqlstore/datasource_test.go

@@ -145,7 +145,9 @@ func TestDataAccess(t *testing.T) {
 				err := DeleteDataSourceById(&models.DeleteDataSourceByIdCommand{Id: ds.Id, OrgId: ds.OrgId})
 				So(err, ShouldBeNil)
 
-				GetDataSources(&query)
+				err = GetDataSources(&query)
+				So(err, ShouldBeNil)
+
 				So(len(query.Result), ShouldEqual, 0)
 			})
 
@@ -153,7 +155,9 @@ func TestDataAccess(t *testing.T) {
 				err := DeleteDataSourceByName(&models.DeleteDataSourceByNameCommand{Name: ds.Name, OrgId: ds.OrgId})
 				So(err, ShouldBeNil)
 
-				GetDataSources(&query)
+				err = GetDataSources(&query)
+				So(err, ShouldBeNil)
+
 				So(len(query.Result), ShouldEqual, 0)
 			})
 
@@ -161,7 +165,9 @@ func TestDataAccess(t *testing.T) {
 				err := DeleteDataSourceById(&models.DeleteDataSourceByIdCommand{Id: ds.Id, OrgId: 123123})
 				So(err, ShouldBeNil)
 
-				GetDataSources(&query)
+				err = GetDataSources(&query)
+				So(err, ShouldBeNil)
+
 				So(len(query.Result), ShouldEqual, 1)
 			})
 		})

pkg/services/sqlstore/migrations/annotation_mig.go

@@ -155,6 +155,6 @@ func (m *AddMakeRegionSingleRowMigration) Exec(sess *xorm.Session, mg *Migrator)
 		}
 	}
 
-	sess.Exec("DELETE FROM annotation WHERE region_id > 0 AND id <> region_id")
-	return nil
+	_, err = sess.Exec("DELETE FROM annotation WHERE region_id > 0 AND id <> region_id")
+	return err
 }

pkg/services/sqlstore/migrations/migrations_test.go

@@ -25,7 +25,8 @@ func TestMigrations(t *testing.T) {
 			x, err := xorm.NewEngine(testDB.DriverName, testDB.ConnStr)
 			So(err, ShouldBeNil)
 
-			NewDialect(x).CleanDB()
+			err = NewDialect(x).CleanDB()
+			So(err, ShouldBeNil)
 
 			_, err = x.SQL(sql).Get(&r)
 			So(err, ShouldNotBeNil)

pkg/services/sqlstore/migrator/migrator.go

@@ -6,6 +6,7 @@ import (
 	_ "github.com/go-sql-driver/mysql"
 	"github.com/go-xorm/xorm"
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/util/errutil"
 	_ "github.com/lib/pq"
 	_ "github.com/mattn/go-sqlite3"
 )
@@ -99,12 +100,14 @@ func (mg *Migrator) Start() error {
 			if err != nil {
 				mg.Logger.Error("Exec failed", "error", err, "sql", sql)
 				record.Error = err.Error()
-				sess.Insert(&record)
+				if _, err := sess.Insert(&record); err != nil {
+					return err
+				}
 				return err
 			}
 			record.Success = true
-			sess.Insert(&record)
-			return nil
+			_, err = sess.Insert(&record)
+			return err
 		})
 
 		if err != nil {
@@ -158,21 +161,22 @@ func (mg *Migrator) exec(m Migration, sess *xorm.Session) error {
 type dbTransactionFunc func(sess *xorm.Session) error
 
 func (mg *Migrator) inTransaction(callback dbTransactionFunc) error {
-	var err error
-
 	sess := mg.x.NewSession()
 	defer sess.Close()
 
-	if err = sess.Begin(); err != nil {
+	if err := sess.Begin(); err != nil {
 		return err
 	}
 
-	err = callback(sess)
+	if err := callback(sess); err != nil {
+		if rollErr := sess.Rollback(); err != rollErr {
+			return errutil.Wrapf(err, "Failed to roll back transaction due to error: %s", rollErr)
+		}
 
-	if err != nil {
-		sess.Rollback()
 		return err
-	} else if err = sess.Commit(); err != nil {
+	}
+
+	if err := sess.Commit(); err != nil {
 		return err
 	}
 

pkg/services/sqlstore/org_test.go

@@ -51,8 +51,10 @@ func TestAccountDataAccess(t *testing.T) {
 
 				q1 := m.GetUserOrgListQuery{UserId: ac1cmd.Result.Id}
 				q2 := m.GetUserOrgListQuery{UserId: ac2cmd.Result.Id}
-				GetUserOrgList(&q1)
-				GetUserOrgList(&q2)
+				err = GetUserOrgList(&q1)
+				So(err, ShouldBeNil)
+				err = GetUserOrgList(&q2)
+				So(err, ShouldBeNil)
 
 				So(q1.Result[0].OrgId, ShouldEqual, q2.Result[0].OrgId)
 				So(q1.Result[0].Role, ShouldEqual, "Viewer")

pkg/services/sqlstore/preferences_test.go

@@ -23,67 +23,91 @@ func TestPreferencesDataAccess(t *testing.T) {
 		})
 
 		Convey("GetPreferencesWithDefaults with saved org and user home dashboard should return user home dashboard", func() {
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			err := SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			So(err, ShouldBeNil)
 
 			query := &models.GetPreferencesWithDefaultsQuery{User: &models.SignedInUser{OrgId: 1, UserId: 1}}
-			err := GetPreferencesWithDefaults(query)
+			err = GetPreferencesWithDefaults(query)
 			So(err, ShouldBeNil)
 			So(query.Result.HomeDashboardId, ShouldEqual, 4)
 		})
 
 		Convey("GetPreferencesWithDefaults with saved org and other user home dashboard should return org home dashboard", func() {
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			err := SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			So(err, ShouldBeNil)
 
 			query := &models.GetPreferencesWithDefaultsQuery{User: &models.SignedInUser{OrgId: 1, UserId: 2}}
-			err := GetPreferencesWithDefaults(query)
+			err = GetPreferencesWithDefaults(query)
 			So(err, ShouldBeNil)
 			So(query.Result.HomeDashboardId, ShouldEqual, 1)
 		})
 
 		Convey("GetPreferencesWithDefaults with saved org and teams home dashboard should return last team home dashboard", func() {
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
+			err := SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
+			So(err, ShouldBeNil)
 
-			query := &models.GetPreferencesWithDefaultsQuery{User: &models.SignedInUser{OrgId: 1, Teams: []int64{2, 3}}}
-			err := GetPreferencesWithDefaults(query)
+			query := &models.GetPreferencesWithDefaultsQuery{
+				User: &models.SignedInUser{OrgId: 1, Teams: []int64{2, 3}},
+			}
+			err = GetPreferencesWithDefaults(query)
 			So(err, ShouldBeNil)
 			So(query.Result.HomeDashboardId, ShouldEqual, 3)
 		})
 
 		Convey("GetPreferencesWithDefaults with saved org and other teams home dashboard should return org home dashboard", func() {
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
+			err := SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
+			So(err, ShouldBeNil)
 
 			query := &models.GetPreferencesWithDefaultsQuery{User: &models.SignedInUser{OrgId: 1}}
-			err := GetPreferencesWithDefaults(query)
+			err = GetPreferencesWithDefaults(query)
 			So(err, ShouldBeNil)
 			So(query.Result.HomeDashboardId, ShouldEqual, 1)
 		})
 
 		Convey("GetPreferencesWithDefaults with saved org, teams and user home dashboard should return user home dashboard", func() {
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			err := SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			So(err, ShouldBeNil)
 
-			query := &models.GetPreferencesWithDefaultsQuery{User: &models.SignedInUser{OrgId: 1, UserId: 1, Teams: []int64{2, 3}}}
-			err := GetPreferencesWithDefaults(query)
+			query := &models.GetPreferencesWithDefaultsQuery{
+				User: &models.SignedInUser{OrgId: 1, UserId: 1, Teams: []int64{2, 3}},
+			}
+			err = GetPreferencesWithDefaults(query)
 			So(err, ShouldBeNil)
 			So(query.Result.HomeDashboardId, ShouldEqual, 4)
 		})
 
 		Convey("GetPreferencesWithDefaults with saved org, other teams and user home dashboard should return org home dashboard", func() {
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
-			SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			err := SavePreferences(&models.SavePreferencesCommand{OrgId: 1, HomeDashboardId: 1})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 2, HomeDashboardId: 2})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, TeamId: 3, HomeDashboardId: 3})
+			So(err, ShouldBeNil)
+			err = SavePreferences(&models.SavePreferencesCommand{OrgId: 1, UserId: 1, HomeDashboardId: 4})
+			So(err, ShouldBeNil)
 
-			query := &models.GetPreferencesWithDefaultsQuery{User: &models.SignedInUser{OrgId: 1, UserId: 2}}
-			err := GetPreferencesWithDefaults(query)
+			query := &models.GetPreferencesWithDefaultsQuery{
+				User: &models.SignedInUser{OrgId: 1, UserId: 2},
+			}
+			err = GetPreferencesWithDefaults(query)
 			So(err, ShouldBeNil)
 			So(query.Result.HomeDashboardId, ShouldEqual, 1)
 		})

pkg/services/sqlstore/session.go

@@ -68,13 +68,18 @@ func withDbSession(ctx context.Context, callback dbTransactionFunc) error {
 func (sess *DBSession) InsertId(bean interface{}) (int64, error) {
 	table := sess.DB().Mapper.Obj2Table(getTypeName(bean))
 
-	dialect.PreInsertId(table, sess.Session)
-
+	if err := dialect.PreInsertId(table, sess.Session); err != nil {
+		return 0, err
+	}
 	id, err := sess.Session.InsertOne(bean)
+	if err != nil {
+		return 0, err
+	}
+	if err := dialect.PostInsertId(table, sess.Session); err != nil {
+		return 0, err
+	}
 
-	dialect.PostInsertId(table, sess.Session)
-
-	return id, err
+	return id, nil
 }
 
 func getTypeName(bean interface{}) (res string) {

pkg/services/sqlstore/sql_test_data.go

@@ -57,10 +57,12 @@ func InsertSqlTestData(cmd *m.InsertSqlTestDataCommand) error {
 		rows, _ := res.RowsAffected()
 		sqlog.Info("SQL TestData: Truncate done", "rows", rows)
 
-		sqlRandomWalk("server1", "frontend", 100, 1.123, sess)
-		sqlRandomWalk("server2", "frontend", 100, 1.123, sess)
-		sqlRandomWalk("server3", "frontend", 100, 1.123, sess)
-
-		return err
+		if err := sqlRandomWalk("server1", "frontend", 100, 1.123, sess); err != nil {
+			return err
+		}
+		if err := sqlRandomWalk("server2", "frontend", 100, 1.123, sess); err != nil {
+			return err
+		}
+		return sqlRandomWalk("server3", "frontend", 100, 1.123, sess)
 	})
 }

pkg/services/sqlstore/sqlstore.go

@@ -180,7 +180,10 @@ func (ss *SqlStore) buildConnectionString() (string, error) {
 			if err != nil {
 				return "", err
 			}
-			mysql.RegisterTLSConfig("custom", tlsCert)
+			if err := mysql.RegisterTLSConfig("custom", tlsCert); err != nil {
+				return "", err
+			}
+
 			cnnstr += "&tls=custom"
 		}
 
@@ -205,7 +208,10 @@ func (ss *SqlStore) buildConnectionString() (string, error) {
 		if !filepath.IsAbs(ss.dbCfg.Path) {
 			ss.dbCfg.Path = filepath.Join(ss.Cfg.DataPath, ss.dbCfg.Path)
 		}
-		os.MkdirAll(path.Dir(ss.dbCfg.Path), os.ModePerm)
+		if err := os.MkdirAll(path.Dir(ss.dbCfg.Path), os.ModePerm); err != nil {
+			return "", err
+		}
+
 		cnnstr = fmt.Sprintf("file:%s?cache=%s&mode=rwc", ss.dbCfg.Path, ss.dbCfg.CacheMode)
 		cnnstr += ss.buildExtraConnectionString('&')
 	default:
@@ -312,16 +318,27 @@ func InitTestDB(t ITestDB) *SqlStore {
 
 	// set test db config
 	sqlstore.Cfg = setting.NewCfg()
-	sec, _ := sqlstore.Cfg.Raw.NewSection("database")
-	sec.NewKey("type", dbType)
+	sec, err := sqlstore.Cfg.Raw.NewSection("database")
+	if err != nil {
+		t.Fatalf("Failed to create section: %s", err)
+	}
+	if _, err := sec.NewKey("type", dbType); err != nil {
+		t.Fatalf("Failed to create key: %s", err)
+	}
 
 	switch dbType {
 	case "mysql":
-		sec.NewKey("connection_string", sqlutil.TestDB_Mysql.ConnStr)
+		if _, err := sec.NewKey("connection_string", sqlutil.TestDB_Mysql.ConnStr); err != nil {
+			t.Fatalf("Failed to create key: %s", err)
+		}
 	case "postgres":
-		sec.NewKey("connection_string", sqlutil.TestDB_Postgres.ConnStr)
+		if _, err := sec.NewKey("connection_string", sqlutil.TestDB_Postgres.ConnStr); err != nil {
+			t.Fatalf("Failed to create key: %s", err)
+		}
 	default:
-		sec.NewKey("connection_string", sqlutil.TestDB_Sqlite3.ConnStr)
+		if _, err := sec.NewKey("connection_string", sqlutil.TestDB_Sqlite3.ConnStr); err != nil {
+			t.Fatalf("Failed to create key: %s", err)
+		}
 	}
 
 	// need to get engine to clean db before we init

pkg/services/sqlstore/sqlstore_test.go

@@ -90,12 +90,18 @@ func TestSqlConnectionString(t *testing.T) {
 func makeSqlStoreTestConfig(dbType string, host string) *setting.Cfg {
 	cfg := setting.NewCfg()
 
-	sec, _ := cfg.Raw.NewSection("database")
-	sec.NewKey("type", dbType)
-	sec.NewKey("host", host)
-	sec.NewKey("user", "user")
-	sec.NewKey("name", "test_db")
-	sec.NewKey("password", "pass")
+	sec, err := cfg.Raw.NewSection("database")
+	So(err, ShouldBeNil)
+	_, err = sec.NewKey("type", dbType)
+	So(err, ShouldBeNil)
+	_, err = sec.NewKey("host", host)
+	So(err, ShouldBeNil)
+	_, err = sec.NewKey("user", "user")
+	So(err, ShouldBeNil)
+	_, err = sec.NewKey("name", "test_db")
+	So(err, ShouldBeNil)
+	_, err = sec.NewKey("password", "pass")
+	So(err, ShouldBeNil)
 
 	return cfg
 }

pkg/services/sqlstore/team_test.go

@@ -216,20 +216,22 @@ func TestTeamCommandsAndQueries(t *testing.T) {
 				})
 
 				Convey("A user should be able to remove an admin if there are other admins", func() {
-					AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[1], Permission: models.PERMISSION_ADMIN})
+					err = AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[1], Permission: models.PERMISSION_ADMIN})
+					So(err, ShouldBeNil)
 					err = RemoveTeamMember(&models.RemoveTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0], ProtectLastAdmin: true})
-					So(err, ShouldEqual, nil)
+					So(err, ShouldBeNil)
 				})
 
 				Convey("A user should not be able to remove the admin permission for the last admin", func() {
 					err = UpdateTeamMember(&models.UpdateTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0], Permission: 0, ProtectLastAdmin: true})
-					So(err, ShouldEqual, models.ErrLastTeamAdmin)
+					So(err, ShouldBeError, models.ErrLastTeamAdmin)
 				})
 
 				Convey("A user should be able to remove the admin permission if there are other admins", func() {
-					AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[1], Permission: models.PERMISSION_ADMIN})
+					err = AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[1], Permission: models.PERMISSION_ADMIN})
+					So(err, ShouldBeNil)
 					err = UpdateTeamMember(&models.UpdateTeamMemberCommand{OrgId: testOrgId, TeamId: group1.Result.Id, UserId: userIds[0], Permission: 0, ProtectLastAdmin: true})
-					So(err, ShouldEqual, nil)
+					So(err, ShouldBeNil)
 				})
 			})
 

pkg/services/sqlstore/transactions.go

@@ -7,6 +7,7 @@ import (
 	"github.com/go-xorm/xorm"
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/util/errutil"
 	sqlite3 "github.com/mattn/go-sqlite3"
 )
 
@@ -41,19 +42,24 @@ func inTransactionWithRetryCtx(ctx context.Context, engine *xorm.Engine, callbac
 	err = callback(sess)
 
 	// special handling of database locked errors for sqlite, then we can retry 5 times
-	if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 {
-		if sqlError.Code == sqlite3.ErrLocked || sqlError.Code == sqlite3.ErrBusy {
-			sess.Rollback()
-			time.Sleep(time.Millisecond * time.Duration(10))
-			sqlog.Info("Database locked, sleeping then retrying", "error", err, "retry", retry)
-			return inTransactionWithRetry(callback, retry+1)
+	if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 && sqlError.Code ==
+		sqlite3.ErrLocked || sqlError.Code == sqlite3.ErrBusy {
+		if rollErr := sess.Rollback(); rollErr != nil {
+			return errutil.Wrapf(err, "Rolling back transaction due to error failed: %s", rollErr)
 		}
+
+		time.Sleep(time.Millisecond * time.Duration(10))
+		sqlog.Info("Database locked, sleeping then retrying", "error", err, "retry", retry)
+		return inTransactionWithRetry(callback, retry+1)
 	}
 
 	if err != nil {
-		sess.Rollback()
+		if rollErr := sess.Rollback(); rollErr != nil {
+			return errutil.Wrapf(err, "Rolling back transaction due to error failed: %s", rollErr)
+		}
 		return err
-	} else if err = sess.Commit(); err != nil {
+	}
+	if err := sess.Commit(); err != nil {
 		return err
 	}
 

pkg/services/sqlstore/user.go

@@ -284,7 +284,9 @@ func UpdateUserLastSeenAt(cmd *models.UpdateUserLastSeenAtCommand) error {
 
 func SetUsingOrg(cmd *models.SetUsingOrgCommand) error {
 	getOrgsForUserCmd := &models.GetUserOrgListQuery{UserId: cmd.UserId}
-	GetUserOrgList(getOrgsForUserCmd)
+	if err := GetUserOrgList(getOrgsForUserCmd); err != nil {
+		return err
+	}
 
 	valid := false
 	for _, other := range getOrgsForUserCmd.Result {
@@ -292,7 +294,6 @@ func SetUsingOrg(cmd *models.SetUsingOrgCommand) error {
 			valid = true
 		}
 	}
-
 	if !valid {
 		return fmt.Errorf("user does not belong to org")
 	}
@@ -507,7 +508,9 @@ func SearchUsers(query *models.SearchUsersQuery) error {
 func DisableUser(cmd *models.DisableUserCommand) error {
 	user := models.User{}
 	sess := x.Table("user")
-	sess.ID(cmd.UserId).Get(&user)
+	if _, err := sess.ID(cmd.UserId).Get(&user); err != nil {
+		return err
+	}
 
 	user.IsDisabled = cmd.IsDisabled
 	sess.UseBool("is_disabled")
@@ -573,7 +576,9 @@ func deleteUserInTransaction(sess *DBSession, cmd *models.DeleteUserCommand) err
 func UpdateUserPermissions(cmd *models.UpdateUserPermissionsCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 		user := models.User{}
-		sess.ID(cmd.UserId).Get(&user)
+		if _, err := sess.ID(cmd.UserId).Get(&user); err != nil {
+			return err
+		}
 
 		user.IsAdmin = cmd.IsGrafanaAdmin
 		sess.UseBool("is_admin")

pkg/services/sqlstore/user_test.go

@@ -227,13 +227,21 @@ func TestUserDataAccess(t *testing.T) {
 			})
 
 			Convey("when a user is an org member and has been assigned permissions", func() {
-				err := AddOrgUser(&models.AddOrgUserCommand{LoginOrEmail: users[1].Login, Role: models.ROLE_VIEWER, OrgId: users[0].OrgId, UserId: users[1].Id})
+				err := AddOrgUser(&models.AddOrgUserCommand{
+					LoginOrEmail: users[1].Login, Role: models.ROLE_VIEWER,
+					OrgId: users[0].OrgId, UserId: users[1].Id,
+				})
 				So(err, ShouldBeNil)
 
-				testHelperUpdateDashboardAcl(1, models.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[1].Id, Permission: models.PERMISSION_EDIT})
+				err = testHelperUpdateDashboardAcl(1, models.DashboardAcl{
+					DashboardId: 1, OrgId: users[0].OrgId, UserId: users[1].Id,
+					Permission: models.PERMISSION_EDIT,
+				})
 				So(err, ShouldBeNil)
 
-				err = SavePreferences(&models.SavePreferencesCommand{UserId: users[1].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"})
+				err = SavePreferences(&models.SavePreferencesCommand{
+					UserId: users[1].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark",
+				})
 				So(err, ShouldBeNil)
 
 				Convey("when the user is deleted", func() {