kubernetes / kubernetes/kubectl

CheckErr has trouble with handling wrapped errors

Open
#1,205 11 comments 0 reactions 0 assignees View on GitHub
kind/feature lifecycle/rotten needs-triage
Dominant language
Go
Stars
3.3k
Forks
1k
PR merge metrics
No merged PRs in 30d

Description

**What would you like to be added**:

Errors handled in `CheckErr` function are unable to be elegantly handled if they are wrapped.

https://github.com/kubernetes/kubectl/blob/18a5313a74f7d83f6b54377d72b421b5ebfa66c9/pkg/cmd/util/helpers.go#L152

In the following code, the `apierrors.isInvalid` checks the error with `errors.As` which can process the error chain but the type conversion below `err.(*apierrors.StatusError)` could lead to bad type conversion when encountering wrapped errors.
```golang
case apierrors.IsInvalid(err):
details := err.(*apierrors.StatusError).Status().Details
s := "The request is invalid"
if details == nil {
handleErr(s, DefaultErrorExitCode)
return
}
```

**Why is this needed**:

I'm not sure if kubectl library should be able to handle the wrapped errors. We want to reuse the CheckErr function from kubectl in other projects where errors can be wrapped. This can be fixed by converting err into APIStatus first and then extract the Details. If this enhancement is permitted, I would be willing to make a PR for it.

Contributor guide

Open the contributing guide

Research direction

Start at CheckErr in pkg/cmd/util/helpers.go, using the linked apierrors.IsInvalid branch as the entry point. Check how wrapped errors reach the invalid-error details handling, then run the relevant kubectl tests; done means wrapped errors no longer cause the invalid-error handling to fail.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.