Skip to content

Commit 145d498

Browse files
committed
improve error handling in pkg cat command to include package context
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
1 parent bdffd40 commit 145d498

2 files changed

Lines changed: 16 additions & 13 deletions

File tree

thirdparty/cmdconfig/commands/cmdcat/cmdcat.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414

1515
"github.com/kptdev/kpt/internal/docs/generated/pkgdocs"
1616
kptfilev1 "github.com/kptdev/kpt/pkg/api/kptfile/v1"
17-
"github.com/kptdev/kpt/pkg/printer"
1817
"github.com/kptdev/kpt/thirdparty/cmdconfig/commands/runner"
1918
"github.com/spf13/cobra"
2019
"sigs.k8s.io/kustomize/kyaml/kio"
@@ -114,14 +113,13 @@ func (r *CatRunner) ExecuteCmd(w io.Writer, pkgPath string) error {
114113
}.Execute()
115114

116115
if err != nil {
117-
// Emit a contextual diagnostic on stderr so the user knows which
118-
// package failed; the runner currently aborts on the first error.
119-
fmt.Fprintf(printer.FromContextOrDie(r.Ctx).ErrStream(),
120-
"kpt pkg cat: %s in package %q\n", err.Error(), pkgPath)
121-
return err
116+
// Wrap with package context so the user knows which package failed;
117+
// the root command's error handler is responsible for printing.
118+
return fmt.Errorf("kpt pkg cat: %q: %w", pkgPath, err)
122119
}
123-
fmt.Fprint(w, out.String())
124-
if out.String() != "" {
120+
outStr := out.String()
121+
fmt.Fprint(w, outStr)
122+
if outStr != "" {
125123
fmt.Fprint(w, "---")
126124
}
127125
return nil

thirdparty/cmdconfig/commands/cmdcat/cmdcat_test.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -398,20 +398,25 @@ data:
398398

399399
_, err := runCat(t, d)
400400
assert.Error(t, err)
401+
// The returned error must carry the pkg-path context ("kpt pkg cat: "<pkg>":")
402+
// as well as the underlying file that failed. The root cobra handler
403+
// (main.handleErr) is responsible for printing; ExecuteCmd must not
404+
// write to stderr itself, or the user would see the message twice.
405+
assert.Contains(t, err.Error(), "kpt pkg cat:")
406+
assert.Contains(t, err.Error(), d)
401407
assert.Contains(t, err.Error(), "broken.yaml")
402408

403-
// Stderr must carry the contextual diagnostic. Inject a printer via
404-
// context (mirroring how run.go wires the root cobra err stream) and
405-
// assert on its err stream.
409+
// Sanity-check that ExecuteCmd itself does not write to the printer's
410+
// ErrStream — otherwise the root handler would duplicate the output.
406411
var errBuf bytes.Buffer
407412
ctx := printer.WithContext(context.Background(), printer.New(nil, &errBuf))
408413
r := GetCatRunner(ctx, "")
409414
r.Command.SetArgs([]string{d})
410415
r.Command.SetOut(&bytes.Buffer{})
411416
r.Command.SetErr(&bytes.Buffer{})
412417
_ = r.Command.Execute()
413-
assert.Contains(t, errBuf.String(), "kpt pkg cat:")
414-
assert.Contains(t, errBuf.String(), "broken.yaml")
418+
assert.Empty(t, errBuf.String(),
419+
"ExecuteCmd must not print to ErrStream; the root handler prints the returned error")
415420
}
416421

417422
// TestCmd_NonExistent verifies a clean error on a missing path.

0 commit comments

Comments
 (0)