Repository: gomods/athens (go)

Issue:
Enforce timeout on vcsLister.List()
**Is your feature request related to a problem? Please describe.**

When listing available versions for certain modules, `vcsLister.List()` can sometimes take several minutes to execute. I would like to configure a timeout on that operation so proxy users aren't stuck waiting that long for a response.

**Describe the solution you'd like**

Have the vcsLister respect the existing `ATHENS_TIMEOUT` setting already used by storage backends.

**Describe alternatives you've considered**

None

**Additional context**

Run `go list -m -versions -json github.com/aws/aws-sdk-go-v2/credentials/v2@latest` to see an example of one such module that takes a really long time to list versions for.

Here's a trace from athens for that same module:

![image](https://github.com/gomods/athens/assets/202034/7d062e55-6046-4f54-beec-8ad41b890213)



Pull request:
Add timeout to vcsLister.List()
<!-- 
    2. Put "[WIP]" at the beginning of your PR title if you're not ready to have this merged yet (we have a bot that will tell everyone that it's a work in progress)
-->

## What is the problem I am trying to address?

`vcsLister.List()` can take several minutes to execute for certain modules

## How is the fix applied?

Add a `timeout` field to `vcsLister` that can be used when calling `vcsLister.List()`. The timeout is set by `ATHENS_TIMEOUT`

## What GitHub issue(s) does this PR fix or close?

Fixes #1884 

Patch:
diff --git a/cmd/proxy/actions/app_proxy.go b/cmd/proxy/actions/app_proxy.go
index 0117bc9d..86c2a247 100644
--- a/cmd/proxy/actions/app_proxy.go
+++ b/cmd/proxy/actions/app_proxy.go
@@ -99,7 +99,7 @@ func addProxyRoutes(
 		return err
 	}
 
-	lister := module.NewVCSLister(c.GoBinary, c.GoBinaryEnvVars, fs)
+	lister := module.NewVCSLister(c.GoBinary, c.GoBinaryEnvVars, fs, c.TimeoutDuration())
 	checker := storage.WithChecker(s)
 	withSingleFlight, err := getSingleFlight(l, c, s, checker)
 	if err != nil {
diff --git a/pkg/download/list.go b/pkg/download/list.go
index 04971cdb..16188db8 100644
--- a/pkg/download/list.go
+++ b/pkg/download/list.go
@@ -28,7 +28,7 @@ func ListHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Handle
 
 		versions, err := dp.List(r.Context(), mod)
 		if err != nil {
-			severityLevel := errors.Expect(err, errors.KindNotFound)
+			severityLevel := errors.Expect(err, errors.KindNotFound, errors.KindGatewayTimeout)
 			err = errors.E(op, err, severityLevel)
 			lggr.SystemErr(err)
 			w.WriteHeader(errors.Kind(err))
diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go
index 6b502dc1..b2378b19 100644
--- a/pkg/errors/errors.go
+++ b/pkg/errors/errors.go
@@ -18,6 +18,7 @@ const (
 	KindRateLimit      = http.StatusTooManyRequests
 	KindNotImplemented = http.StatusNotImplemented
 	KindRedirect       = http.StatusMovedPermanently
+	KindGatewayTimeout = http.StatusGatewayTimeout
 )
 
 // Error is an Athens system error.
diff --git a/pkg/module/go_vcs_lister.go b/pkg/module/go_vcs_lister.go
index 86abcc38..b9e3f041 100644
--- a/pkg/module/go_vcs_lister.go
+++ b/pkg/module/go_vcs_lister.go
@@ -28,15 +28,17 @@ type vcsLister struct {
 	env       []string
 	fs        afero.Fs
 	sfg       *singleflight.Group
+	timeout   time.Duration
 }
 
 // NewVCSLister creates an UpstreamLister which uses VCS to fetch a list of available versions.
-func NewVCSLister(goBinPath string, env []string, fs afero.Fs) UpstreamLister {
+func NewVCSLister(goBinPath string, env []string, fs afero.Fs, timeout time.Duration) UpstreamLister {
 	return &vcsLister{
 		goBinPath: goBinPath,
 		env:       env,
 		fs:        fs,
 		sfg:       &singleflight.Group{},
+		timeout:   timeout,
 	}
 }
 
@@ -56,7 +58,11 @@ func (l *vcsLister) List(ctx context.Context, module string) (*storage.RevInfo,
 		}
 		defer func() { _ = l.fs.RemoveAll(tmpDir) }()
 
-		cmd := exec.Command(
+		timeoutCtx, cancel := context.WithTimeout(ctx, l.timeout)
+		defer cancel()
+
+		cmd := exec.CommandContext(
+			timeoutCtx,
 			l.goBinPath,
 			"list", "-m", "-versions", "-json",
 			config.FmtModVer(module, "latest"),
@@ -77,6 +83,10 @@ func (l *vcsLister) List(ctx context.Context, module string) (*storage.RevInfo,
 		err = cmd.Run()
 		if err != nil {
 			err = fmt.Errorf("%w: %s", err, stderr)
+			if errors.IsErr(timeoutCtx.Err(), context.DeadlineExceeded) {
+				return nil, errors.E(op, err, errors.KindGatewayTimeout)
+			}
+
 			// as of now, we can't recognize between a true NotFound
 			// and an unexpected error, so we choose the more
 			// hopeful path of NotFound. This way the Go command


Test patch:
diff --git a/pkg/download/protocol_test.go b/pkg/download/protocol_test.go
index 1a855984..d7070e01 100644
--- a/pkg/download/protocol_test.go
+++ b/pkg/download/protocol_test.go
@@ -50,7 +50,7 @@ func getDP(t *testing.T) Protocol {
 	return New(&Opts{
 		Storage:     s,
 		Stasher:     st,
-		Lister:      module.NewVCSLister(goBin, conf.GoBinaryEnvVars, fs),
+		Lister:      module.NewVCSLister(goBin, conf.GoBinaryEnvVars, fs, conf.TimeoutDuration()),
 		NetworkMode: Strict,
 	})
 }