Skip to content

Commit

Permalink
Better errors for git failures (#1871)
Browse files Browse the repository at this point in the history
* Better errors for git failures

* Add more comments

* address comments
  • Loading branch information
mortent committed May 3, 2021
1 parent 48cd9c1 commit 05596cb
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 76 deletions.
59 changes: 34 additions & 25 deletions internal/errors/resolver/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,33 +39,37 @@ against repo {{ printf "%q " .repo }}
for reference {{ printf "%q " .ref }}
{{- end }}
{{- if or (gt (len .stdout) 0) (gt (len .stderr) 0)}}
{{ printf "\nDetails:" }}
{{- end }}
{{- if gt (len .stdout) 0 }}
{{ printf "%s" .stdout }}
{{- end }}
{{- if gt (len .stderr) 0 }}
{{ printf "%s" .stderr }}
{{- end }}
{{- template "ExecOutputDetails" . }}
`

unknownRefGitExecError = `
Error: Unknown ref {{ printf "%q" .ref }}. Please verify that the reference exists in upstream repo {{ printf "%q" .repo }}.
{{- if or (gt (len .stdout) 0) (gt (len .stderr) 0)}}
{{ printf "\nDetails:" }}
{{- end }}
{{- template "ExecOutputDetails" . }}
`

{{- if gt (len .stdout) 0 }}
{{ printf "%s" .stdout }}
{{- end }}
noGitBinaryError = `
Error: No git executable found. kpt requires git to be installed and available in the path.
{{- if gt (len .stderr) 0 }}
{{ printf "%s" .stderr }}
{{- end }}
{{- template "ExecOutputDetails" . }}
`

httpsAuthRequired = `
Error: Repository {{ printf "%q" .repo }} requires authentication. kpt does not support this for the 'https' protocol. Please use the 'git' protocol instead.
{{- template "ExecOutputDetails" . }}
`

repositoryUnavailable = `
Error: Unable to access repository {{ printf "%q" .repo }}.
{{- template "ExecOutputDetails" . }}
`

repositoryNotFound = `
Error: Repository {{ printf "%q" .repo }} not found.
{{- template "ExecOutputDetails" . }}
`
)

Expand All @@ -88,12 +92,17 @@ func (*gitExecErrorResolver) Resolve(err error) (ResolvedResult, bool) {
"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"):
switch gitExecErr.Type {
case gitutil.UnknownReference:
msg = ExecuteTemplate(unknownRefGitExecError, tmplArgs)
case gitutil.GitExecutableNotFound:
msg = ExecuteTemplate(noGitBinaryError, tmplArgs)
case gitutil.HTTPSAuthRequired:
msg = ExecuteTemplate(httpsAuthRequired, tmplArgs)
case gitutil.RepositoryUnavailable:
msg = ExecuteTemplate(repositoryUnavailable, tmplArgs)
case gitutil.RepositoryNotFound:
msg = ExecuteTemplate(repositoryNotFound, tmplArgs)
default:
msg = ExecuteTemplate(genericGitExecError, tmplArgs)
}
Expand Down
23 changes: 0 additions & 23 deletions internal/errors/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@

package resolver

import (
"bytes"
"fmt"
"strings"
"text/template"
)

// errorResolvers is the list of known resolvers for kpt errors.
var errorResolvers []ErrorResolver

Expand All @@ -43,22 +36,6 @@ func ResolveError(err error) (ResolvedResult, bool) {
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 {
tmpl, tmplErr := template.New("kpterror").Parse(text)
if tmplErr != nil {
panic(fmt.Errorf("error creating template: %w", tmplErr))
}

var b bytes.Buffer
execErr := tmpl.Execute(&b, data)
if execErr != nil {
panic(fmt.Errorf("error executing template: %w", execErr))
}
return strings.TrimSpace(b.String())
}

type ResolvedResult struct {
Message string
ExitCode int
Expand Down
63 changes: 63 additions & 0 deletions internal/errors/resolver/template.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// 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 (
"bytes"
"fmt"
"strings"
"text/template"
)

var baseTemplate = func() *template.Template {
tmpl := template.New("base")
tmpl = template.Must(tmpl.Parse(detailsHelperTemplate))
return tmpl
}()

var (
// detailsHelperTemplate is a helper subtemplate that is available to
// the top-level templates. It is useful when including information from
// execing other commands in the error message.
detailsHelperTemplate = `
{{- define "ExecOutputDetails" }}
{{- if or (gt (len .stdout) 0) (gt (len .stderr) 0)}}
{{ printf "\nDetails:" }}
{{- end }}
{{- if gt (len .stdout) 0 }}
{{ printf "%s" .stdout }}
{{- end }}
{{- if gt (len .stderr) 0 }}
{{ printf "%s" .stderr }}
{{- end }}
{{ end }}
`
)

// 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 {
tmpl := template.Must(baseTemplate.Clone())
template.Must(tmpl.Parse(text))

var b bytes.Buffer
execErr := tmpl.Execute(&b, data)
if execErr != nil {
panic(fmt.Errorf("error executing template: %w", execErr))
}
return strings.TrimSpace(b.String())
}
102 changes: 102 additions & 0 deletions internal/gitutil/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// 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 gitutil

import (
"regexp"
"strings"

"github.com/GoogleContainerTools/kpt/internal/errors"
)

// GitExecErrorType is used to enumerate git errors.
type GitExecErrorType int

const (
// Unknown is used when we can't classify an error into any of the other
// categories.
Unknown GitExecErrorType = iota
// GitExecutableNotFound means the git executable wasn't available.
GitExecutableNotFound
// UnknownReference means that provided reference (tag, branch) wasn't
// found
UnknownReference
// HTTPSAuthRequired means we try to access the repo using the https
// protocol, but the repo required authentication.
HTTPSAuthRequired
// RepositoryNotFound means the provided repo uri doesn't seem to point
// to a valid git repo.
RepositoryNotFound
// RepositoryUnavailable means we weren't able to connect to the provided
// uri.
RepositoryUnavailable
)

// GitExecError is an error type returned if kpt encounters an error while
// executing a git command. It includes information about the command that
// was executed and the output from git.
type GitExecError struct {
Type GitExecErrorType
Args []string
Err error
Command string
Repo string
Ref string
StdErr string
StdOut string
}

func (e *GitExecError) Error() string {
b := new(strings.Builder)
b.WriteString(e.Err.Error())
b.WriteString(": ")
b.WriteString(e.StdErr)
return b.String()
}

// AmendGitExecError provides a way to amend the GitExecError returned by
// the GitLocalRunner.run command.
func AmendGitExecError(err error, f func(e *GitExecError)) {
var gitExecErr *GitExecError
if errors.As(err, &gitExecErr) {
f(gitExecErr)
}
}

// determineErrorType looks at the output to stderr after executing a git
// command and tries to categorize the error.
func determineErrorType(stdErr string) GitExecErrorType {
switch {
case strings.Contains(stdErr, "unknown revision or path not in the working tree"):
return UnknownReference
case strings.Contains(stdErr, "could not read Username"):
return HTTPSAuthRequired
case strings.Contains(stdErr, "Could not resolve host"):
return RepositoryUnavailable
case matches(`fatal: repository '.*' not found`, stdErr):
return RepositoryNotFound
}
return Unknown
}

func matches(pattern, s string) bool {
matched, err := regexp.Match(pattern, []byte(s))
if err != nil {
// This should only return an error if the pattern is invalid, so
// we just panic if that happens.
panic(err)
}
return matched
}
36 changes: 8 additions & 28 deletions internal/gitutil/gitutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ func NewLocalGitRunner(pkg string) (*GitLocalRunner, error) {
const op errors.Op = "gitutil.NewLocalGitRunner"
p, err := exec.LookPath("git")
if err != nil {
return nil, errors.E(op, errors.Git,
fmt.Errorf("no 'git' program on path: %w", err))
return nil, errors.E(op, errors.Git, &GitExecError{
Type: GitExecutableNotFound,
Err: err,
})
}

return &GitLocalRunner{
Expand Down Expand Up @@ -95,7 +97,9 @@ func (g *GitLocalRunner) run(ctx context.Context, verbose bool, command string,
fullArgs := append([]string{command}, args...)
cmd := exec.CommandContext(ctx, g.gitPath, fullArgs...)
cmd.Dir = g.Dir
cmd.Env = os.Environ()
// Disable git prompting the user for credentials.
cmd.Env = append(os.Environ(),
"GIT_TERMINAL_PROMPT=0")

cmdStdout := &bytes.Buffer{}
cmdStderr := &bytes.Buffer{}
Expand All @@ -118,6 +122,7 @@ func (g *GitLocalRunner) run(ctx context.Context, verbose bool, command string,
}
if err != nil {
return RunResult{}, errors.E(op, errors.Git, &GitExecError{
Type: determineErrorType(cmdStderr.String()),
Args: args,
Command: command,
Err: err,
Expand All @@ -131,31 +136,6 @@ func (g *GitLocalRunner) run(ctx context.Context, verbose bool, command string,
}, nil
}

type GitExecError struct {
Args []string
Err error
Command string
Repo string
Ref string
StdErr string
StdOut string
}

func (e *GitExecError) Error() string {
b := new(strings.Builder)
b.WriteString(e.Err.Error())
b.WriteString(": ")
b.WriteString(e.StdErr)
return b.String()
}

func AmendGitExecError(err error, f func(e *GitExecError)) {
var gitExecErr *GitExecError
if errors.As(err, &gitExecErr) {
f(gitExecErr)
}
}

// NewGitUpstreamRepo returns a new GitUpstreamRepo for an upstream package.
func NewGitUpstreamRepo(ctx context.Context, uri string) (*GitUpstreamRepo, error) {
const op errors.Op = "gitutil.NewGitUpstreamRepo"
Expand Down

0 comments on commit 05596cb

Please sign in to comment.