diff --git a/.golangci.yml b/.golangci.yml index 1f6eed68b..4310e276c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ linters: - depguard - dogsled - errcheck + - errorlint - exportloopref - gocyclo - goprintffuncname diff --git a/core/agents/lastfm/agent.go b/core/agents/lastfm/agent.go index 670fc29db..e6f10ac08 100644 --- a/core/agents/lastfm/agent.go +++ b/core/agents/lastfm/agent.go @@ -2,6 +2,7 @@ package lastfm import ( "context" + "errors" "net/http" "github.com/navidrome/navidrome/conf" @@ -118,7 +119,9 @@ func (l *lastfmAgent) GetTopSongs(ctx context.Context, id, artistName, mbid stri func (l *lastfmAgent) callArtistGetInfo(ctx context.Context, name string, mbid string) (*Artist, error) { a, err := l.client.ArtistGetInfo(ctx, name, mbid) - lfErr, isLastFMError := err.(*lastFMError) + var lfErr *lastFMError + isLastFMError := errors.As(err, &lfErr) + if mbid != "" && ((err == nil && a.Name == "[unknown]") || (isLastFMError && lfErr.Code == 6)) { log.Warn(ctx, "LastFM/artist.getInfo could not find artist by mbid, trying again", "artist", name, "mbid", mbid) return l.callArtistGetInfo(ctx, name, "") @@ -133,7 +136,8 @@ func (l *lastfmAgent) callArtistGetInfo(ctx context.Context, name string, mbid s func (l *lastfmAgent) callArtistGetSimilar(ctx context.Context, name string, mbid string, limit int) ([]Artist, error) { s, err := l.client.ArtistGetSimilar(ctx, name, mbid, limit) - lfErr, isLastFMError := err.(*lastFMError) + var lfErr *lastFMError + isLastFMError := errors.As(err, &lfErr) if mbid != "" && ((err == nil && s.Attr.Artist == "[unknown]") || (isLastFMError && lfErr.Code == 6)) { log.Warn(ctx, "LastFM/artist.getSimilar could not find artist by mbid, trying again", "artist", name, "mbid", mbid) return l.callArtistGetSimilar(ctx, name, "", limit) @@ -147,7 +151,8 @@ func (l *lastfmAgent) callArtistGetSimilar(ctx context.Context, name string, mbi func (l *lastfmAgent) callArtistGetTopTracks(ctx context.Context, artistName, mbid string, count int) ([]Track, error) { t, err := l.client.ArtistGetTopTracks(ctx, artistName, mbid, count) - lfErr, isLastFMError := err.(*lastFMError) + var lfErr *lastFMError + isLastFMError := errors.As(err, &lfErr) if mbid != "" && ((err == nil && t.Attr.Artist == "[unknown]") || (isLastFMError && lfErr.Code == 6)) { log.Warn(ctx, "LastFM/artist.getTopTracks could not find artist by mbid, trying again", "artist", artistName, "mbid", mbid) return l.callArtistGetTopTracks(ctx, artistName, "", count) @@ -204,7 +209,8 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S if err == nil { return nil } - lfErr, isLastFMError := err.(*lastFMError) + var lfErr *lastFMError + isLastFMError := errors.As(err, &lfErr) if !isLastFMError { log.Warn(ctx, "Last.fm client.scrobble returned error", "track", s.Title, err) return scrobbler.ErrRetryLater diff --git a/core/agents/listenbrainz/agent.go b/core/agents/listenbrainz/agent.go index e6a83aaad..069fa1da5 100644 --- a/core/agents/listenbrainz/agent.go +++ b/core/agents/listenbrainz/agent.go @@ -2,6 +2,7 @@ package listenbrainz import ( "context" + "errors" "net/http" "github.com/navidrome/navidrome/conf" @@ -88,7 +89,8 @@ func (l *listenBrainzAgent) Scrobble(ctx context.Context, userId string, s scrob if err == nil { return nil } - lbErr, isListenBrainzError := err.(*listenBrainzError) + var lbErr *listenBrainzError + isListenBrainzError := errors.As(err, &lbErr) if !isListenBrainzError { log.Warn(ctx, "ListenBrainz Scrobble returned HTTP error", "track", s.Title, err) return scrobbler.ErrRetryLater diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 77d90401c..2cb98429d 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -95,7 +95,8 @@ var _ = Describe("UserRepository", func() { user := *loggedUser user.NewPassword = "new" err := validatePasswordChange(&user, loggedUser) - verr := err.(*rest.ValidationError) + var verr *rest.ValidationError + errors.As(err, &verr) Expect(verr.Errors).To(HaveLen(1)) Expect(verr.Errors).To(HaveKeyWithValue("currentPassword", "ra.validation.required")) }) @@ -104,7 +105,8 @@ var _ = Describe("UserRepository", func() { user := *loggedUser user.CurrentPassword = "abc123" err := validatePasswordChange(&user, loggedUser) - verr := err.(*rest.ValidationError) + var verr *rest.ValidationError + errors.As(err, &verr) Expect(verr.Errors).To(HaveLen(1)) Expect(verr.Errors).To(HaveKeyWithValue("password", "ra.validation.required")) }) @@ -114,7 +116,8 @@ var _ = Describe("UserRepository", func() { user.CurrentPassword = "current" user.NewPassword = "new" err := validatePasswordChange(&user, loggedUser) - verr := err.(*rest.ValidationError) + var verr *rest.ValidationError + errors.As(err, &verr) Expect(verr.Errors).To(HaveLen(1)) Expect(verr.Errors).To(HaveKeyWithValue("currentPassword", "ra.validation.passwordDoesNotMatch")) }) @@ -136,7 +139,8 @@ var _ = Describe("UserRepository", func() { user := *loggedUser user.NewPassword = "new" err := validatePasswordChange(&user, loggedUser) - verr := err.(*rest.ValidationError) + var verr *rest.ValidationError + errors.As(err, &verr) Expect(verr.Errors).To(HaveLen(1)) Expect(verr.Errors).To(HaveKeyWithValue("currentPassword", "ra.validation.required")) }) @@ -145,7 +149,8 @@ var _ = Describe("UserRepository", func() { user := *loggedUser user.CurrentPassword = "abc123" err := validatePasswordChange(&user, loggedUser) - verr := err.(*rest.ValidationError) + var verr *rest.ValidationError + errors.As(err, &verr) Expect(verr.Errors).To(HaveLen(1)) Expect(verr.Errors).To(HaveKeyWithValue("password", "ra.validation.required")) }) @@ -155,7 +160,8 @@ var _ = Describe("UserRepository", func() { user.CurrentPassword = "current" user.NewPassword = "new" err := validatePasswordChange(&user, loggedUser) - verr := err.(*rest.ValidationError) + var verr *rest.ValidationError + errors.As(err, &verr) Expect(verr.Errors).To(HaveLen(1)) Expect(verr.Errors).To(HaveKeyWithValue("currentPassword", "ra.validation.passwordDoesNotMatch")) }) @@ -186,8 +192,11 @@ var _ = Describe("UserRepository", func() { It("returns ValidationError if username already exists", func() { var newUser = &model.User{ID: "2", UserName: "johndoe"} err := validateUsernameUnique(repo, newUser) - Expect(err).To(BeAssignableToTypeOf(&rest.ValidationError{})) - Expect(err.(*rest.ValidationError).Errors).To(HaveKeyWithValue("userName", "ra.validation.unique")) + var verr *rest.ValidationError + isValidationError := errors.As(err, &verr) + + Expect(isValidationError).To(BeTrue()) + Expect(verr.Errors).To(HaveKeyWithValue("userName", "ra.validation.unique")) }) It("returns generic error if repository call fails", func() { repo.Error = errors.New("fake error") diff --git a/server/subsonic/album_lists_test.go b/server/subsonic/album_lists_test.go index 8266d9307..95bce2714 100644 --- a/server/subsonic/album_lists_test.go +++ b/server/subsonic/album_lists_test.go @@ -2,6 +2,7 @@ package subsonic import ( "context" + "errors" "net/http/httptest" "github.com/navidrome/navidrome/server/subsonic/responses" @@ -46,9 +47,12 @@ var _ = Describe("AlbumListController", func() { It("should fail if missing type parameter", func() { r := newGetRequest() _, err := controller.GetAlbumList(w, r) + var subErr subError + isSubError := errors.As(err, &subErr) - Expect(err).To(MatchError("required 'type' parameter is missing")) - Expect(err.(subError).code).To(Equal(responses.ErrorMissingParameter)) + Expect(isSubError).To(BeTrue()) + Expect(subErr).To(MatchError("required 'type' parameter is missing")) + Expect(subErr.code).To(Equal(responses.ErrorMissingParameter)) }) It("should return error if call fails", func() { @@ -58,7 +62,9 @@ var _ = Describe("AlbumListController", func() { _, err := controller.GetAlbumList(w, r) Expect(err).ToNot(BeNil()) - Expect(err.(subError).code).To(Equal(responses.ErrorGeneric)) + var subErr subError + errors.As(err, &subErr) + Expect(subErr.code).To(Equal(responses.ErrorGeneric)) }) }) @@ -82,8 +88,11 @@ var _ = Describe("AlbumListController", func() { r := newGetRequest() _, err := controller.GetAlbumList2(w, r) - Expect(err).To(MatchError("required 'type' parameter is missing")) - Expect(err.(subError).code).To(Equal(responses.ErrorMissingParameter)) + var subErr subError + errors.As(err, &subErr) + + Expect(subErr).To(MatchError("required 'type' parameter is missing")) + Expect(subErr.code).To(Equal(responses.ErrorMissingParameter)) }) It("should return error if call fails", func() { @@ -92,8 +101,10 @@ var _ = Describe("AlbumListController", func() { _, err := controller.GetAlbumList2(w, r) - Expect(err).ToNot(BeNil()) - Expect(err.(subError).code).To(Equal(responses.ErrorGeneric)) + var subErr subError + errors.As(err, &subErr) + Expect(subErr).ToNot(BeNil()) + Expect(subErr.code).To(Equal(responses.ErrorGeneric)) }) }) }) diff --git a/server/subsonic/api.go b/server/subsonic/api.go index f55703b5e..85665ace4 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -183,7 +183,8 @@ func h(r chi.Router, path string, f handler) { res, err := f(w, r) if err != nil { // If it is not a Subsonic error, convert it to an ErrorGeneric - if _, ok := err.(subError); !ok { + var subErr subError + if !errors.As(err, &subErr) { if errors.Is(err, model.ErrNotFound) { err = newError(responses.ErrorDataNotFound, "data not found") } else { @@ -233,8 +234,9 @@ func h410(r chi.Router, paths ...string) { func sendError(w http.ResponseWriter, r *http.Request, err error) { response := newResponse() code := responses.ErrorGeneric - if e, ok := err.(subError); ok { - code = e.code + var subErr subError + if errors.As(err, &subErr) { + code = subErr.code } response.Status = "failed" response.Error = &responses.Error{Code: code, Message: err.Error()}