CI: enable error-handling linters and fix a few bugs#6479
CI: enable error-handling linters and fix a few bugs#6479algorandskiy merged 7 commits intoalgorand:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6479 +/- ##
==========================================
- Coverage 47.44% 47.29% -0.16%
==========================================
Files 666 659 -7
Lines 88310 88225 -85
==========================================
- Hits 41900 41726 -174
- Misses 43649 43733 +84
- Partials 2761 2766 +5 ☔ View full report in Codecov by Sentry. |
|
I just went through and ran |
algorandskiy
left a comment
There was a problem hiding this comment.
LGTM except the local.go changes
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades golangci-lint to v2.6.0 and enables several error-handling focused linters (nilness, nilnesserr, and testifylint's error-is-as check) to improve code safety. The changes fix multiple error-handling bugs discovered by these linters across the codebase.
Key changes:
- Fixes incorrect error variable usage in error handling and logging
- Replaces incorrect assertion methods with proper error-checking assertions
- Removes unreachable or incorrectly conditioned error-handling code
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .golangci.yml | Enables new linters (nilness, nilnesserr, testifylint) for improved error handling checks |
| .github/workflows/reviewdog.yml | Updates golangci-lint version to v2.6.0 |
| scripts/buildtools/versions | Updates golangci-lint dependency version to v2.6.0 |
| network/p2pNetwork.go | Fixes error variable mismatch in logging statement |
| test/netperf-go/puppeteer/puppeteer.go | Restructures error handling to properly check and handle errors from metric operations |
| test/e2e-go/features/participation/overlappingParticipationKeys_test.go | Removes unreachable error handling code |
| test/e2e-go/features/catchup/catchpointCatchup_test.go | Removes redundant nil check and fixes error message formatting |
| netdeploy/remote/nodecfg/nodeConfigurator.go | Removes unreachable error handling code |
| ledger/catchupaccessor.go | Fixes const declaration syntax (moved outside closing paren) |
| cmd/algorelay/relayCmd.go | Removes unreachable error handling code |
| cmd/dispenser/server.go | Fixes incorrect error usage in HTTP response |
| cmd/tealdbg/local.go | Removes unnecessary nil checks before logging errors |
| util/db/dbutil_test.go | Replaces require.Error with require.ErrorIs for proper error comparison |
| ledger/txtail_test.go | Replaces require.Truef(errors.As) with require.ErrorAsf |
| ledger/store/trackerdb/sqlitedriver/sql_test.go | Replaces require.False(errors.As) with require.NotErrorAs |
| ledger/ledger_test.go | Replaces require.True(errors.As) with require.ErrorAs and fixes error variable check |
| ledger/eval/prefetcher/prefetcher_test.go | Replaces require.True(errors.Is) with require.ErrorIs |
| data/txHandler_test.go | Replaces require.Error with require.ErrorIs for proper error comparison |
| daemon/algod/api/server/common/test/handlers_test.go | Changes error assertion from comparing error objects to comparing error strings |
| crypto/merklesignature/merkleSignatureScheme_test.go | Removes duplicate error assertion |
| catchup/universalFetcher_test.go | Fixes incorrect require.Error usage to properly check error type |
| agreement/pseudonode_test.go | Replaces require.ErrorAs with require.ErrorIs for proper error comparison |
| agreement/fuzzer/networkFacade_test.go | Replaces panic(nil) with descriptive panic message |
| test/linttest/lintissues.go | Removes test file for linter validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
While upgrading to golangci-lint v2.6.0 after #6474 and #6473, I enabled some more safety linters (not style linters) to see if anything turned up, and found these error-handling bugs spotted by the
govetliter'snilnesscheck, thenilnesserrlinter, and thetestifylintlinter'serror-is-ascheck.Test Plan
Existing tests should pass.