+ The GOPROXY environment variable now supports skipping proxies
+ that return errors. Proxy URLs may now be separated with either commas
+ (,) or pipe characters (|). If a proxy URL is
+ followed by a comma, the go command will only try the next proxy
+ in the list after a 404 or 410 HTTP response. If a proxy URL is followed by a
+ pipe character, the go command will try the next proxy in the
+ list after any error. Note that the default value of GOPROXY
+ remains https://proxy.golang.org,direct, which does not fall
+ back to direct in case of errors.
+
TODO
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index ef054c89388..a20a92d03d9 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -2694,15 +2694,15 @@ // Go module mirror run by Google and fall back to a direct connection // if the proxy reports that it does not have the module (HTTP error 404 or 410). // See https://proxy.golang.org/privacy for the service's privacy policy. -// If GOPROXY is set to the string "direct", downloads use a direct connection -// to source control servers. Setting GOPROXY to "off" disallows downloading -// modules from any source. Otherwise, GOPROXY is expected to be a comma-separated -// list of the URLs of module proxies, in which case the go command will fetch -// modules from those proxies. For each request, the go command tries each proxy -// in sequence, only moving to the next if the current proxy returns a 404 or 410 -// HTTP response. The string "direct" may appear in the proxy list, -// to cause a direct connection to be attempted at that point in the search. -// Any proxies listed after "direct" are never consulted. +// +// If GOPROXY is set to the string "direct", downloads use a direct connection to +// source control servers. Setting GOPROXY to "off" disallows downloading modules +// from any source. Otherwise, GOPROXY is expected to be list of module proxy URLs +// separated by either comma (,) or pipe (|) characters, which control error +// fallback behavior. For each request, the go command tries each proxy in +// sequence. If there is an error, the go command will try the next proxy in the +// list if the error is a 404 or 410 HTTP response or if the current proxy is +// followed by a pipe character, indicating it is safe to fall back on any error. // // The GOPRIVATE and GONOPROXY environment variables allow bypassing // the proxy for selected modules. See 'go help module-private' for details. diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go index dcea71adb3a..67b06cbcd69 100644 --- a/src/cmd/go/internal/modfetch/proxy.go +++ b/src/cmd/go/internal/modfetch/proxy.go @@ -101,27 +101,51 @@ cached module versions with GOPROXY=https://example.com/proxy. var proxyOnce struct { sync.Once - list []string + list []proxySpec err error } -func proxyURLs() ([]string, error) { +type proxySpec struct { + // url is the proxy URL or one of "off", "direct", "noproxy". + url string + + // fallBackOnError is true if a request should be attempted on the next proxy + // in the list after any error from this proxy. If fallBackOnError is false, + // the request will only be attempted on the next proxy if the error is + // equivalent to os.ErrNotFound, which is true for 404 and 410 responses. + fallBackOnError bool +} + +func proxyList() ([]proxySpec, error) { proxyOnce.Do(func() { if cfg.GONOPROXY != "" && cfg.GOPROXY != "direct" { - proxyOnce.list = append(proxyOnce.list, "noproxy") + proxyOnce.list = append(proxyOnce.list, proxySpec{url: "noproxy"}) } - for _, proxyURL := range strings.Split(cfg.GOPROXY, ",") { - proxyURL = strings.TrimSpace(proxyURL) - if proxyURL == "" { + + goproxy := cfg.GOPROXY + for goproxy != "" { + var url string + fallBackOnError := false + if i := strings.IndexAny(goproxy, ",|"); i >= 0 { + url = goproxy[:i] + fallBackOnError = goproxy[i] == '|' + goproxy = goproxy[i+1:] + } else { + url = goproxy + goproxy = "" + } + + url = strings.TrimSpace(url) + if url == "" { continue } - if proxyURL == "off" { + if url == "off" { // "off" always fails hard, so can stop walking list. - proxyOnce.list = append(proxyOnce.list, "off") + proxyOnce.list = append(proxyOnce.list, proxySpec{url: "off"}) break } - if proxyURL == "direct" { - proxyOnce.list = append(proxyOnce.list, "direct") + if url == "direct" { + proxyOnce.list = append(proxyOnce.list, proxySpec{url: "direct"}) // For now, "direct" is the end of the line. We may decide to add some // sort of fallback behavior for them in the future, so ignore // subsequent entries for forward-compatibility. @@ -131,18 +155,21 @@ func proxyURLs() ([]string, error) { // Single-word tokens are reserved for built-in behaviors, and anything // containing the string ":/" or matching an absolute file path must be a // complete URL. For all other paths, implicitly add "https://". - if strings.ContainsAny(proxyURL, ".:/") && !strings.Contains(proxyURL, ":/") && !filepath.IsAbs(proxyURL) && !path.IsAbs(proxyURL) { - proxyURL = "https://" + proxyURL + if strings.ContainsAny(url, ".:/") && !strings.Contains(url, ":/") && !filepath.IsAbs(url) && !path.IsAbs(url) { + url = "https://" + url } // Check that newProxyRepo accepts the URL. // It won't do anything with the path. - _, err := newProxyRepo(proxyURL, "golang.org/x/text") - if err != nil { + if _, err := newProxyRepo(url, "golang.org/x/text"); err != nil { proxyOnce.err = err return } - proxyOnce.list = append(proxyOnce.list, proxyURL) + + proxyOnce.list = append(proxyOnce.list, proxySpec{ + url: url, + fallBackOnError: fallBackOnError, + }) } }) @@ -150,15 +177,16 @@ func proxyURLs() ([]string, error) { } // TryProxies iterates f over each configured proxy (including "noproxy" and -// "direct" if applicable) until f returns an error that is not -// equivalent to os.ErrNotExist. +// "direct" if applicable) until f returns no error or until f returns an +// error that is not equivalent to os.ErrNotExist on a proxy configured +// not to fall back on errors. // // TryProxies then returns that final error. // // If GOPROXY is set to "off", TryProxies invokes f once with the argument // "off". func TryProxies(f func(proxy string) error) error { - proxies, err := proxyURLs() + proxies, err := proxyList() if err != nil { return err } @@ -166,28 +194,39 @@ func TryProxies(f func(proxy string) error) error { return f("off") } - var lastAttemptErr error + // We try to report the most helpful error to the user. "direct" and "noproxy" + // errors are best, followed by proxy errors other than ErrNotExist, followed + // by ErrNotExist. Note that errProxyOff, errNoproxy, and errUseProxy are + // equivalent to ErrNotExist. + const ( + notExistRank = iota + proxyRank + directRank + ) + var bestErr error + bestErrRank := notExistRank for _, proxy := range proxies { - err = f(proxy) - if !errors.Is(err, os.ErrNotExist) { - lastAttemptErr = err - break + err := f(proxy.url) + if err == nil { + return nil + } + isNotExistErr := errors.Is(err, os.ErrNotExist) + + if proxy.url == "direct" || proxy.url == "noproxy" { + bestErr = err + bestErrRank = directRank + } else if bestErrRank <= proxyRank && !isNotExistErr { + bestErr = err + bestErrRank = proxyRank + } else if bestErrRank == notExistRank { + bestErr = err } - // The error indicates that the module does not exist. - // In general we prefer to report the last such error, - // because it indicates the error that occurs after all other - // options have been exhausted. - // - // However, for modules in the NOPROXY list, the most useful error occurs - // first (with proxy set to "noproxy"), and the subsequent errors are all - // errNoProxy (which is not particularly helpful). Do not overwrite a more - // useful error with errNoproxy. - if lastAttemptErr == nil || !errors.Is(err, errNoproxy) { - lastAttemptErr = err + if !proxy.fallBackOnError && !isNotExistErr { + break } } - return lastAttemptErr + return bestErr } type proxyRepo struct { diff --git a/src/cmd/go/internal/modfetch/sumdb.go b/src/cmd/go/internal/modfetch/sumdb.go index 1ed71dfb85c..ff81ef687e3 100644 --- a/src/cmd/go/internal/modfetch/sumdb.go +++ b/src/cmd/go/internal/modfetch/sumdb.go @@ -26,6 +26,7 @@ import ( "cmd/go/internal/lockedfile" "cmd/go/internal/str" "cmd/go/internal/web" + "golang.org/x/mod/module" "golang.org/x/mod/sumdb" "golang.org/x/mod/sumdb/note" @@ -146,49 +147,50 @@ func (c *dbClient) initBase() { } // Try proxies in turn until we find out how to connect to this database. - urls, err := proxyURLs() - if err != nil { - c.baseErr = err - return - } - for _, proxyURL := range urls { - if proxyURL == "noproxy" { - continue - } - if proxyURL == "direct" || proxyURL == "off" { - break - } - proxy, err := url.Parse(proxyURL) - if err != nil { - c.baseErr = err - return - } - // Quoting https://golang.org/design/25530-sumdb#proxying-a-checksum-database: - // - // Before accessing any checksum database URL using a proxy, - // the proxy client should first fetch