From 3856c6feee6e9ec0772e885f73ad9446d84abd21 Mon Sep 17 00:00:00 2001 From: Taylor Chen <95328002+taylorychen@users.noreply.github.com> Date: Thu, 19 Sep 2024 23:19:47 -0700 Subject: [PATCH] add timeout to vcsLister.List() (#1986) --- cmd/proxy/actions/app_proxy.go | 2 +- pkg/download/list.go | 2 +- pkg/download/protocol_test.go | 2 +- pkg/errors/errors.go | 1 + pkg/module/go_vcs_lister.go | 14 ++++++++++++-- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/cmd/proxy/actions/app_proxy.go b/cmd/proxy/actions/app_proxy.go index 0117bc9db..86c2a2478 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 04971cdb5..16188db8c 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/download/protocol_test.go b/pkg/download/protocol_test.go index 1a8559842..d7070e013 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, }) } diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 6b502dc1b..b2378b19d 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 86abcc38c..b9e3f041b 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