fix: improve retry handling for expired HTTP redirect url (#1263)

internal/protocol/http/fetcher.go

@@ -588,6 +588,10 @@ func (f *Fetcher) Start() error {
 		// Already downloading, this is a resume from pause
 		return f.doStart()
 
+	case stateError:
+		// Retry after error: reset and restart
+		return f.doStart()
+
 	default:
 		return fmt.Errorf("cannot start in current state: %v", state)
 	}
@@ -598,10 +602,31 @@ func (f *Fetcher) doStart() error {
 	<-f.resolvedCh
 
 	state := f.getState()
-	if state == stateDone || state == stateError {
+	if state == stateDone {
 		return nil
 	}
 
+	// If retrying after error, reset connection states for retry
+	if state == stateError {
+		// Drain any pending error from doneCh before retry
+		select {
+		case <-f.doneCh:
+		default:
+		}
+
+		f.connMu.Lock()
+		for _, conn := range f.connections {
+			// Reset connections that can be retried
+			if !conn.Completed && conn.State != connCompleted {
+				conn.State = connNotStarted
+				conn.failed = false
+				conn.retryTimes = 0
+				conn.lastErr = nil
+			}
+		}
+		f.connMu.Unlock()
+	}
+
 	// Open or create target file first (needed for prefetch copy)
 	name := f.meta.SingleFilepath()
 	var err error
@@ -948,6 +973,8 @@ func (f *Fetcher) runConnection(conn *connection) {
 }
 
 // downloadChunkOnce performs a single HTTP request for the current chunk without retrying.
+// If the redirect URL fails with an expiration-related error (401, 403, 410),
+// it will automatically retry with the original URL and update the redirect URL on success.
 func (f *Fetcher) downloadChunkOnce(conn *connection, client *http.Client, buf []byte) error {
 	if conn.ctx.Err() != nil {
 		return conn.ctx.Err()
@@ -981,11 +1008,32 @@ func (f *Fetcher) downloadChunkOnce(conn *connection, client *http.Client, buf [
 	if err != nil {
 		return err
 	}
-	defer resp.Body.Close()
 
 	if resp.StatusCode != base.HttpCodeOK && resp.StatusCode != base.HttpCodePartialContent {
-		return NewRequestError(resp.StatusCode)
+		resp.Body.Close()
+		originalErr := NewRequestError(resp.StatusCode)
+
+		// Check if this might be a redirect URL expiration error
+		// If so, try falling back to the original URL
+		if f.hasRedirectURL() && isRedirectExpiredError(originalErr) {
+			fallbackResp, fallbackErr := f.tryFallbackToOriginalURL(conn.ctx, client, rangeStart, rangeEnd)
+			if fallbackErr == nil && fallbackResp != nil {
+				// Fallback succeeded, use this response instead
+				resp = fallbackResp
+				// Update the redirect URL from the response
+				f.updateRedirectURL(resp)
+			} else {
+				// Fallback also failed, return the original error
+				if fallbackResp != nil {
+					fallbackResp.Body.Close()
+				}
+				return originalErr
+			}
+		} else {
+			return originalErr
+		}
 	}
+	defer resp.Body.Close()
 
 	// Record successful connection time for adaptive timeout
 	f.updateMaxConnTime(time.Since(connStartTime))

internal/protocol/http/fetcher_manager.go

@@ -15,6 +15,7 @@ import (
 
 type fetcherData struct {
 	Connections []*connection
+	RedirectURL string // Saved redirect URL for resume
 }
 
 // ============================================================================
@@ -71,8 +72,12 @@ func (fm *FetcherManager) DefaultConfig() any {
 
 func (fm *FetcherManager) Store(f fetcher.Fetcher) (data any, err error) {
 	_f := f.(*Fetcher)
+	_f.redirectLock.Lock()
+	redirectURL := _f.redirectURL
+	_f.redirectLock.Unlock()
 	return &fetcherData{
 		Connections: _f.connections,
+		RedirectURL: redirectURL,
 	}, nil
 }
 
@@ -87,6 +92,10 @@ func (fm *FetcherManager) Restore() (v any, f func(meta *fetcher.FetcherMeta, v
 		if len(fd.Connections) > 0 {
 			fetcher.connections = fd.Connections
 		}
+		// Restore redirect URL for resume
+		if fd.RedirectURL != "" {
+			fetcher.redirectURL = fd.RedirectURL
+		}
 		return fetcher
 	}
 }

internal/protocol/http/fetcher_test.go

@@ -778,6 +778,168 @@ func TestFetcher_AsyncPrefetch(t *testing.T) {
 	})
 }
 
+// TestFetcher_DownloadExpiringRedirectURL tests that the fetcher correctly handles
+// expiring redirect URLs by falling back to the original URL and getting a new redirect.
+func TestFetcher_DownloadExpiringRedirectURL(t *testing.T) {
+	// Test with redirect expiring after 2 requests
+	// This means:
+	// - Request 1: Resolve (original URL redirects to temp URL v1)
+	// - Request 2: First download request to temp URL v1 succeeds
+	// - Request 3: Second download request to temp URL v1 returns 403
+	// - Fetcher should then retry with original URL, get temp URL v2
+	// - Continue downloading from temp URL v2
+	t.Run("RedirectExpiresAfter2Requests", func(t *testing.T) {
+		os.Remove(test.DownloadFile)
+
+		// Create server with redirect expiring after 2 requests, with slow transfer to ensure
+		// multiple connection attempts are needed
+		listener := test.StartTestExpiringRedirectServer(2, 100*time.Nanosecond)
+		defer listener.Close()
+
+		fetcher := buildFetcher()
+		err := fetcher.Resolve(&base.Request{
+			URL: "http://" + listener.Addr().String() + "/" + test.BuildName,
+		}, &base.Options{
+			Name: test.DownloadName,
+			Path: test.Dir,
+			Extra: &http.OptsExtra{
+				Connections: 4, // Use multiple connections to trigger redirect expiration
+			},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = fetcher.Start()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = fetcher.Wait()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Verify file content is correct despite redirect expiration
+		want := test.FileMd5(test.BuildFile)
+		got := test.FileMd5(test.DownloadFile)
+		if want != got {
+			t.Errorf("Download() got = %v, want %v", got, want)
+		}
+
+		os.Remove(test.DownloadFile)
+	})
+
+	// Test with redirect expiring after 5 requests (more room for initial connections)
+	t.Run("RedirectExpiresAfter5Requests", func(t *testing.T) {
+		os.Remove(test.DownloadFile)
+
+		listener := test.StartTestExpiringRedirectServer(5, 100*time.Nanosecond)
+		defer listener.Close()
+
+		fetcher := buildFetcher()
+		err := fetcher.Resolve(&base.Request{
+			URL: "http://" + listener.Addr().String() + "/" + test.BuildName,
+		}, &base.Options{
+			Name: test.DownloadName,
+			Path: test.Dir,
+			Extra: &http.OptsExtra{
+				Connections: 8,
+			},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = fetcher.Start()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = fetcher.Wait()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Verify file content
+		want := test.FileMd5(test.BuildFile)
+		got := test.FileMd5(test.DownloadFile)
+		if want != got {
+			t.Errorf("Download() got = %v, want %v", got, want)
+		}
+
+		os.Remove(test.DownloadFile)
+	})
+}
+
+// TestFetcher_RetryAfterError tests that the fetcher can retry downloading
+// after a previous download attempt failed by calling Start() again.
+func TestFetcher_RetryAfterError(t *testing.T) {
+	os.Remove(test.DownloadFile)
+
+	// Server fails first 3 requests (after resolve), then recovers
+	// With 1 connection and 3 retries:
+	// - First Start(): requests 2, 3, 4 → all fail (3 retries exhausted) → returns error
+	// - Second Start(): request 5 → succeeds (server recovered after 3 failures)
+	listener := test.StartTestFailThenRecoverServer(3)
+	defer listener.Close()
+
+	fetcher := buildFetcher()
+	err := fetcher.Resolve(&base.Request{
+		URL: "http://" + listener.Addr().String() + "/" + test.BuildName,
+	}, &base.Options{
+		Name: test.DownloadName,
+		Path: test.Dir,
+		Extra: &http.OptsExtra{
+			Connections: 1, // Use single connection to simplify test
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// First download attempt - should fail because server returns 416 after resolve
+	err = fetcher.Start()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = fetcher.Wait()
+	// First attempt should fail with 416 error
+	if err == nil {
+		t.Fatal("Expected first download attempt to fail, but it succeeded")
+	}
+	t.Logf("First attempt failed as expected: %v", err)
+
+	// Check that fetcher is in error state
+	state := fetcher.getState()
+	if state != stateError {
+		t.Errorf("Expected fetcher to be in stateError, got %v", state)
+	}
+
+	// Verify that we can call Start() again after error
+	// This tests the stateError handling in Start()
+	err = fetcher.Start()
+	if err != nil {
+		t.Fatalf("Start() after error failed: %v", err)
+	}
+
+	// Wait for second attempt - should succeed now that server has recovered
+	err = fetcher.Wait()
+	if err != nil {
+		t.Fatalf("Retry failed: %v", err)
+	}
+
+	// Verify file content
+	want := test.FileMd5(test.BuildFile)
+	got := test.FileMd5(test.DownloadFile)
+	if want != got {
+		t.Errorf("Download() got = %v, want %v", got, want)
+	}
+
+	os.Remove(test.DownloadFile)
+}
+
 func TestFetcherManager_ParseName(t *testing.T) {
 	type args struct {
 		u string

internal/protocol/http/helper.go

@@ -65,14 +65,23 @@ func extractRequestError(err error) *RequestError {
 	return nil
 }
 
-// ============================================================================
-// HTTP Request/Client Building
-// ============================================================================
-
+// buildRequest creates an HTTP request using the redirect URL if available.
 func (f *Fetcher) buildRequest(ctx context.Context, req *base.Request) (httpReq *http.Request, err error) {
+	return f.buildRequestWithURL(ctx, req, true)
+}
+
+// buildRequestWithOriginalURL creates an HTTP request using the original URL.
+// This is used for retrying when the redirect URL has expired.
+func (f *Fetcher) buildRequestWithOriginalURL(ctx context.Context, req *base.Request) (httpReq *http.Request, err error) {
+	return f.buildRequestWithURL(ctx, req, false)
+}
+
+// buildRequestWithURL creates an HTTP request.
+// If useRedirect is true and a redirect URL exists, it will be used; otherwise the original URL is used.
+func (f *Fetcher) buildRequestWithURL(ctx context.Context, req *base.Request, useRedirect bool) (httpReq *http.Request, err error) {
 	var reqUrl string
 	f.redirectLock.Lock()
-	if f.redirectURL != "" {
+	if useRedirect && f.redirectURL != "" {
 		reqUrl = f.redirectURL
 	} else {
 		reqUrl = req.URL
@@ -121,6 +130,68 @@ func (f *Fetcher) buildRequest(ctx context.Context, req *base.Request) (httpReq
 	return httpReq, nil
 }
 
+// updateRedirectURL updates the redirect URL from the response.
+// This is called when a request using the original URL succeeds after the redirect URL expired.
+func (f *Fetcher) updateRedirectURL(resp *http.Response) {
+	if resp != nil && resp.Request != nil {
+		newRedirectURL := resp.Request.URL.String()
+		f.redirectLock.Lock()
+		f.redirectURL = newRedirectURL
+		f.redirectLock.Unlock()
+	}
+}
+
+// hasRedirectURL checks if a redirect URL exists and is different from the original URL.
+func (f *Fetcher) hasRedirectURL() bool {
+	f.redirectLock.Lock()
+	defer f.redirectLock.Unlock()
+	return f.redirectURL != "" && f.redirectURL != f.meta.Req.URL
+}
+
+// isRedirectExpiredError checks if the error indicates that the redirect URL may have expired.
+// This includes 403 (Forbidden), 401 (Unauthorized), 410 (Gone), and network errors.
+func isRedirectExpiredError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	// Check for specific HTTP error codes that might indicate URL expiration
+	if re := extractRequestError(err); re != nil {
+		switch re.Code {
+		case 401, 403, 404, 410:
+			return true
+		}
+	}
+
+	return false
+}
+
+// tryFallbackToOriginalURL attempts to make a request using the original URL
+// when the redirect URL has expired. Returns the response if successful.
+func (f *Fetcher) tryFallbackToOriginalURL(ctx context.Context, client *http.Client, rangeStart, rangeEnd int64) (*http.Response, error) {
+	httpReq, err := f.buildRequestWithOriginalURL(ctx, f.meta.Req)
+	if err != nil {
+		return nil, err
+	}
+
+	if f.meta.Res.Range && rangeEnd > 0 {
+		httpReq.Header.Set(base.HttpHeaderRange,
+			fmt.Sprintf(base.HttpHeaderRangeFormat, rangeStart, rangeEnd))
+	}
+
+	resp, err := client.Do(httpReq)
+	if err != nil {
+		return nil, err
+	}
+
+	if resp.StatusCode != base.HttpCodeOK && resp.StatusCode != base.HttpCodePartialContent {
+		resp.Body.Close()
+		return nil, NewRequestError(resp.StatusCode)
+	}
+
+	return resp, nil
+}
+
 // buildClient creates an HTTP client with the default connection timeout.
 // Used for resolve phase where we don't have connection time data yet.
 func (f *Fetcher) buildClient() *http.Client {

internal/test/httptest.go

@@ -335,6 +335,67 @@ func StartTestOneTimeServer() net.Listener {
 	})
 }
 
+// StartTestExpiringRedirectServer creates a server that simulates expiring redirect URLs.
+// The original URL redirects to a temporary URL that expires after a specified number of requests.
+// When the temporary URL expires (returns 403), the client should retry with the original URL
+// to get a new redirect URL.
+// Parameters:
+//   - requestsBeforeExpire: number of requests the temporary URL accepts before expiring
+//   - delayPerByte: optional delay per byte for slow transfer (use 0 for no delay)
+func StartTestExpiringRedirectServer(requestsBeforeExpire int32, delayPerByte time.Duration) net.Listener {
+	var redirectVersion atomic.Int32
+	var requestCount atomic.Int32
+	redirectVersion.Store(1)
+
+	return startTestServer(func(sl *shutdownListener) http.Handler {
+		return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+			path := request.URL.Path
+
+			// Original URL - redirects to current version of temporary URL
+			if path == "/"+BuildName {
+				// Reset request count when original URL is accessed
+				requestCount.Store(0)
+				// Redirect to versioned temporary URL
+				version := redirectVersion.Load()
+				redirectURL := fmt.Sprintf("/redirect-v%d/%s", version, BuildName)
+				http.Redirect(writer, request, redirectURL, http.StatusFound)
+				return
+			}
+
+			// Temporary URL handler - matches /redirect-v{N}/... pattern
+			if strings.HasPrefix(path, "/redirect-v") {
+				// Check if the redirect has expired
+				count := requestCount.Add(1)
+				if count > requestsBeforeExpire {
+					// Redirect expired - increment version for next redirect
+					redirectVersion.Add(1)
+					writer.WriteHeader(403)
+					writer.Write([]byte("Redirect URL expired"))
+					return
+				}
+
+				// Serve the file with range support
+				rangeFileHandle(
+					writer,
+					request,
+					nil,
+					func(file *os.File, n int64) {
+						if delayPerByte > 0 {
+							slowCopyNWithDelay(sl, writer, file, n, delayPerByte)
+						} else {
+							io.CopyN(writer, file, n)
+						}
+					},
+				)
+				return
+			}
+
+			// Not found
+			writer.WriteHeader(404)
+		})
+	})
+}
+
 // StartTestSlowStartServer creates a server with configurable delay per request
 // This allows testing slow-start connection expansion to reach max connections
 func StartTestSlowStartServer(delayPerByte time.Duration) net.Listener {
@@ -496,6 +557,57 @@ func StartTestTemporary500Server(errorDuration time.Duration) net.Listener {
 	})
 }
 
+// StartTestFailThenRecoverServer creates a server that fails all connections initially,
+// then recovers after a specified number of failed requests.
+// This tests the retry functionality when calling Start() again after a download fails.
+// Parameters:
+//   - failedRequestsBeforeRecover: number of requests that will fail before server recovers
+//
+// Note: Uses 416 (Range Not Satisfiable) instead of 500 because 5xx errors are exempt
+// from failure counting and will retry indefinitely. 416 is counted as a failure.
+func StartTestFailThenRecoverServer(failedRequestsBeforeRecover int32) net.Listener {
+	var requestCount atomic.Int32
+
+	return startTestServer(func(sl *shutdownListener) http.Handler {
+		mux := http.NewServeMux()
+		mux.HandleFunc("/"+BuildName, func(writer http.ResponseWriter, request *http.Request) {
+			reqNum := requestCount.Add(1)
+
+			// First request (Resolve) always succeeds
+			if reqNum == 1 {
+				rangeFileHandle(
+					writer,
+					request,
+					nil,
+					func(file *os.File, n int64) {
+						io.CopyN(writer, file, n)
+					},
+				)
+				return
+			}
+
+			// Subsequent requests fail until we've had enough failures
+			// Use 416 Range Not Satisfiable - this error is counted towards failure limit
+			if reqNum <= failedRequestsBeforeRecover+1 { // +1 because first request is resolve
+				writer.WriteHeader(416)
+				writer.Write([]byte("Range Not Satisfiable"))
+				return
+			}
+
+			// After enough failures, server recovers
+			rangeFileHandle(
+				writer,
+				request,
+				nil,
+				func(file *os.File, n int64) {
+					io.CopyN(writer, file, n)
+				},
+			)
+		})
+		return mux
+	})
+}
+
 // StartTestRangeBugServer simulate bug server:
 // Don't follow Range request rules, always return more data than range, e.g. Range: bytes=0-100, return 150 bytes
 func StartTestRangeBugServer() net.Listener {