From 9593d57914c2058ad577209394f80b8d743e7f5e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 19 Nov 2020 14:47:17 +0100 Subject: [PATCH] Chore: Enable errorlint linter (#29227) * Enable errorlint linter * Handle wrapped errors Signed-off-by: Arve Knudsen Co-authored-by: Emil Tullstedt --- pkg/cmd/grafana-cli/services/api_client.go | 4 ++-- .../grafana-cli/services/api_client_test.go | 6 +++-- pkg/middleware/auth_proxy/auth_proxy.go | 9 ++++---- pkg/middleware/middleware.go | 5 ++-- pkg/middleware/recovery.go | 23 +++++++++++-------- pkg/services/alerting/conditions/query.go | 3 ++- pkg/services/alerting/extractor.go | 2 +- pkg/services/alerting/notifier.go | 9 ++++---- .../alerting/notifiers/threema_test.go | 13 ++++++++--- pkg/services/alerting/result_handler.go | 4 ++-- pkg/services/alerting/rule_test.go | 20 ++++++++-------- pkg/services/dashboards/folder_service.go | 16 +++++++------ .../dashboards/folder_service_test.go | 6 ++--- pkg/services/guardian/guardian_test.go | 22 ++++++------------ pkg/services/ldap/ldap.go | 8 +++---- pkg/services/multildap/multildap.go | 2 +- pkg/services/oauthtoken/oauth_token_util.go | 3 ++- .../provisioning/dashboards/file_reader.go | 9 ++++---- .../provisioning/datasources/datasources.go | 4 ++-- .../plugins/plugin_provisioner.go | 4 +++- pkg/services/rendering/http_mode.go | 5 ++-- pkg/services/rendering/plugin_mode.go | 5 ++-- pkg/services/rendering/rendering.go | 3 ++- pkg/services/sqlstore/database_wrapper.go | 3 ++- pkg/services/sqlstore/migrator/migrator.go | 5 ++-- .../sqlstore/migrator/mysql_dialect.go | 7 ++++-- .../sqlstore/migrator/postgres_dialect.go | 7 ++++-- .../sqlstore/migrator/sqlite_dialect.go | 7 ++++-- pkg/services/sqlstore/transactions.go | 5 ++-- pkg/services/sqlstore/user_auth.go | 3 ++- pkg/tsdb/cloudwatch/log_actions.go | 5 ++-- pkg/tsdb/mysql/mysql.go | 6 +++-- pkg/util/filepath.go | 2 +- scripts/go/configs/.golangci.toml | 8 +++++++ 34 files changed, 142 insertions(+), 101 deletions(-) diff --git a/pkg/cmd/grafana-cli/services/api_client.go b/pkg/cmd/grafana-cli/services/api_client.go index 9a7eecbe25b..2aa40a2a2de 100644 --- a/pkg/cmd/grafana-cli/services/api_client.go +++ b/pkg/cmd/grafana-cli/services/api_client.go @@ -4,6 +4,7 @@ import ( "bufio" "crypto/md5" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -25,9 +26,8 @@ type GrafanaComClient struct { func (client *GrafanaComClient) GetPlugin(pluginId, repoUrl string) (models.Plugin, error) { logger.Debugf("getting plugin metadata from: %v pluginId: %v \n", repoUrl, pluginId) body, err := sendRequestGetBytes(HttpClient, repoUrl, "repo", pluginId) - if err != nil { - if err == ErrNotFoundError { + if errors.Is(err, ErrNotFoundError) { return models.Plugin{}, errutil.Wrap("Failed to find requested plugin, check if the plugin_id is correct", err) } return models.Plugin{}, errutil.Wrap("Failed to send request", err) diff --git a/pkg/cmd/grafana-cli/services/api_client_test.go b/pkg/cmd/grafana-cli/services/api_client_test.go index 7a2e197b16d..3ea9c584e09 100644 --- a/pkg/cmd/grafana-cli/services/api_client_test.go +++ b/pkg/cmd/grafana-cli/services/api_client_test.go @@ -2,6 +2,7 @@ package services import ( "bytes" + "errors" "io" "io/ioutil" "net/http" @@ -74,8 +75,9 @@ func makeBody(body string) io.ReadCloser { } func asBadRequestError(t *testing.T, err error) *BadRequestError { - if badRequestError, ok := err.(*BadRequestError); ok { - return badRequestError + var badErr *BadRequestError + if errors.As(err, &badErr) { + return badErr } assert.FailNow(t, "Error was not of type BadRequestError") return nil diff --git a/pkg/middleware/auth_proxy/auth_proxy.go b/pkg/middleware/auth_proxy/auth_proxy.go index d1cd4b293f9..f825038fb8c 100644 --- a/pkg/middleware/auth_proxy/auth_proxy.go +++ b/pkg/middleware/auth_proxy/auth_proxy.go @@ -2,6 +2,7 @@ package authproxy import ( "encoding/hex" + "errors" "fmt" "hash/fnv" "net" @@ -180,12 +181,12 @@ func (auth *AuthProxy) Login(logger log.Logger, ignoreCache bool) (int64, *Error } if isLDAPEnabled() { - id, e := auth.LoginViaLDAP() - if e != nil { - if e == ldap.ErrInvalidCredentials { + id, err := auth.LoginViaLDAP() + if err != nil { + if errors.Is(err, ldap.ErrInvalidCredentials) { return 0, newError("proxy authentication required", ldap.ErrInvalidCredentials) } - return 0, newError("failed to get the user", e) + return 0, newError("failed to get the user", err) } return id, nil diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index 866151b1056..5aa3ec061b4 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -2,6 +2,7 @@ package middleware import ( "context" + "errors" "fmt" "net/url" "strconv" @@ -182,7 +183,7 @@ func initContextWithBasicAuth(ctx *models.ReqContext, orgId int64) bool { "err", err, ) - if err == models.ErrUserNotFound { + if errors.Is(err, models.ErrUserNotFound) { err = login.ErrInvalidCredentials } ctx.JsonApiErr(401, errStringInvalidUsernamePassword, err) @@ -250,7 +251,7 @@ func rotateEndOfRequestFunc(ctx *models.ReqContext, authTokenService models.User // if the request is cancelled by the client we should not try // to rotate the token since the client would not accept any result. - if ctx.Context.Req.Context().Err() == context.Canceled { + if errors.Is(ctx.Context.Req.Context().Err(), context.Canceled) { return } diff --git a/pkg/middleware/recovery.go b/pkg/middleware/recovery.go index db03b3e9482..c238ecbfeee 100644 --- a/pkg/middleware/recovery.go +++ b/pkg/middleware/recovery.go @@ -17,6 +17,7 @@ package middleware import ( "bytes" + "errors" "fmt" "io/ioutil" "net/http" @@ -102,7 +103,7 @@ func function(pc uintptr) []byte { func Recovery() macaron.Handler { return func(c *macaron.Context) { defer func() { - if err := recover(); err != nil { + if r := recover(); r != nil { panicLogger := log.Root // try to get request logger if ctx, ok := c.Data["ctx"]; ok { @@ -110,16 +111,18 @@ func Recovery() macaron.Handler { panicLogger = ctxTyped.Logger } - // http.ErrAbortHandler is suppressed by default in the http package - // and used as a signal for aborting requests. Suppresses stacktrace - // since it doesn't add any important information. - if err == http.ErrAbortHandler { - panicLogger.Error("Request error", "error", err) - return + if err, ok := r.(error); ok { + // http.ErrAbortHandler is suppressed by default in the http package + // and used as a signal for aborting requests. Suppresses stacktrace + // since it doesn't add any important information. + if errors.Is(err, http.ErrAbortHandler) { + panicLogger.Error("Request error", "error", err) + return + } } stack := stack(3) - panicLogger.Error("Request error", "error", err, "stack", string(stack)) + panicLogger.Error("Request error", "error", r, "stack", string(stack)) // if response has already been written, skip. if c.Written() { @@ -131,8 +134,8 @@ func Recovery() macaron.Handler { c.Data["Theme"] = setting.DefaultTheme if setting.Env == setting.Dev { - if theErr, ok := err.(error); ok { - c.Data["Title"] = theErr.Error() + if err, ok := r.(error); ok { + c.Data["Title"] = err.Error() } c.Data["ErrorMsg"] = string(stack) diff --git a/pkg/services/alerting/conditions/query.go b/pkg/services/alerting/conditions/query.go index 80f7c425414..9d7b92557de 100644 --- a/pkg/services/alerting/conditions/query.go +++ b/pkg/services/alerting/conditions/query.go @@ -1,6 +1,7 @@ package conditions import ( + "errors" "fmt" "strings" "time" @@ -158,7 +159,7 @@ func (c *QueryCondition) executeQuery(context *alerting.EvalContext, timeRange * resp, err := c.HandleRequest(context.Ctx, getDsInfo.Result, req) if err != nil { - if err == gocontext.DeadlineExceeded { + if errors.Is(err, gocontext.DeadlineExceeded) { return nil, fmt.Errorf("alert execution exceeded the timeout") } diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index e3ffd594dd9..1c5cdb787a0 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -167,7 +167,7 @@ func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json, } if err := bus.Dispatch(&dsFilterQuery); err != nil { - if err != bus.ErrHandlerNotFound { + if !errors.Is(err, bus.ErrHandlerNotFound) { return nil, err } } else { diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 585124278b3..839b4eda027 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -2,6 +2,7 @@ package alerting import ( "context" + "errors" "fmt" "time" @@ -159,11 +160,11 @@ func (n *notificationService) sendNotification(evalContext *EvalContext, notifie } err := bus.DispatchCtx(evalContext.Ctx, setPendingCmd) - if err == models.ErrAlertNotificationStateVersionConflict { - return nil - } - if err != nil { + if errors.Is(err, models.ErrAlertNotificationStateVersionConflict) { + return nil + } + return err } diff --git a/pkg/services/alerting/notifiers/threema_test.go b/pkg/services/alerting/notifiers/threema_test.go index d22e5813505..4c6b1ed87f3 100644 --- a/pkg/services/alerting/notifiers/threema_test.go +++ b/pkg/services/alerting/notifiers/threema_test.go @@ -1,6 +1,7 @@ package notifiers import ( + "errors" "testing" "github.com/grafana/grafana/pkg/components/simplejson" @@ -70,7 +71,9 @@ func TestThreemaNotifier(t *testing.T) { not, err := NewThreemaNotifier(model) So(not, ShouldBeNil) - So(err.(alerting.ValidationError).Reason, ShouldEqual, "Invalid Threema Gateway ID: Must start with a *") + var valErr alerting.ValidationError + So(errors.As(err, &valErr), ShouldBeTrue) + So(valErr.Reason, ShouldEqual, "Invalid Threema Gateway ID: Must start with a *") }) Convey("invalid Threema Gateway IDs should be rejected (length)", func() { @@ -90,7 +93,9 @@ func TestThreemaNotifier(t *testing.T) { not, err := NewThreemaNotifier(model) So(not, ShouldBeNil) - So(err.(alerting.ValidationError).Reason, ShouldEqual, "Invalid Threema Gateway ID: Must be 8 characters long") + var valErr alerting.ValidationError + So(errors.As(err, &valErr), ShouldBeTrue) + So(valErr.Reason, ShouldEqual, "Invalid Threema Gateway ID: Must be 8 characters long") }) Convey("invalid Threema Recipient IDs should be rejected (length)", func() { @@ -110,7 +115,9 @@ func TestThreemaNotifier(t *testing.T) { not, err := NewThreemaNotifier(model) So(not, ShouldBeNil) - So(err.(alerting.ValidationError).Reason, ShouldEqual, "Invalid Threema Recipient ID: Must be 8 characters long") + var valErr alerting.ValidationError + So(errors.As(err, &valErr), ShouldBeTrue) + So(valErr.Reason, ShouldEqual, "Invalid Threema Recipient ID: Must be 8 characters long") }) }) }) diff --git a/pkg/services/alerting/result_handler.go b/pkg/services/alerting/result_handler.go index dac84bd3a52..a6cd961a2a9 100644 --- a/pkg/services/alerting/result_handler.go +++ b/pkg/services/alerting/result_handler.go @@ -59,12 +59,12 @@ func (handler *defaultResultHandler) handle(evalContext *EvalContext) error { } if err := bus.Dispatch(cmd); err != nil { - if err == models.ErrCannotChangeStateOnPausedAlert { + if errors.Is(err, models.ErrCannotChangeStateOnPausedAlert) { handler.log.Error("Cannot change state on alert that's paused", "error", err) return err } - if err == models.ErrRequiresNewState { + if errors.Is(err, models.ErrRequiresNewState) { handler.log.Info("Alert already updated") return nil } diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index c38d494e2e9..212fc6578e5 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -7,6 +7,8 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/sqlstore" . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type FakeCondition struct{} @@ -34,15 +36,15 @@ func TestAlertRuleFrequencyParsing(t *testing.T) { } for _, tc := range tcs { - r, err := getTimeDurationStringToSeconds(tc.input) - if err != tc.err { - t.Errorf("expected error: '%v' got: '%v'", tc.err, err) - return - } - - if r != tc.result { - t.Errorf("expected result: %d got %d", tc.result, r) - } + t.Run(tc.input, func(t *testing.T) { + r, err := getTimeDurationStringToSeconds(tc.input) + if tc.err == nil { + require.NoError(t, err) + } else { + require.EqualError(t, err, tc.err.Error()) + } + assert.Equal(t, tc.result, r) + }) } } diff --git a/pkg/services/dashboards/folder_service.go b/pkg/services/dashboards/folder_service.go index a3c88863e79..c2692dd7c1c 100644 --- a/pkg/services/dashboards/folder_service.go +++ b/pkg/services/dashboards/folder_service.go @@ -1,6 +1,8 @@ package dashboards import ( + "errors" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/guardian" @@ -209,31 +211,31 @@ func dashToFolder(dash *models.Dashboard) *models.Folder { } func toFolderError(err error) error { - if err == models.ErrDashboardTitleEmpty { + if errors.Is(err, models.ErrDashboardTitleEmpty) { return models.ErrFolderTitleEmpty } - if err == models.ErrDashboardUpdateAccessDenied { + if errors.Is(err, models.ErrDashboardUpdateAccessDenied) { return models.ErrFolderAccessDenied } - if err == models.ErrDashboardWithSameNameInFolderExists { + if errors.Is(err, models.ErrDashboardWithSameNameInFolderExists) { return models.ErrFolderSameNameExists } - if err == models.ErrDashboardWithSameUIDExists { + if errors.Is(err, models.ErrDashboardWithSameUIDExists) { return models.ErrFolderWithSameUIDExists } - if err == models.ErrDashboardVersionMismatch { + if errors.Is(err, models.ErrDashboardVersionMismatch) { return models.ErrFolderVersionMismatch } - if err == models.ErrDashboardNotFound { + if errors.Is(err, models.ErrDashboardNotFound) { return models.ErrFolderNotFound } - if err == models.ErrDashboardFailedGenerateUniqueUid { + if errors.Is(err, models.ErrDashboardFailedGenerateUniqueUid) { err = models.ErrFolderFailedGenerateUniqueUid } diff --git a/pkg/services/dashboards/folder_service_test.go b/pkg/services/dashboards/folder_service_test.go index bdf7556413f..9631bc90646 100644 --- a/pkg/services/dashboards/folder_service_test.go +++ b/pkg/services/dashboards/folder_service_test.go @@ -5,6 +5,7 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" + "github.com/stretchr/testify/assert" "github.com/grafana/grafana/pkg/services/guardian" @@ -194,9 +195,8 @@ func TestFolderService(t *testing.T) { for _, tc := range testCases { actualError := toFolderError(tc.ActualError) - if actualError != tc.ExpectedError { - t.Errorf("For error '%s' expected error '%s', actual '%s'", tc.ActualError, tc.ExpectedError, actualError) - } + assert.EqualErrorf(t, actualError, tc.ExpectedError.Error(), + "For error '%s' expected error '%s', actual '%s'", tc.ActualError, tc.ExpectedError, actualError) } }) }) diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index 24f44d4497e..2f81fca49af 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -1,6 +1,7 @@ package guardian import ( + "errors" "fmt" "runtime" "testing" @@ -300,7 +301,7 @@ func (sc *scenarioContext) verifyDuplicatePermissionsShouldNotBeAllowed() { sc.updatePermissions = p _, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, p) - if err != ErrGuardianPermissionExists { + if !errors.Is(err, ErrGuardianPermissionExists) { sc.reportFailure(tc, ErrGuardianPermissionExists, err) } sc.reportSuccess() @@ -314,8 +315,7 @@ func (sc *scenarioContext) verifyDuplicatePermissionsShouldNotBeAllowed() { } sc.updatePermissions = p _, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, p) - - if err != ErrGuardianPermissionExists { + if !errors.Is(err, ErrGuardianPermissionExists) { sc.reportFailure(tc, ErrGuardianPermissionExists, err) } sc.reportSuccess() @@ -330,7 +330,7 @@ func (sc *scenarioContext) verifyDuplicatePermissionsShouldNotBeAllowed() { sc.updatePermissions = p _, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, p) - if err != ErrGuardianPermissionExists { + if !errors.Is(err, ErrGuardianPermissionExists) { sc.reportFailure(tc, ErrGuardianPermissionExists, err) } sc.reportSuccess() @@ -344,8 +344,7 @@ func (sc *scenarioContext) verifyDuplicatePermissionsShouldNotBeAllowed() { } sc.updatePermissions = p _, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, p) - - if err != ErrGuardianPermissionExists { + if !errors.Is(err, ErrGuardianPermissionExists) { sc.reportFailure(tc, ErrGuardianPermissionExists, err) } sc.reportSuccess() @@ -358,8 +357,7 @@ func (sc *scenarioContext) verifyDuplicatePermissionsShouldNotBeAllowed() { } sc.updatePermissions = p _, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, p) - - if err != ErrGuardianPermissionExists { + if !errors.Is(err, ErrGuardianPermissionExists) { sc.reportFailure(tc, ErrGuardianPermissionExists, err) } sc.reportSuccess() @@ -402,7 +400,6 @@ func (sc *scenarioContext) verifyUpdateDashboardPermissionsShouldBeAllowed(pt pe sc.updatePermissions = permissionList ok, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, permissionList) - if err != nil { sc.reportFailure(tc, nil, err) } @@ -442,7 +439,6 @@ func (sc *scenarioContext) verifyUpdateDashboardPermissionsShouldNotBeAllowed(pt sc.updatePermissions = permissionList ok, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, permissionList) - if err != nil { sc.reportFailure(tc, nil, err) } @@ -505,7 +501,6 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsShouldBeAllowed( sc.updatePermissions = permissionList ok, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, permissionList) - if err != nil { sc.reportFailure(tc, nil, err) } @@ -568,7 +563,6 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsShouldNotBeAllow sc.updatePermissions = permissionList ok, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, permissionList) - if err != nil { sc.reportFailure(tc, nil, err) } @@ -616,8 +610,7 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou sc.updatePermissions = permissionList _, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, permissionList) - - if err != ErrGuardianOverride { + if !errors.Is(err, ErrGuardianOverride) { sc.reportFailure(tc, ErrGuardianOverride, err) } sc.reportSuccess() @@ -665,7 +658,6 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou } sc.updatePermissions = permissionList ok, err := sc.g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, permissionList) - if err != nil { sc.reportFailure(tc, nil, err) } diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index f494b315073..1d9b02d786e 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -475,11 +475,11 @@ func (server *Server) AdminBind() error { func (server *Server) userBind(path, password string) error { err := server.Connection.Bind(path, password) if err != nil { - if ldapErr, ok := err.(*ldap.Error); ok { - if ldapErr.ResultCode == 49 { - return ErrInvalidCredentials - } + var ldapErr *ldap.Error + if errors.As(err, &ldapErr) && ldapErr.ResultCode == 49 { + return ErrInvalidCredentials } + return err } diff --git a/pkg/services/multildap/multildap.go b/pkg/services/multildap/multildap.go index a29dd004e5d..3fa28c908c8 100644 --- a/pkg/services/multildap/multildap.go +++ b/pkg/services/multildap/multildap.go @@ -234,7 +234,7 @@ func isSilentError(err error) bool { continueErrs := []error{ErrInvalidCredentials, ErrCouldNotFindUser} for _, cerr := range continueErrs { - if err == cerr { + if errors.Is(err, cerr) { return true } } diff --git a/pkg/services/oauthtoken/oauth_token_util.go b/pkg/services/oauthtoken/oauth_token_util.go index 30c08753295..40a49b7872e 100644 --- a/pkg/services/oauthtoken/oauth_token_util.go +++ b/pkg/services/oauthtoken/oauth_token_util.go @@ -2,6 +2,7 @@ package oauthtoken import ( "context" + "errors" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" @@ -23,7 +24,7 @@ func GetCurrentOAuthToken(ctx context.Context, user *models.SignedInUser) *oauth authInfoQuery := &models.GetAuthInfoQuery{UserId: user.UserId} if err := bus.Dispatch(authInfoQuery); err != nil { - if err == models.ErrUserNotFound { + if errors.Is(err, models.ErrUserNotFound) { // Not necessarily an error. User may be logged in another way. logger.Debug("no OAuth token for user found", "userId", user.UserId, "username", user.Login) } else { diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index be800a6dddd..709dba0bb3f 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -117,8 +117,7 @@ func (fr *FileReader) startWalkingDisk() error { // storeDashboardsInFolder saves dashboards from the filesystem on disk to the folder from config func (fr *FileReader) storeDashboardsInFolder(filesFoundOnDisk map[string]os.FileInfo, dashboardRefs map[string]*models.DashboardProvisioning, sanityChecker *provisioningSanityChecker) error { folderID, err := getOrCreateFolderID(fr.Cfg, fr.dashboardProvisioningService, fr.Cfg.Folder) - - if err != nil && err != ErrFolderNameMissing { + if err != nil && !errors.Is(err, ErrFolderNameMissing) { return err } @@ -145,7 +144,7 @@ func (fr *FileReader) storeDashboardsInFoldersFromFileStructure(filesFoundOnDisk } folderID, err := getOrCreateFolderID(fr.Cfg, fr.dashboardProvisioningService, folderName) - if err != nil && err != ErrFolderNameMissing { + if err != nil && !errors.Is(err, ErrFolderNameMissing) { return fmt.Errorf("can't provision folder %q from file system structure: %w", folderName, err) } @@ -264,12 +263,12 @@ func getOrCreateFolderID(cfg *config, service dashboards.DashboardProvisioningSe cmd := &models.GetDashboardQuery{Slug: models.SlugifyTitle(folderName), OrgId: cfg.OrgID} err := bus.Dispatch(cmd) - if err != nil && err != models.ErrDashboardNotFound { + if err != nil && !errors.Is(err, models.ErrDashboardNotFound) { return 0, err } // dashboard folder not found. create one. - if err == models.ErrDashboardNotFound { + if errors.Is(err, models.ErrDashboardNotFound) { dash := &dashboards.SaveDashboardDTO{} dash.Dashboard = models.NewDashboardFolder(folderName) dash.Dashboard.IsFolder = true diff --git a/pkg/services/provisioning/datasources/datasources.go b/pkg/services/provisioning/datasources/datasources.go index 9498e9c1f59..3560a7ed484 100644 --- a/pkg/services/provisioning/datasources/datasources.go +++ b/pkg/services/provisioning/datasources/datasources.go @@ -45,11 +45,11 @@ func (dc *DatasourceProvisioner) apply(cfg *configs) error { for _, ds := range cfg.Datasources { cmd := &models.GetDataSourceByNameQuery{OrgId: ds.OrgID, Name: ds.Name} err := bus.Dispatch(cmd) - if err != nil && err != models.ErrDataSourceNotFound { + if err != nil && !errors.Is(err, models.ErrDataSourceNotFound) { return err } - if err == models.ErrDataSourceNotFound { + if errors.Is(err, models.ErrDataSourceNotFound) { dc.log.Info("inserting datasource from configuration ", "name", ds.Name, "uid", ds.UID) insertCmd := createInsertCommand(ds) if err := bus.Dispatch(insertCmd); err != nil { diff --git a/pkg/services/provisioning/plugins/plugin_provisioner.go b/pkg/services/provisioning/plugins/plugin_provisioner.go index f7a6eaf3d31..2c2327da688 100644 --- a/pkg/services/provisioning/plugins/plugin_provisioner.go +++ b/pkg/services/provisioning/plugins/plugin_provisioner.go @@ -1,6 +1,8 @@ package plugins import ( + "errors" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" @@ -42,7 +44,7 @@ func (ap *PluginProvisioner) apply(cfg *pluginsAsConfig) error { query := &models.GetPluginSettingByIdQuery{OrgId: app.OrgID, PluginId: app.PluginID} err := bus.Dispatch(query) if err != nil { - if err != models.ErrPluginSettingNotFound { + if !errors.Is(err, models.ErrPluginSettingNotFound) { return err } } else { diff --git a/pkg/services/rendering/http_mode.go b/pkg/services/rendering/http_mode.go index 3361ca4b783..23bb912a75f 100644 --- a/pkg/services/rendering/http_mode.go +++ b/pkg/services/rendering/http_mode.go @@ -2,6 +2,7 @@ package rendering import ( "context" + "errors" "fmt" "io" "net" @@ -79,7 +80,7 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, renderKey string, defer resp.Body.Close() // check for timeout first - if reqContext.Err() == context.DeadlineExceeded { + if errors.Is(reqContext.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") return nil, ErrTimeout } @@ -99,7 +100,7 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, renderKey string, _, err = io.Copy(out, resp.Body) if err != nil { // check that we didn't timeout while receiving the response. - if reqContext.Err() == context.DeadlineExceeded { + if errors.Is(reqContext.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") return nil, ErrTimeout } diff --git a/pkg/services/rendering/plugin_mode.go b/pkg/services/rendering/plugin_mode.go index 7418d3c0c95..6d3d4d15b26 100644 --- a/pkg/services/rendering/plugin_mode.go +++ b/pkg/services/rendering/plugin_mode.go @@ -2,6 +2,7 @@ package rendering import ( "context" + "errors" "fmt" "time" @@ -45,7 +46,7 @@ func (rs *RenderingService) renderViaPluginV1(ctx context.Context, renderKey str rs.log.Debug("calling renderer plugin", "req", req) rsp, err := rs.pluginInfo.GrpcPluginV1.Render(ctx, req) - if ctx.Err() == context.DeadlineExceeded { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") return nil, ErrTimeout } @@ -88,7 +89,7 @@ func (rs *RenderingService) renderViaPluginV2(ctx context.Context, renderKey str rs.log.Debug("Calling renderer plugin", "req", req) rsp, err := rs.pluginInfo.GrpcPluginV2.Render(ctx, req) - if ctx.Err() == context.DeadlineExceeded { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { rs.log.Info("Rendering timed out") return nil, ErrTimeout } diff --git a/pkg/services/rendering/rendering.go b/pkg/services/rendering/rendering.go index a4b23337bfc..8e53f6add53 100644 --- a/pkg/services/rendering/rendering.go +++ b/pkg/services/rendering/rendering.go @@ -2,6 +2,7 @@ package rendering import ( "context" + "errors" "fmt" "math" "net/url" @@ -136,7 +137,7 @@ func (rs *RenderingService) Render(ctx context.Context, opts Opts) (*RenderResul elapsedTime := time.Since(startTime).Milliseconds() result, err := rs.render(ctx, opts) if err != nil { - if err == ErrTimeout { + if errors.Is(err, ErrTimeout) { metrics.MRenderingRequestTotal.WithLabelValues("timeout").Inc() metrics.MRenderingSummary.WithLabelValues("timeout").Observe(float64(elapsedTime)) } else { diff --git a/pkg/services/sqlstore/database_wrapper.go b/pkg/services/sqlstore/database_wrapper.go index f05d8b6d669..9826319d769 100644 --- a/pkg/services/sqlstore/database_wrapper.go +++ b/pkg/services/sqlstore/database_wrapper.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "database/sql/driver" + "errors" "time" "github.com/gchaincl/sqlhooks" @@ -87,7 +88,7 @@ func (h *databaseQueryWrapper) After(ctx context.Context, query string, args ... func (h *databaseQueryWrapper) OnError(ctx context.Context, err error, query string, args ...interface{}) error { status := "error" // https://golang.org/pkg/database/sql/driver/#ErrSkip - if err == nil || err == driver.ErrSkip { + if err == nil || errors.Is(err, driver.ErrSkip) { status = "success" } diff --git a/pkg/services/sqlstore/migrator/migrator.go b/pkg/services/sqlstore/migrator/migrator.go index 1d9310e819c..80b17fbab60 100644 --- a/pkg/services/sqlstore/migrator/migrator.go +++ b/pkg/services/sqlstore/migrator/migrator.go @@ -1,6 +1,7 @@ package migrator import ( + "errors" "time" _ "github.com/go-sql-driver/mysql" @@ -168,8 +169,8 @@ func (mg *Migrator) inTransaction(callback dbTransactionFunc) error { } 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 rollErr := sess.Rollback(); !errors.Is(err, rollErr) { + return errutil.Wrapf(err, "failed to roll back transaction due to error: %s", rollErr) } return err diff --git a/pkg/services/sqlstore/migrator/mysql_dialect.go b/pkg/services/sqlstore/migrator/mysql_dialect.go index 5ec4763b858..eebcdd75c8d 100644 --- a/pkg/services/sqlstore/migrator/mysql_dialect.go +++ b/pkg/services/sqlstore/migrator/mysql_dialect.go @@ -1,6 +1,7 @@ package migrator import ( + "errors" "fmt" "strconv" "strings" @@ -169,7 +170,8 @@ func (db *MySQLDialect) TruncateDBTables() error { } func (db *MySQLDialect) isThisError(err error, errcode uint16) bool { - if driverErr, ok := err.(*mysql.MySQLError); ok { + var driverErr *mysql.MySQLError + if errors.As(err, &driverErr) { if driverErr.Number == errcode { return true } @@ -183,7 +185,8 @@ func (db *MySQLDialect) IsUniqueConstraintViolation(err error) bool { } func (db *MySQLDialect) ErrorMessage(err error) string { - if driverErr, ok := err.(*mysql.MySQLError); ok { + var driverErr *mysql.MySQLError + if errors.As(err, &driverErr) { return driverErr.Message } return "" diff --git a/pkg/services/sqlstore/migrator/postgres_dialect.go b/pkg/services/sqlstore/migrator/postgres_dialect.go index fdfdbd6b195..978b5b6bffe 100644 --- a/pkg/services/sqlstore/migrator/postgres_dialect.go +++ b/pkg/services/sqlstore/migrator/postgres_dialect.go @@ -1,6 +1,7 @@ package migrator import ( + "errors" "fmt" "strconv" "strings" @@ -172,7 +173,8 @@ func (db *PostgresDialect) TruncateDBTables() error { } func (db *PostgresDialect) isThisError(err error, errcode string) bool { - if driverErr, ok := err.(*pq.Error); ok { + var driverErr *pq.Error + if errors.As(err, &driverErr) { if string(driverErr.Code) == errcode { return true } @@ -182,7 +184,8 @@ func (db *PostgresDialect) isThisError(err error, errcode string) bool { } func (db *PostgresDialect) ErrorMessage(err error) string { - if driverErr, ok := err.(*pq.Error); ok { + var driverErr *pq.Error + if errors.As(err, &driverErr) { return driverErr.Message } return "" diff --git a/pkg/services/sqlstore/migrator/sqlite_dialect.go b/pkg/services/sqlstore/migrator/sqlite_dialect.go index bb7b1d3fc5c..f98cec1b649 100644 --- a/pkg/services/sqlstore/migrator/sqlite_dialect.go +++ b/pkg/services/sqlstore/migrator/sqlite_dialect.go @@ -1,6 +1,7 @@ package migrator import ( + "errors" "fmt" "github.com/grafana/grafana/pkg/util/errutil" @@ -120,7 +121,8 @@ func (db *SQLite3) TruncateDBTables() error { } func (db *SQLite3) isThisError(err error, errcode int) bool { - if driverErr, ok := err.(sqlite3.Error); ok { + var driverErr sqlite3.Error + if errors.As(err, &driverErr) { if int(driverErr.ExtendedCode) == errcode { return true } @@ -130,7 +132,8 @@ func (db *SQLite3) isThisError(err error, errcode int) bool { } func (db *SQLite3) ErrorMessage(err error) string { - if driverErr, ok := err.(sqlite3.Error); ok { + var driverErr sqlite3.Error + if errors.As(err, &driverErr) { return driverErr.Error() } return "" diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index d81569c1317..31ae27c3370 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -2,6 +2,7 @@ package sqlstore import ( "context" + "errors" "time" "github.com/grafana/grafana/pkg/bus" @@ -42,8 +43,8 @@ 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 && sqlError.Code == - sqlite3.ErrLocked || sqlError.Code == sqlite3.ErrBusy { + var sqlError sqlite3.Error + if errors.As(err, &sqlError) && 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) } diff --git a/pkg/services/sqlstore/user_auth.go b/pkg/services/sqlstore/user_auth.go index ede3ae12451..0bef79e1600 100644 --- a/pkg/services/sqlstore/user_auth.go +++ b/pkg/services/sqlstore/user_auth.go @@ -2,6 +2,7 @@ package sqlstore import ( "encoding/base64" + "errors" "time" "github.com/grafana/grafana/pkg/bus" @@ -33,7 +34,7 @@ func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error { authQuery.AuthId = query.AuthId err = GetAuthInfo(authQuery) - if err != models.ErrUserNotFound { + if !errors.Is(err, models.ErrUserNotFound) { if err != nil { return err } diff --git a/pkg/tsdb/cloudwatch/log_actions.go b/pkg/tsdb/cloudwatch/log_actions.go index 9760d837bdf..617fc7da088 100644 --- a/pkg/tsdb/cloudwatch/log_actions.go +++ b/pkg/tsdb/cloudwatch/log_actions.go @@ -2,6 +2,7 @@ package cloudwatch import ( "context" + "errors" "fmt" "sort" @@ -258,11 +259,11 @@ func (e *cloudWatchExecutor) executeStopQuery(ctx context.Context, logsClient cl response, err := logsClient.StopQueryWithContext(ctx, queryInput) if err != nil { - awsErr, ok := err.(awserr.Error) // If the query has already stopped by the time CloudWatch receives the stop query request, // an "InvalidParameterException" error is returned. For our purposes though the query has been // stopped, so we ignore the error. - if ok && awsErr.Code() == "InvalidParameterException" { + var awsErr awserr.Error + if errors.As(err, &awsErr) && awsErr.Code() == "InvalidParameterException" { response = &cloudwatchlogs.StopQueryOutput{Success: aws.Bool(false)} err = nil } diff --git a/pkg/tsdb/mysql/mysql.go b/pkg/tsdb/mysql/mysql.go index 75b6ea1610d..e395ec3bdb0 100644 --- a/pkg/tsdb/mysql/mysql.go +++ b/pkg/tsdb/mysql/mysql.go @@ -140,8 +140,10 @@ func (t *mysqlQueryResultTransformer) TransformQueryResult(columnTypes []*sql.Co } func (t *mysqlQueryResultTransformer) TransformQueryError(err error) error { - if driverErr, ok := err.(*mysql.MySQLError); ok { - if driverErr.Number != mysqlerr.ER_PARSE_ERROR && driverErr.Number != mysqlerr.ER_BAD_FIELD_ERROR && driverErr.Number != mysqlerr.ER_NO_SUCH_TABLE { + var driverErr *mysql.MySQLError + if errors.As(err, &driverErr) { + if driverErr.Number != mysqlerr.ER_PARSE_ERROR && driverErr.Number != mysqlerr.ER_BAD_FIELD_ERROR && + driverErr.Number != mysqlerr.ER_NO_SUCH_TABLE { t.log.Error("query error", "err", err) return errQueryFailed } diff --git a/pkg/util/filepath.go b/pkg/util/filepath.go index 1b9a85618b3..17350a209b3 100644 --- a/pkg/util/filepath.go +++ b/pkg/util/filepath.go @@ -50,7 +50,7 @@ func walk(path string, info os.FileInfo, resolvedPath string, symlinkPathsFollow } err := walkFn(resolvedPath, info, nil) if err != nil { - if info.IsDir() && err == ErrWalkSkipDir { + if info.IsDir() && errors.Is(err, ErrWalkSkipDir) { err = nil } return err diff --git a/scripts/go/configs/.golangci.toml b/scripts/go/configs/.golangci.toml index f9fdaaec752..8d7dc7162f0 100644 --- a/scripts/go/configs/.golangci.toml +++ b/scripts/go/configs/.golangci.toml @@ -39,6 +39,10 @@ enable = [ "varcheck", "whitespace", "gocyclo", + "typecheck", + "asciicheck", + "errorlint", + "sqlclosecheck", ] # Disabled linters (might want them later) @@ -95,3 +99,7 @@ text = "404" [[issues.exclude-rules]] linters = ["misspell"] text = "Unknwon` is a misspelling of `Unknown" + +[[issues.exclude-rules]] +linters = ["errorlint"] +text = "non-wrapping format verb for fmt.Errorf"