add timeout to vcsLister.List() (#1986)

This commit is contained in:
Taylor Chen
2024-09-19 23:19:47 -07:00
committed by GitHub
parent 6f1346fdb9
commit 3856c6feee
5 changed files with 16 additions and 5 deletions
+1 -1
View File
@@ -99,7 +99,7 @@ func addProxyRoutes(
return err return err
} }
lister := module.NewVCSLister(c.GoBinary, c.GoBinaryEnvVars, fs) lister := module.NewVCSLister(c.GoBinary, c.GoBinaryEnvVars, fs, c.TimeoutDuration())
checker := storage.WithChecker(s) checker := storage.WithChecker(s)
withSingleFlight, err := getSingleFlight(l, c, s, checker) withSingleFlight, err := getSingleFlight(l, c, s, checker)
if err != nil { if err != nil {
+1 -1
View File
@@ -28,7 +28,7 @@ func ListHandler(dp Protocol, lggr log.Entry, df *mode.DownloadFile) http.Handle
versions, err := dp.List(r.Context(), mod) versions, err := dp.List(r.Context(), mod)
if err != nil { if err != nil {
severityLevel := errors.Expect(err, errors.KindNotFound) severityLevel := errors.Expect(err, errors.KindNotFound, errors.KindGatewayTimeout)
err = errors.E(op, err, severityLevel) err = errors.E(op, err, severityLevel)
lggr.SystemErr(err) lggr.SystemErr(err)
w.WriteHeader(errors.Kind(err)) w.WriteHeader(errors.Kind(err))
+1 -1
View File
@@ -50,7 +50,7 @@ func getDP(t *testing.T) Protocol {
return New(&Opts{ return New(&Opts{
Storage: s, Storage: s,
Stasher: st, Stasher: st,
Lister: module.NewVCSLister(goBin, conf.GoBinaryEnvVars, fs), Lister: module.NewVCSLister(goBin, conf.GoBinaryEnvVars, fs, conf.TimeoutDuration()),
NetworkMode: Strict, NetworkMode: Strict,
}) })
} }
+1
View File
@@ -18,6 +18,7 @@ const (
KindRateLimit = http.StatusTooManyRequests KindRateLimit = http.StatusTooManyRequests
KindNotImplemented = http.StatusNotImplemented KindNotImplemented = http.StatusNotImplemented
KindRedirect = http.StatusMovedPermanently KindRedirect = http.StatusMovedPermanently
KindGatewayTimeout = http.StatusGatewayTimeout
) )
// Error is an Athens system error. // Error is an Athens system error.
+12 -2
View File
@@ -28,15 +28,17 @@ type vcsLister struct {
env []string env []string
fs afero.Fs fs afero.Fs
sfg *singleflight.Group sfg *singleflight.Group
timeout time.Duration
} }
// NewVCSLister creates an UpstreamLister which uses VCS to fetch a list of available versions. // 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{ return &vcsLister{
goBinPath: goBinPath, goBinPath: goBinPath,
env: env, env: env,
fs: fs, fs: fs,
sfg: &singleflight.Group{}, 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) }() defer func() { _ = l.fs.RemoveAll(tmpDir) }()
cmd := exec.Command( timeoutCtx, cancel := context.WithTimeout(ctx, l.timeout)
defer cancel()
cmd := exec.CommandContext(
timeoutCtx,
l.goBinPath, l.goBinPath,
"list", "-m", "-versions", "-json", "list", "-m", "-versions", "-json",
config.FmtModVer(module, "latest"), config.FmtModVer(module, "latest"),
@@ -77,6 +83,10 @@ func (l *vcsLister) List(ctx context.Context, module string) (*storage.RevInfo,
err = cmd.Run() err = cmd.Run()
if err != nil { if err != nil {
err = fmt.Errorf("%w: %s", err, stderr) 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 // as of now, we can't recognize between a true NotFound
// and an unexpected error, so we choose the more // and an unexpected error, so we choose the more
// hopeful path of NotFound. This way the Go command // hopeful path of NotFound. This way the Go command