Skip to content

Commit

Permalink
add timeout to vcsLister.List() (gomods#1986)
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorychen authored Sep 20, 2024
1 parent 6f1346f commit 3856c6f
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cmd/proxy/actions/app_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/download/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion pkg/download/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
Expand Down
1 change: 1 addition & 0 deletions pkg/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
KindRateLimit = http.StatusTooManyRequests
KindNotImplemented = http.StatusNotImplemented
KindRedirect = http.StatusMovedPermanently
KindGatewayTimeout = http.StatusGatewayTimeout
)

// Error is an Athens system error.
Expand Down
14 changes: 12 additions & 2 deletions pkg/module/go_vcs_lister.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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"),
Expand All @@ -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
Expand Down

0 comments on commit 3856c6f

Please sign in to comment.