Skip to content

Commit

Permalink
cmd/go/internal/modfetch: return structured errors from proxy operations
Browse files Browse the repository at this point in the history
CL 181881 added structured error types for direct fetches.
Use those same structured errors to format proxy errors consistently.

Also ensure that an empty @v/list is treated as equivalent to the module
not existing at all.

Updates #27173
Updates #32715

Change-Id: I203fd8259bc4f28b3389745f1a1fde936b0fa24d
Reviewed-on: https://go-review.googlesource.com/c/go/+/183619
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
  • Loading branch information
Bryan C. Mills committed Jun 25, 2019
1 parent df901bc commit e28f0d9
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 21 deletions.
14 changes: 13 additions & 1 deletion src/cmd/go/internal/modfetch/codehost/codehost.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,20 @@ type UnknownRevisionError struct {
func (e *UnknownRevisionError) Error() string {
return "unknown revision " + e.Rev
}
func (UnknownRevisionError) Is(err error) bool {
return err == os.ErrNotExist
}

// ErrNoCommits is an error equivalent to os.ErrNotExist indicating that a given
// repository or module contains no commits.
var ErrNoCommits error = noCommitsError{}

func (e *UnknownRevisionError) Is(err error) bool {
type noCommitsError struct{}

func (noCommitsError) Error() string {
return "no commits"
}
func (noCommitsError) Is(err error) bool {
return err == os.ErrNotExist
}

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modfetch/codehost/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func (r *gitRepo) Latest() (*RevInfo, error) {
return nil, r.refsErr
}
if r.refs["HEAD"] == "" {
return nil, fmt.Errorf("no commits")
return nil, ErrNoCommits
}
return r.Stat(r.refs["HEAD"])
}
Expand Down
52 changes: 36 additions & 16 deletions src/cmd/go/internal/modfetch/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,26 @@ func (p *proxyRepo) ModulePath() string {
return p.path
}

// versionError returns err wrapped in a ModuleError for p.path.
func (p *proxyRepo) versionError(version string, err error) error {
if version != "" && version != module.CanonicalVersion(version) {
return &module.ModuleError{
Path: p.path,
Err: &module.InvalidVersionError{
Version: version,
Pseudo: IsPseudoVersion(version),
Err: err,
},
}
}

return &module.ModuleError{
Path: p.path,
Version: version,
Err: err,
}
}

func (p *proxyRepo) getBytes(path string) ([]byte, error) {
body, err := p.getBody(path)
if err != nil {
Expand Down Expand Up @@ -226,7 +246,7 @@ func (p *proxyRepo) getBody(path string) (io.ReadCloser, error) {
func (p *proxyRepo) Versions(prefix string) ([]string, error) {
data, err := p.getBytes("@v/list")
if err != nil {
return nil, err
return nil, p.versionError("", err)
}
var list []string
for _, line := range strings.Split(string(data), "\n") {
Expand All @@ -242,7 +262,7 @@ func (p *proxyRepo) Versions(prefix string) ([]string, error) {
func (p *proxyRepo) latest() (*RevInfo, error) {
data, err := p.getBytes("@v/list")
if err != nil {
return nil, err
return nil, p.versionError("", err)
}
var best time.Time
var bestVersion string
Expand All @@ -257,7 +277,7 @@ func (p *proxyRepo) latest() (*RevInfo, error) {
}
}
if bestVersion == "" {
return nil, fmt.Errorf("no commits")
return nil, p.versionError("", codehost.ErrNoCommits)
}
info := &RevInfo{
Version: bestVersion,
Expand All @@ -271,21 +291,21 @@ func (p *proxyRepo) latest() (*RevInfo, error) {
func (p *proxyRepo) Stat(rev string) (*RevInfo, error) {
encRev, err := module.EncodeVersion(rev)
if err != nil {
return nil, err
return nil, p.versionError(rev, err)
}
data, err := p.getBytes("@v/" + encRev + ".info")
if err != nil {
return nil, err
return nil, p.versionError(rev, err)
}
info := new(RevInfo)
if err := json.Unmarshal(data, info); err != nil {
return nil, err
return nil, p.versionError(rev, err)
}
if info.Version != rev && rev == module.CanonicalVersion(rev) && module.Check(p.path, rev) == nil {
// If we request a correct, appropriate version for the module path, the
// proxy must return either exactly that version or an error — not some
// arbitrary other version.
return nil, fmt.Errorf("requested canonical version %s, but proxy returned info for version %s", rev, info.Version)
return nil, p.versionError(rev, fmt.Errorf("proxy returned info for version %s instead of requested version", info.Version))
}
return info, nil
}
Expand All @@ -298,48 +318,48 @@ func (p *proxyRepo) Latest() (*RevInfo, error) {
}
info := new(RevInfo)
if err := json.Unmarshal(data, info); err != nil {
return nil, err
return nil, p.versionError("", err)
}
return info, nil
}

func (p *proxyRepo) GoMod(version string) ([]byte, error) {
if version != module.CanonicalVersion(version) {
return nil, fmt.Errorf("version %s is not canonical", version)
return nil, p.versionError(version, fmt.Errorf("internal error: version passed to GoMod is not canonical"))
}

encVer, err := module.EncodeVersion(version)
if err != nil {
return nil, err
return nil, p.versionError(version, err)
}
data, err := p.getBytes("@v/" + encVer + ".mod")
if err != nil {
return nil, err
return nil, p.versionError(version, err)
}
return data, nil
}

func (p *proxyRepo) Zip(dst io.Writer, version string) error {
if version != module.CanonicalVersion(version) {
return fmt.Errorf("version %s is not canonical", version)
return p.versionError(version, fmt.Errorf("internal error: version passed to Zip is not canonical"))
}

encVer, err := module.EncodeVersion(version)
if err != nil {
return err
return p.versionError(version, err)
}
body, err := p.getBody("@v/" + encVer + ".zip")
if err != nil {
return err
return p.versionError(version, err)
}
defer body.Close()

lr := &io.LimitedReader{R: body, N: codehost.MaxZipFile + 1}
if _, err := io.Copy(dst, lr); err != nil {
return err
return p.versionError(version, err)
}
if lr.N <= 0 {
return fmt.Errorf("downloaded zip file too large")
return p.versionError(version, fmt.Errorf("downloaded zip file too large"))
}
return nil
}
Expand Down
9 changes: 7 additions & 2 deletions src/cmd/go/internal/modload/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,13 @@ func queryProxy(proxy, path, query, current string, allowed func(module.Version)
if mayUseLatest {
// Special case for "latest": if no tags match, use latest commit in repo,
// provided it is not excluded.
if latest, err := repo.Latest(); err == nil && allowed(module.Version{Path: path, Version: latest.Version}) {
return lookup(latest.Version)
latest, err := repo.Latest()
if err == nil {
if allowed(module.Version{Path: path, Version: latest.Version}) {
return lookup(latest.Version)
}
} else if !errors.Is(err, os.ErrNotExist) {
return nil, err
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/cmd/go/internal/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,10 @@ func EncodePath(path string) (encoding string, err error) {
// and not contain exclamation marks.
func EncodeVersion(v string) (encoding string, err error) {
if err := checkElem(v, true); err != nil || strings.Contains(v, "!") {
return "", fmt.Errorf("disallowed version string %q", v)
return "", &InvalidVersionError{
Version: v,
Err: fmt.Errorf("disallowed version string"),
}
}
return encodeString(v)
}
Expand Down
52 changes: 52 additions & 0 deletions src/cmd/go/testdata/script/mod_query_empty.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
env GO111MODULE=on
env GOSUMDB=off

go mod download example.com/join@v1.1.0

# If the proxy serves a bogus result for the @latest version,
# reading that version should cause 'go get' to fail.
env GOPROXY=file:///$WORK/badproxy
cp go.mod.orig go.mod
! go get -d example.com/join/subpkg
stderr 'go get example.com/join/subpkg: example.com/join/subpkg@v0.0.0-20190624000000-123456abcdef: .*'

# If @v/list is empty, the 'go' command should still try to resolve
# other module paths.
env GOPROXY=file:///$WORK/emptysub
cp go.mod.orig go.mod
go get -d example.com/join/subpkg
go list -m example.com/join/...
! stdout 'example.com/join/subpkg'
stdout 'example.com/join v1.1.0'

# If @v/list includes a version that the proxy does not actually serve,
# that version is treated as nonexistent.
env GOPROXY=file:///$WORK/notfound
cp go.mod.orig go.mod
go get -d example.com/join/subpkg
go list -m example.com/join/...
! stdout 'example.com/join/subpkg'
stdout 'example.com/join v1.1.0'

-- go.mod.orig --
module example.com/othermodule
go 1.13
-- $WORK/badproxy/example.com/join/subpkg/@v/list --
v0.0.0-20190624000000-123456abcdef
-- $WORK/badproxy/example.com/join/subpkg/@v/v0.0.0-20190624000000-123456abcdef.info --
This file is not valid JSON.
-- $WORK/badproxy/example.com/join/@v/list --
v1.1.0
-- $WORK/badproxy/example.com/join/@v/v1.1.0.info --
{"Version": "v1.1.0"}
-- $WORK/emptysub/example.com/join/subpkg/@v/list --
-- $WORK/emptysub/example.com/join/@v/list --
v1.1.0
-- $WORK/emptysub/example.com/join/@v/v1.1.0.info --
{"Version": "v1.1.0"}
-- $WORK/notfound/example.com/join/subpkg/@v/list --
v1.0.0-does-not-exist
-- $WORK/notfound/example.com/join/@v/list --
v1.1.0
-- $WORK/notfound/example.com/join/@v/v1.1.0.info --
{"Version": "v1.1.0"}

0 comments on commit e28f0d9

Please sign in to comment.