Move error handling for kpt live commands into kpt error resolver (#1840)

* Move error handling for kpt live commands into kpt error resolver

* Addressed comments

Author: mortent

internal/errors/resolver/resolvers.go renamed to internal/errors/resolver/git.go

@@ -73,10 +73,10 @@ Error: Unknown ref {{ printf "%q" .ref }}. Please verify that the reference exis
 // that can produce error messages for errors of the gitutil.GitExecError type.
 type gitExecErrorResolver struct{}
 
-func (*gitExecErrorResolver) Resolve(err error) (string, bool) {
+func (*gitExecErrorResolver) Resolve(err error) (ResolvedResult, bool) {
 	var gitExecErr *gitutil.GitExecError
 	if !goerrors.As(err, &gitExecErr) {
-		return "", false
+		return ResolvedResult{}, false
 	}
 	fullCommand := fmt.Sprintf("git %s %s", gitExecErr.Command,
 		strings.Join(gitExecErr.Args, " "))
@@ -87,29 +87,37 @@ func (*gitExecErrorResolver) Resolve(err error) (string, bool) {
 		"stdout": gitExecErr.StdOut,
 		"stderr": gitExecErr.StdErr,
 	}
+	var msg string
 	switch {
 	// TODO(mortent): Checking the content of the output at this level seems a bit awkward. We might
 	// consider doing this the the gitutil package and use some kind of error code to signal
 	// the different error cases to higher levels in the stack.
 	case strings.Contains(gitExecErr.StdErr, " unknown revision or path not in the working tree"):
-		return ExecuteTemplate(unknownRefGitExecError, tmplArgs)
+		msg = ExecuteTemplate(unknownRefGitExecError, tmplArgs)
 	default:
-		return ExecuteTemplate(genericGitExecError, tmplArgs)
+		msg = ExecuteTemplate(genericGitExecError, tmplArgs)
 	}
+	return ResolvedResult{
+		Message:  msg,
+		ExitCode: 1,
+	}, true
 }
 
 // gitExecErrorResolver is an implementation of the ErrorResolver interface
 // that can produce error messages for errors of the FnExecError type.
 type fnExecErrorResolver struct{}
 
-func (*fnExecErrorResolver) Resolve(err error) (string, bool) {
+func (*fnExecErrorResolver) Resolve(err error) (ResolvedResult, bool) {
 	kioErr := errors.UnwrapKioError(err)
 
 	var fnErr *errors.FnExecError
 	if !goerrors.As(kioErr, &fnErr) {
-		return "", false
+		return ResolvedResult{}, false
 	}
 	// TODO: write complete details to a file
 
-	return fnErr.String(), true
+	return ResolvedResult{
+		Message: fnErr.String(),
+		ExitCode: 1,
+	}, true
 }

internal/errors/resolver/live.go

@@ -0,0 +1,78 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package resolver
+
+import (
+	"sigs.k8s.io/cli-utils/pkg/apply/taskrunner"
+	"sigs.k8s.io/cli-utils/pkg/inventory"
+)
+
+//nolint:gochecknoinits
+func init() {
+	AddErrorResolver(&liveErrorResolver{})
+}
+
+const (
+	noInventoryObjError = `
+Error: Package uninitialized. Please run "kpt live init" command.
+
+The package needs to be initialized to generate the template
+which will store state for resource sets. This state is
+necessary to perform functionality such as deleting an entire
+package or automatically deleting omitted resources (pruning).
+`
+	multipleInventoryObjError = `
+Error: Package has multiple inventory object templates.
+
+The package should have one and only one inventory object template.
+`
+	//nolint:lll
+	timeoutError = `
+Error: Timeout after {{printf "%.0f" .err.Timeout.Seconds}} seconds waiting for {{printf "%d" (len .err.TimedOutResources)}} out of {{printf "%d" (len .err.Identifiers)}} resources to reach condition {{ .err.Condition}}:{{ printf "\n" }}
+
+{{- range .err.TimedOutResources}}
+{{printf "%s/%s %s %s" .Identifier.GroupKind.Kind .Identifier.Name .Status .Message }}
+{{- end}}
+`
+
+	TimeoutErrorExitCode = 3
+)
+
+// liveErrorResolver is an implementation of the ErrorResolver interface
+// that can resolve error types used in the live functionality.
+type liveErrorResolver struct{}
+
+func (*liveErrorResolver) Resolve(err error) (ResolvedResult, bool) {
+	tmplArgs := map[string]interface{}{
+		"err": err,
+	}
+	switch err.(type) {
+	case *inventory.NoInventoryObjError:
+		return ResolvedResult{
+			Message: ExecuteTemplate(noInventoryObjError, tmplArgs),
+		}, true
+	case *inventory.MultipleInventoryObjError:
+		return ResolvedResult{
+			Message: ExecuteTemplate(multipleInventoryObjError, tmplArgs),
+		}, true
+	case *taskrunner.TimeoutError:
+		return ResolvedResult{
+			Message:  ExecuteTemplate(timeoutError, tmplArgs),
+			ExitCode: TimeoutErrorExitCode,
+		}, true
+	default:
+		return ResolvedResult{}, false
+	}
+}

internal/errors/resolver/resolver.go

@@ -33,19 +33,19 @@ func AddErrorResolver(er ErrorResolver) {
 // ResolveError attempts to resolve the provided error into a descriptive
 // string which will be displayed to the user. If the last return value is false,
 // the error could not be resolved.
-func ResolveError(err error) (string, bool) {
+func ResolveError(err error) (ResolvedResult, bool) {
 	for _, resolver := range errorResolvers {
 		msg, found := resolver.Resolve(err)
 		if found {
 			return msg, true
 		}
 	}
-	return "", false
+	return ResolvedResult{}, false
 }
 
 // ExecuteTemplate takes the provided template string and data, and renders
 // the template. If something goes wrong, it panics.
-func ExecuteTemplate(text string, data interface{}) (string, bool) {
+func ExecuteTemplate(text string, data interface{}) string {
 	tmpl, tmplErr := template.New("kpterror").Parse(text)
 	if tmplErr != nil {
 		panic(fmt.Errorf("error creating template: %w", tmplErr))
@@ -56,11 +56,16 @@ func ExecuteTemplate(text string, data interface{}) (string, bool) {
 	if execErr != nil {
 		panic(fmt.Errorf("error executing template: %w", execErr))
 	}
-	return strings.TrimSpace(b.String()), true
+	return strings.TrimSpace(b.String())
+}
+
+type ResolvedResult struct {
+	Message  string
+	ExitCode int
 }
 
 // ErrorResolver is an interface that allows kpt to resolve an error into
 // an error message suitable for the end user.
 type ErrorResolver interface {
-	Resolve(err error) (string, bool)
+	Resolve(err error) (ResolvedResult, bool)
 }

main.go

@@ -26,13 +26,12 @@ import (
 
 	"github.com/GoogleContainerTools/kpt/internal/errors"
 	"github.com/GoogleContainerTools/kpt/internal/errors/resolver"
-	"github.com/GoogleContainerTools/kpt/internal/util/cmdutil"
 	"github.com/GoogleContainerTools/kpt/run"
 	"github.com/spf13/cobra"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/klog"
+	k8scmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"k8s.io/kubectl/pkg/util/logs"
-	cliutilserror "sigs.k8s.io/cli-utils/pkg/errors"
 )
 
 func main() {
@@ -72,24 +71,27 @@ func runMain() int {
 	return 0
 }
 
-// TODO(mortent): Reconcile the different error handlers here. This is partly
-// a result of previously having the cobra commands in several different repos.
+// handleErr takes care of printing an error message for a given error.
 func handleErr(cmd *cobra.Command, err error) int {
-	msg, found := resolver.ResolveError(err)
-	if found {
-		fmt.Fprintf(cmd.ErrOrStderr(), "\n%s \n", msg)
-		return 1
+	// First attempt to see if we can resolve the error into a specific
+	// error message.
+	re, resolved := resolver.ResolveError(err)
+	if resolved {
+		fmt.Fprintf(cmd.ErrOrStderr(), "\n%s \n", re.Message)
+		return re.ExitCode
 	}
 
+	// Then try to see if it is of type *errors.Error
 	var kptErr *errors.Error
 	if errors.As(err, &kptErr) {
 		fmt.Fprintf(cmd.ErrOrStderr(), "%s \n", kptErr.Error())
 		return 1
 	}
-	// fmt.Fprintf(cmd.ErrOrStderr(), "%s\n", err)
-	cmdutil.PrintErrorStacktrace(err)
-	// TODO: find a way to avoid having to provide `kpt live` as a
-	// parameter here.
-	cliutilserror.CheckErr(cmd.ErrOrStderr(), err, "kpt live")
+
+	// Finally just let the error handler for kubectl handle it. This handles
+	// printing of several error types used in kubectl
+	// TODO: See if we can handle this in kpt and get a uniform experience
+	// across all of kpt.
+	k8scmdutil.CheckErr(err)
 	return 1
 }

thirdparty/cli-utils/printers/table/printer.go

@@ -138,7 +138,6 @@ func (t *Printer) runPrintLoop(coll *ResourceStateCollector, stop chan struct{})
 				ticker.Stop()
 				latestState := coll.LatestState()
 				linesPrinted = baseTablePrinter.PrintTable(latestState, linesPrinted)
-				_, _ = fmt.Fprint(t.IOStreams.Out, "\n")
 				return
 			case <-ticker.C:
 				latestState := coll.LatestState()