From bfa074f6c6d70e72dd68646092b21bd70ed1ec0d Mon Sep 17 00:00:00 2001 From: Noah S-C Date: Tue, 10 Jan 2023 14:53:49 +0000 Subject: [PATCH] codeintel: dont swallow errors in upload retry mechanism (#46281) --- lib/codeintel/upload/retry.go | 19 +++++++++++++------ lib/codeintel/upload/upload.go | 10 +++++++--- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/codeintel/upload/retry.go b/lib/codeintel/upload/retry.go index 493d528a7c8..7c70735827c 100644 --- a/lib/codeintel/upload/retry.go +++ b/lib/codeintel/upload/retry.go @@ -1,6 +1,10 @@ package upload -import "time" +import ( + "time" + + "github.com/sourcegraph/sourcegraph/lib/errors" +) // RetryableFunc is a function that takes the invocation index and returns an error as well as a // boolean-value flag indicating whether or not the error is considered retryable. @@ -15,16 +19,19 @@ func makeRetry(n int, interval time.Duration) func(f RetryableFunc) error { // retry will re-invoke the given function until it returns a nil error value, the function returns // a non-retryable error (as indicated by its boolean return value), or until the maximum number of -// retries have been attempted. The returned error will be the last error to occur. -func retry(f RetryableFunc, n int, interval time.Duration) (err error) { - var retry bool +// retries have been attempted. All errors encountered will be returned. +func retry(f RetryableFunc, n int, interval time.Duration) (errs error) { for i := 0; i <= n; i++ { - if retry, err = f(i); err == nil || !retry { + retry, err := f(i) + + errs = errors.CombineErrors(errs, err) + + if err == nil || !retry { break } time.Sleep(interval) } - return err + return errs } diff --git a/lib/codeintel/upload/upload.go b/lib/codeintel/upload/upload.go index bb22bf3c0bd..a677f03ebaa 100644 --- a/lib/codeintel/upload/upload.go +++ b/lib/codeintel/upload/upload.go @@ -102,8 +102,10 @@ func uploadIndex(ctx context.Context, httpClient Client, opts UploadOptions, r i // uploadIndexFile uploads the contents available via the given reader to a // Sourcegraph instance with the given request options.i -func uploadIndexFile(ctx context.Context, httpClient Client, uploadOptions UploadOptions, reader io.ReadSeeker, readerLen int64, requestOptions uploadRequestOptions, progress output.Progress, retry func(message string) output.Progress, barIndex int, numParts int) error { - return makeRetry(uploadOptions.MaxRetries, uploadOptions.RetryInterval)(func(attempt int) (_ bool, err error) { +func uploadIndexFile(ctx context.Context, httpClient Client, uploadOptions UploadOptions, reader io.ReadSeeker, readerLen int64, requestOptions uploadRequestOptions, progress output.Progress, retry onRetryLogFn, barIndex int, numParts int) error { + retrier := makeRetry(uploadOptions.MaxRetries, uploadOptions.RetryInterval) + + return retrier(func(attempt int) (_ bool, err error) { defer func() { if err != nil && !errors.Is(err, ctx.Err()) && progress != nil { progress.SetValue(barIndex, 0) @@ -337,13 +339,15 @@ func logPending(out *output.Output, pendingMessage, successMessage, failureMessa return retry, complete } +type onRetryLogFn func(message string) output.Progress + // logProgress creates and returns a progress from the given output value and bars configuration. // This function also returns a retry function that can be called to print a message then reset the // progress bar display, and a complete function that should be called once the work attached to // this log call has completed. This complete function takes an error value that determines whether // the success or failure message is displayed. If the given output value is nil then a no-op complete // function is returned. -func logProgress(out *output.Output, bars []output.ProgressBar, successMessage, failureMessage string) (output.Progress, func(message string) output.Progress, func(error)) { +func logProgress(out *output.Output, bars []output.ProgressBar, successMessage, failureMessage string) (output.Progress, onRetryLogFn, func(error)) { if out == nil { return nil, func(message string) output.Progress { return nil }, func(err error) {} }