Repository navigation
fix: check error received from adhoc error channel - #729
Merged
Merged
Conversation
golangci-lint is installed unpinned (@latest) by the check_lint make target, and errcheck in golangci-lint 2.15.0 now reports a bare channel receive whose value is an error. This fails CI on main and on every open PR without any code change: internal/workflow/workflow.go:222:4: Error return value is not checked (errcheck) The receive is a completion barrier and AdhocFunc already reports its own errors, so consume the value and log it at debug level, matching the handling of the perf error channel in cmd/metrics. Using the blank identifier instead would trip staticcheck's S1005, which runs earlier in make check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
make check_lint fails on main and on every open PR, with no code change required to trigger it:
internal/workflow/workflow.go:222:4: Error return value is not checked (errcheck)
<-adhocErrorChannel
^
1 issues:
make: *** [Makefile:148: check_lint] Error 1
The check_lint target installs the linter unpinned:
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest
golangci-lint 2.15.0 ships an errcheck that now reports a bare channel receive whose value is an error. The code it flags has been unchanged since #599, so this is an upstream linter change rather than a regression in the tree.
Fix
The receive in question is a completion barrier, and AdhocFunc already reports its own errors — hence the original dropped value. This consumes the value and logs it at debug level, mirroring how the perf error channel is handled in cmd/metrics/metrics.go:
// wait for all adhoc actions to complete, errors were reported by the AdhocFunc
for range myTargets {
if adhocErr := <-adhocErrorChannel; adhocErr != nil {
slog.Debug("adhoc function error", slog.String("error", adhocErr.Error()))
}
}
Behavior is unchanged: the loop still blocks until every adhoc action has reported, and errors remain surfaced to the user by AdhocFunc.
_ = <-adhocErrorChannel was considered and rejected — it satisfies errcheck but trips staticcheck's S1005 (unnecessary blank identifier), which runs earlier in make check.
Validation
Run against golangci-lint 2.15.0, the version that surfaced the failure:
golangci-lint run → 0 issues
staticcheck ./internal/workflow/... → clean
go build ./..., gofmt -l, go vet → clean
go test ./internal/workflow/... → pass
Follow-up worth considering (not in this PR)
check_lint, check_static, check_vuln, and check_sec all install their tools with @latest, so any upstream release can turn CI red with no change to this repo — exactly what happened here. Pinning those versions in the Makefile would make such failures land deliberately, on a bump commit. Happy to open a separate PR if maintainers want that.