Fix more errorlint warnings. Use `errors.As` and `errors.Is` for error handling. by atombrella · Pull Request #31667 · helm/helm

atombrella · 2025-12-20T11:31:58Z

What this PR does / why we need it:

Use errors.As instead of type-casting, errors.Is for comparison and use of %w for error formatting in fmt.Errorf. The changes do not fully fix all of errorlint. I don't feel comfortable touching files under v1-3 as this may break compatibility with API users.

I have not successfully been able to change the use of switch with comparison of the error type. There are a few instances of this:

pkg/provenance/sign_test.go:401:9: type switch on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
	switch err.(type) {
	       ^
pkg/strvals/parser.go:247:4: switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors (errorlint)
			case io.EOF:
			^
pkg/strvals/parser.go:379:3: switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors (errorlint)
		case io.EOF:

Special notes for your reviewer:

If applicable:

this PR contains user facing changes (the docs needed label should be applied if so)
this PR contains unit tests
this PR has been tested for backwards compatibility

gjenkins8

Changes look good. Thanks!

These large refactor PRs, while valid, can be difficult to merge due to coordinating two maintainers before a conflict arises.

Can you please fix the conflicts, and I can re-approve. And hopefully another maintainer before a conflict arises.

atombrella · 2026-01-17T06:23:38Z

Changes look good. Thanks!

These large refactor PRs, while valid, can be difficult to merge due to coordinating two maintainers before a conflict arises.

Can you please fix the conflicts, and I can re-approve. And hopefully another maintainer before a conflict arises.

@gjenkins8 @TerryHowe Sure. I have fixed the merge-conflicts.

Copilot

Pull request overview

This pull request improves error handling across the Helm codebase by adopting modern Go error handling patterns. It replaces direct error comparisons with errors.Is, type assertions with errors.As, and uses %w instead of %v in fmt.Errorf to properly wrap errors.

Changes:

Replaced direct error comparisons (err == specificError) with errors.Is(err, specificError) for better wrapped error support
Replaced type assertions (err.(*Type)) with errors.As(err, &variable) for safer type checking
Changed error formatting from %v to %w in fmt.Errorf calls to preserve error chains
Grouped related constants in engine.go for better code organization

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/strvals/parser.go	Updated io.EOF comparisons to use errors.Is
pkg/strvals/literal_parser.go	Updated io.EOF comparisons to use errors.Is (contains critical bugs)
pkg/storage/driver/sql_test.go	Changed ErrReleaseNotFound comparison to errors.Is
pkg/storage/driver/sql.go	Updated error wrapping to use %w format
pkg/storage/driver/secrets_test.go	Changed ErrReleaseNotFound comparisons to errors.Is
pkg/storage/driver/cfgmaps_test.go	Changed ErrReleaseNotFound comparisons to errors.Is
pkg/registry/transport.go	Updated io.EOF comparison to use errors.Is
pkg/kube/wait.go	Replaced StatusError type assertion with errors.As
pkg/kube/client.go	Updated error comparisons and type assertions to use errors.Is and errors.As
pkg/engine/engine.go	Grouped constants and updated error wrapping to use %w
pkg/downloader/manager.go	Updated error wrapping to use %w format
pkg/downloader/chart_downloader_test.go	Changed ErrNoOwnerRepo comparison to errors.Is
pkg/downloader/chart_downloader.go	Changed ErrNoOwnerRepo comparison to errors.Is
pkg/cmd/upgrade.go	Changed ErrReleaseNotFound comparison to errors.Is
pkg/cmd/template.go	Updated error wrapping to use %w format
pkg/cmd/search_repo.go	Updated error wrapping to use %w format
pkg/cmd/search_hub.go	Updated error wrapping to use %w format
pkg/cmd/rollback.go	Updated error wrapping to use %w format
pkg/cmd/load_plugins.go	Replaced InvokeExecError type assertion with errors.As
pkg/cmd/lint.go	Updated error wrapping to use %w format
pkg/cmd/dependency_build.go	Replaced ErrRepoNotFound type assertion with errors.As
pkg/chart/loader/archive/archive.go	Updated io.EOF comparison and error wrapping
pkg/action/validate.go	Updated error wrapping to use %w format
pkg/action/upgrade.go	Updated error wrapping to use %w format
pkg/action/package_test.go	Changed error comparison to errors.Is
pkg/action/install.go	Updated error wrapping to use %w format
internal/sympath/walk.go	Changed filepath.SkipDir comparisons to errors.Is
internal/plugin/runtime_subprocess_test.go	Replaced InvokeExecError type assertion with errors.As
internal/plugin/runtime_subprocess.go	Replaced exec.ExitError type assertions with errors.As
internal/plugin/installer/local_installer_test.go	Changed ErrPluginNotADirectory comparison to errors.Is

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

atombrella · 2026-02-23T22:29:31Z

@TerryHowe @gjenkins8 Can you please have another look?
I think it'd be nice to eventually enable errorlint, but that is something that is best done in a few iterations, instead of all at once.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T23:16:32Z

 			v, err := semver.NewVersion(ch.Metadata.Version)
 			if err != nil {
-				return fmt.Errorf("invalid version %s for dependency %s: %s", dep.Version, dep.Name, err)
+				return fmt.Errorf("invalid version %s for dependency %s: %w", dep.Version, dep.Name, err)


The error message here says the invalid version is dep.Version, but the parse that can fail is semver.NewVersion(ch.Metadata.Version). This can mislead users during troubleshooting. Consider reporting ch.Metadata.Version (or otherwise clarifying which version string failed to parse).

Suggested change

return fmt.Errorf("invalid version %s for dependency %s: %w", dep.Version, dep.Name, err)

return fmt.Errorf("invalid chart metadata version %s for dependency %s: %w", ch.Metadata.Version, dep.Name, err)

CoPilot keeps suggesting new things. I think it'd change scope a bit here?

Use errors.As instead of type-casting, errors.Is for comparison and use of %w for error formatting in fmt.Errorf. The changes do not fully fix all of errorlint. I don't feel comfortable touching files under v1-3 as this may break compatibility with api users. Signed-off-by: Mads Jensen <atombrella@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T07:49:29Z


 	if !u.DisableHooks {
-		if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {
+		if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.Timeout, serverSideApply); err != nil {


The function signature for execHook requires a waitOptions []kube.WaitOption parameter between waitStrategy and timeout. Line 457 is missing u.WaitOptions in the call to execHook. This should be u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply) to match the correct signature.

Suggested change

if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.Timeout, serverSideApply); err != nil {

if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {

Copilot · 2026-02-27T07:49:29Z

 	// pre-install hooks
 	if !i.DisableHooks {
-		if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil {
+		if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.Timeout, i.ServerSideApply); err != nil {


The function signature for execHook requires a waitOptions []kube.WaitOption parameter between waitStrategy and timeout. This call is missing i.WaitOptions. This should be i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply) to match the correct signature shown in the post-install hook at line 551.

Suggested change

if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.Timeout, i.ServerSideApply); err != nil {

if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil {

Copilot · 2026-02-27T07:49:29Z

+	if idx := strings.Index(remainder, ": executing "); idx != -1 {
+		templateName := remainder[:idx]
+		after := remainder[idx+len(": executing "):]


This change from strings.Cut to manual string index operations appears unrelated to the PR's stated purpose of fixing errorlint warnings. The strings.Cut function is available in Go 1.18+ and was working correctly. This refactoring increases code complexity without providing any benefit and is out of scope for this PR.

Suggested change

if idx := strings.Index(remainder, ": executing "); idx != -1 {

templateName := remainder[:idx]

after := remainder[idx+len(": executing "):]

if templateName, after, ok := strings.Cut(remainder, ": executing "); ok {

Copilot · 2026-02-27T07:49:30Z

+		endLoc := strings.Index(afterAt, ">: ")
+		if endLoc == -1 {
 			return TraceableError{}, false
 		}
+		locationName := afterAt[:endLoc]
+		errMsg := afterAt[endLoc+len(">: "):]


This change from strings.Cut to manual string index operations appears unrelated to the PR's stated purpose of fixing errorlint warnings. The strings.Cut function is available in Go 1.18+ and was working correctly. This refactoring increases code complexity without providing any benefit and is out of scope for this PR.

Suggested change

endLoc := strings.Index(afterAt, ">: ")

if endLoc == -1 {

return TraceableError{}, false

}

locationName := afterAt[:endLoc]

errMsg := afterAt[endLoc+len(">: "):]

locationName, errMsg, found := strings.Cut(afterAt, ">: ")

if !found {

return TraceableError{}, false

}

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Mads Jensen <mje@inducks.org>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

pkg/action/upgrade.go:506

The post-upgrade hook execution is using release.HookPreUpgrade here. That changes behavior (pre-upgrade hooks would run twice and post-upgrade hooks would never run). This should use release.HookPostUpgrade (and keep the correct argument list for execHook).

		if err := u.cfg.execHook(upgradedRelease, release.HookPostUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {
			u.reportToPerformUpgrade(c, upgradedRelease, results.Created, fmt.Errorf("post-upgrade hooks failed: %w", err))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


 	if !u.DisableHooks {
-		if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {
+		if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.Timeout, serverSideApply); err != nil {


 	// pre-install hooks
 	if !i.DisableHooks {
-		if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil {
+		if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.Timeout, i.ServerSideApply); err != nil {


pull-request-size Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 20, 2025

atombrella changed the title ~~Fix more errorlint warnings. Use errors.As and errors.Is for error handling.~~ Fix more errorlint warnings. Use errors.As and errors.Is for error handling. Dec 20, 2025

atombrella force-pushed the feature/errorlint_as_round_2 branch from b16ab16 to 711204d Compare December 20, 2025 11:33

atombrella commented Dec 20, 2025

View reviewed changes

Comment thread pkg/engine/engine.go

atombrella commented Dec 20, 2025

View reviewed changes

Comment thread pkg/kube/wait.go Outdated

atombrella force-pushed the feature/errorlint_as_round_2 branch from 6ec8cd9 to 711204d Compare December 20, 2025 16:16

TerryHowe reviewed Dec 20, 2025

View reviewed changes

Comment thread pkg/kube/wait.go Outdated

Comment thread pkg/storage/driver/sql_test.go

atombrella force-pushed the feature/errorlint_as_round_2 branch 2 times, most recently from 6ded2ff to e597f7f Compare December 20, 2025 17:14

gjenkins8 previously approved these changes Jan 17, 2026

View reviewed changes

atombrella dismissed gjenkins8’s stale review via f8387ea January 17, 2026 06:22

TerryHowe requested a review from Copilot January 17, 2026 16:24

Copilot started reviewing on behalf of TerryHowe January 17, 2026 16:24 View session

Copilot AI reviewed Jan 17, 2026

View reviewed changes

Comment thread pkg/strvals/literal_parser.go Outdated

Comment thread pkg/strvals/literal_parser.go Outdated

Copilot AI review requested due to automatic review settings February 23, 2026 22:32

pull-request-size Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 23, 2026

Copilot started reviewing on behalf of atombrella February 23, 2026 22:33 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Comment thread pkg/registry/transport.go Outdated

Copilot AI review requested due to automatic review settings February 23, 2026 22:45

Copilot started reviewing on behalf of atombrella February 23, 2026 22:46 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

pull-request-size Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 23, 2026

Copilot AI review requested due to automatic review settings February 23, 2026 23:00

Copilot started reviewing on behalf of atombrella February 23, 2026 23:01 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Comment thread pkg/engine/engine.go Outdated

atombrella force-pushed the feature/errorlint_as_round_2 branch from 10b2748 to 8462950 Compare February 23, 2026 23:03

atombrella mentioned this pull request Feb 23, 2026

fix(pkg): errorlint linter #31815

Closed

3 tasks

Copilot AI review requested due to automatic review settings February 23, 2026 23:12

Copilot started reviewing on behalf of atombrella February 23, 2026 23:13 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

atombrella force-pushed the feature/errorlint_as_round_2 branch 2 times, most recently from ef0e907 to cd99cdc Compare February 26, 2026 17:07

Copilot AI review requested due to automatic review settings February 27, 2026 07:46

atombrella force-pushed the feature/errorlint_as_round_2 branch from cd99cdc to 28ffefc Compare February 27, 2026 07:46

Copilot started reviewing on behalf of atombrella February 27, 2026 07:46 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Update pkg/cmd/dependency_build.go

e7a7b92

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Mads Jensen <mje@inducks.org>

Copilot AI review requested due to automatic review settings March 14, 2026 08:41

Update pkg/action/upgrade.go

beb0745

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Mads Jensen <mje@inducks.org>

Copilot started reviewing on behalf of atombrella March 14, 2026 08:41 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

	return fmt.Errorf("invalid version %s for dependency %s: %w", dep.Version, dep.Name, err)
	return fmt.Errorf("invalid chart metadata version %s for dependency %s: %w", ch.Metadata.Version, dep.Name, err)

	if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.Timeout, serverSideApply); err != nil {
	if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {

	if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.Timeout, i.ServerSideApply); err != nil {
	if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil {

Conversation

atombrella commented Dec 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gjenkins8 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atombrella commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

atombrella commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

atombrella Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gjenkins8 left a comment •

edited

Loading

atombrella commented Jan 17, 2026 •

edited

Loading