Bound authentication requests and retry transient network failures

pkg/appstore/appstore.go

@@ -81,7 +81,7 @@ func NewAppStore(args Args) AppStore {
 		downloadClient:      http.NewClient[downloadResult](clientArgs),
 		platformClient:      http.NewClient[platformVersionLookupResult](clientArgs),
 		storefrontClient:    http.NewClient[[]byte](clientArgs),
-		bagClient:           http.NewClient[bagResult](clientArgs),
+		bagClient:           http.NewClient[bagResult](http.Args{CookieJar: args.CookieJar, Timeout: http.DefaultAuthenticationTimeout}),
 		ownedAppsClient:     http.NewClient[[]byte](clientArgs),
 		httpClient:          http.NewClient[interface{}](clientArgs),
 		macDecrypterFactory: defaultMacPackageDecrypterFactory,

pkg/appstore/appstore_login.go

@@ -1,13 +1,17 @@
 package appstore
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
+	"net"
 	gohttp "net/http"
 	"net/url"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 	"unicode"
 
@@ -226,6 +230,7 @@ func (t *appstore) login(email, password, authCode, guid, endpoint string, signe
 
 func (t *appstore) sendAuthenticationRequest(request http.Request) (http.Result[loginResult], error) {
 	statuses := make([]string, 0, maxAuthenticationRequestAttempts)
+	onlyHTTP := true
 
 	sleep := t.authRetrySleep
 	if sleep == nil {
@@ -244,27 +249,48 @@ func (t *appstore) sendAuthenticationRequest(request http.Request) (http.Result[
 			return result, nil
 		}
 
-		statuses = append(statuses, strconv.Itoa(status))
+		outcome := fmt.Sprintf("HTTP %d", status)
+		if status == 0 {
+			outcome = "transport error"
+			onlyHTTP = false
+		}
+
+		statuses = append(statuses, outcome)
 
 		if attempt == maxAuthenticationRequestAttempts {
+			summary := strings.Join(statuses, ", ")
+			if onlyHTTP {
+				// Preserve the existing diagnostic format for HTTP-only failures.
+				summary = strings.ReplaceAll(summary, ", HTTP ", ", ")
+			}
+
 			return result, fmt.Errorf(
-				"authentication request failed after %d attempts (HTTP %s): %w",
-				maxAuthenticationRequestAttempts, strings.Join(statuses, ", "), authenticationRequestError(err),
+				"authentication request failed after %d attempts (%s): %w",
+				maxAuthenticationRequestAttempts, summary, authenticationRequestError(err),
 			)
 		}
 
 		delay := min(authenticationRetryDelay<<(attempt-1), maxAuthenticationRetryDelay)
 
-		var responseErr *http.UnexpectedResponseError
+		var (
+			responseErr  *http.UnexpectedResponseError
+			transportErr *http.TransportError
+			retryAfter   string
+		)
+
 		if errors.As(err, &responseErr) {
-			if requested, ok := authenticationRetryAfter(responseErr.RetryAfter, time.Now()); ok {
-				if requested > maxAuthenticationRetryDelay {
-					return result, fmt.Errorf("apple requested a wait longer than %s; try again later: %w", maxAuthenticationRetryDelay, err)
-				}
+			retryAfter = responseErr.RetryAfter
+		} else if errors.As(err, &transportErr) {
+			retryAfter = transportErr.RetryAfter
+		}
 
-				// Retry-After takes precedence over the fallback backoff.
-				delay = max(requested, time.Second)
+		if requested, ok := authenticationRetryAfter(retryAfter, time.Now()); ok {
+			if requested > maxAuthenticationRetryDelay {
+				return result, fmt.Errorf("apple requested a wait longer than %s; try again later: %w", maxAuthenticationRetryDelay, err)
 			}
+
+			// Retry-After takes precedence over the fallback backoff.
+			delay = max(requested, time.Second)
 		}
 
 		sleep(delay)
@@ -274,7 +300,21 @@ func (t *appstore) sendAuthenticationRequest(request http.Request) (http.Result[
 func retryableAuthenticationError(err error) (int, bool) {
 	var responseErr *http.UnexpectedResponseError
 	if !errors.As(err, &responseErr) {
-		return 0, false
+		var transportErr *http.TransportError
+		if !errors.As(err, &transportErr) || errors.Is(err, context.Canceled) {
+			return 0, false
+		}
+
+		// A url.Error can report Timeout false when its cause is wrapped by
+		// AddHeaderTransport, so inspect each underlying error as well.
+		for cause := transportErr.Err; cause != nil; cause = errors.Unwrap(cause) {
+			if networkErr, ok := cause.(net.Error); ok && networkErr.Timeout() {
+				return 0, true
+			}
+		}
+
+		return 0, errors.Is(transportErr, io.EOF) || errors.Is(transportErr, io.ErrUnexpectedEOF) ||
+			errors.Is(transportErr, syscall.ECONNRESET) || errors.Is(transportErr, syscall.EPIPE)
 	}
 
 	status := responseErr.StatusCode

pkg/appstore/authentication_retry_test.go

@@ -1,12 +1,19 @@
 package appstore
 
 import (
+	"context"
+	"crypto/x509"
 	"encoding/base64"
 	"errors"
+	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"net/http/cookiejar"
 	"net/http/httptest"
+	"net/url"
+	"sync/atomic"
+	"syscall"
 	"time"
 
 	"github.com/majd/ipatool/v2/pkg/keychain"
@@ -19,6 +26,71 @@ import (
 )
 
 var _ = Describe("Authentication recovery", func() {
+	DescribeTable("bounds transport retries and preserves the underlying error", func(cause error, transportFailure bool, calls int) {
+		ctrl := gomock.NewController(GinkgoT())
+		client := apphttp.NewMockClient[loginResult](ctrl)
+		failure := cause
+		if transportFailure {
+			failure = &apphttp.TransportError{Err: cause}
+		}
+		failure = fmt.Errorf("request failed: %w", failure)
+		client.EXPECT().Send(gomock.Any()).Return(apphttp.Result[loginResult]{}, failure).Times(calls)
+		var waits []time.Duration
+		sut := &appstore{loginClient: client, authRetrySleep: func(delay time.Duration) { waits = append(waits, delay) }}
+		_, err := sut.sendAuthenticationRequest(apphttp.Request{})
+		Expect(errors.Is(err, cause)).To(BeTrue())
+		if calls == 1 {
+			Expect(waits).To(BeEmpty())
+		} else {
+			Expect(waits).To(Equal([]time.Duration{10 * time.Second, 20 * time.Second}))
+			Expect(err.Error()).To(ContainSubstring("after 3 attempts (transport error, transport error, transport error)"))
+		}
+	},
+		Entry("request deadline", context.DeadlineExceeded, true, 3),
+		Entry("request deadline wrapped by the header transport", &url.Error{Op: "Post", URL: testAuthEndpoint, Err: fmt.Errorf("failed to make round trip: %w", context.DeadlineExceeded)}, true, 3),
+		Entry("wrapped DNS timeout", &url.Error{Op: "Post", URL: testAuthEndpoint, Err: &net.DNSError{IsTimeout: true}}, true, 3),
+		Entry("DNS timeout wrapped by the header transport", &url.Error{Op: "Post", URL: testAuthEndpoint, Err: fmt.Errorf("failed to make round trip: %w", &net.DNSError{IsTimeout: true})}, true, 3),
+		Entry("connection closed", io.EOF, true, 3),
+		Entry("truncated response body", io.ErrUnexpectedEOF, true, 3),
+		Entry("connection reset", &net.OpError{Op: "read", Net: "tcp", Err: syscall.ECONNRESET}, true, 3),
+		Entry("broken pipe", &net.OpError{Op: "write", Net: "tcp", Err: syscall.EPIPE}, true, 3),
+		Entry("canceled request", context.Canceled, true, 1),
+		Entry("canceled request wrapped by the header transport", &url.Error{Op: "Post", URL: testAuthEndpoint, Err: fmt.Errorf("failed to make round trip: %w", context.Canceled)}, true, 1),
+		Entry("permanent DNS failure", &net.DNSError{IsNotFound: true}, true, 1),
+		Entry("certificate failure", &url.Error{Op: "Post", URL: testAuthEndpoint, Err: x509.UnknownAuthorityError{}}, true, 1),
+		Entry("signing or cookie storage timeout", context.DeadlineExceeded, false, 1),
+		Entry("local EOF", io.EOF, false, 1),
+		Entry("other transport failure", errors.New("unrecoverable transport error"), true, 1),
+	)
+
+	It("shares the attempt budget with HTTP failures and still honors Retry-After", func() {
+		ctrl := gomock.NewController(GinkgoT())
+		client := apphttp.NewMockClient[loginResult](ctrl)
+		last := &apphttp.TransportError{Err: context.DeadlineExceeded}
+		gomock.InOrder(
+			client.EXPECT().Send(gomock.Any()).Return(apphttp.Result[loginResult]{}, &apphttp.TransportError{Err: io.EOF}),
+			client.EXPECT().Send(gomock.Any()).Return(apphttp.Result[loginResult]{}, &apphttp.UnexpectedResponseError{StatusCode: 503, RetryAfter: "2"}),
+			client.EXPECT().Send(gomock.Any()).Return(apphttp.Result[loginResult]{}, last),
+		)
+		var waits []time.Duration
+		sut := &appstore{loginClient: client, authRetrySleep: func(delay time.Duration) { waits = append(waits, delay) }}
+		_, err := sut.sendAuthenticationRequest(apphttp.Request{})
+		Expect(errors.Is(err, last)).To(BeTrue())
+		Expect(waits).To(Equal([]time.Duration{10 * time.Second, 2 * time.Second}))
+		Expect(err.Error()).To(ContainSubstring("after 3 attempts (transport error, HTTP 503, transport error)"))
+	})
+
+	It("stops when an incomplete response requests a wait beyond the budget", func() {
+		ctrl := gomock.NewController(GinkgoT())
+		client := apphttp.NewMockClient[loginResult](ctrl)
+		failure := &apphttp.TransportError{Err: context.DeadlineExceeded, RetryAfter: "3600"}
+		client.EXPECT().Send(gomock.Any()).Return(apphttp.Result[loginResult]{}, failure).Times(1)
+		sut := &appstore{loginClient: client, authRetrySleep: func(time.Duration) { Fail("must not sleep") }}
+		_, err := sut.sendAuthenticationRequest(apphttp.Request{})
+		Expect(errors.Is(err, failure)).To(BeTrue())
+		Expect(err.Error()).To(ContainSubstring("apple requested a wait longer than 30s"))
+	})
+
 	DescribeTable("honors Retry-After without exceeding the wait budget",
 		func(status int, header string, calls int, expectedWaits []time.Duration) {
 			ctrl := gomock.NewController(GinkgoT())
@@ -91,6 +163,55 @@ type loginSessionJar struct{ *cookiejar.Jar }
 func (loginSessionJar) Save() error { return nil }
 
 var _ = Describe("Login session continuity", func() {
+	It("recovers from a body timeout with cookies and a newly signed identical payload", func() {
+		jar, err := cookiejar.New(nil)
+		Expect(err).NotTo(HaveOccurred())
+		client := apphttp.NewClient[loginResult](apphttp.Args{CookieJar: loginSessionJar{jar}, Authentication: true, Timeout: 250 * time.Millisecond})
+		var calls atomic.Int32
+		bodies := make(chan []byte, 2)
+		releaseResponse := make(chan struct{})
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			defer GinkgoRecover()
+			call := calls.Add(1)
+			Expect(r.Method).To(Equal(http.MethodPost))
+			body, err := io.ReadAll(r.Body)
+			Expect(err).NotTo(HaveOccurred())
+			bodies <- body
+			signature, err := base64.StdEncoding.DecodeString(r.Header.Get(apphttp.HeaderAppleActionSignature))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(signature).To(Equal(body))
+			if call == 1 {
+				http.SetCookie(w, &http.Cookie{Name: "session", Value: "retained", Path: "/"})
+				w.Header().Set("Retry-After", "2")
+				w.WriteHeader(http.StatusTooManyRequests)
+				_, err = w.Write([]byte("<dict>"))
+				Expect(err).NotTo(HaveOccurred())
+				w.(http.Flusher).Flush()
+				<-releaseResponse
+
+				return
+			}
+			Expect(call).To(Equal(int32(2)))
+			cookie, err := r.Cookie("session")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(cookie.Value).To(Equal("retained"))
+			_, err = w.Write([]byte("<dict><key>passwordToken</key><string>token</string><key>dsPersonId</key><string>123</string></dict>"))
+			Expect(err).NotTo(HaveOccurred())
+		}))
+		DeferCleanup(srv.Close)
+		DeferCleanup(func() { close(releaseResponse) })
+		var waits []time.Duration
+		sut := &appstore{loginClient: client, authRetrySleep: func(delay time.Duration) { waits = append(waits, delay) }}
+		signer := &stubActionSigner{}
+		request := sut.loginRequest("email", "password", "123456", "guid", srv.URL+PrivateAppStoreAPIPathAuth, 1, signer)
+		result, err := sut.sendAuthenticationRequest(request)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result.Data.PasswordToken).To(Equal("token"))
+		Expect(waits).To(Equal([]time.Duration{2 * time.Second}))
+		Expect(signer.signCalls).To(Equal(2))
+		Expect(<-bodies).To(Equal(<-bodies))
+	})
+
 	It("retains cookies and the signed payload through retries, a pod redirect and 2FA", func() {
 		ctrl := gomock.NewController(GinkgoT())
 		jar, err := cookiejar.New(nil)

pkg/http/client.go

@@ -7,15 +7,19 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"regexp"
 	"strings"
+	"time"
 
 	"howett.net/plist"
 )
 
 const appStoreAuthPath = "/WebObjects/MZFinance.woa/wa/authenticate"
 
+const DefaultAuthenticationTimeout = 30 * time.Second
+
 // IsAuthenticationRedirect reports which Store redirects can replay a signed POST.
 // A 303 explicitly requests a GET and is not part of this authentication flow.
 func IsAuthenticationRedirect(status int) bool {
@@ -51,6 +55,9 @@ type Args struct {
 	CookieJar CookieJar
 	// Authentication isolates login connections and reports only allowlisted response diagnostics.
 	Authentication bool
+	// Timeout covers the request and response body. Zero uses the authentication
+	// default for login clients and leaves other clients without an overall limit.
+	Timeout time.Duration
 }
 
 // UnexpectedResponseError preserves the HTTP status when Apple returns an
@@ -96,19 +103,25 @@ func (t *AddHeaderTransport) RoundTrip(req *http.Request) (*http.Response, error
 
 func NewClient[R interface{}](args Args) Client[R] {
 	transport := http.DefaultTransport
+	timeout := args.Timeout
 
 	if args.Authentication {
+		if timeout == 0 {
+			timeout = DefaultAuthenticationTimeout
+		}
+
 		// Keep the cookie jar, but do not share pooled connections with other
 		// Store operations or reuse a connection for the next login attempt.
 		isolated := http.DefaultTransport.(*http.Transport).Clone()
 		isolated.DisableKeepAlives = true
+		isolated.DialContext = (&net.Dialer{Timeout: 15 * time.Second, KeepAlive: 30 * time.Second}).DialContext
 		transport = isolated
 	}
 
 	return &client[R]{
 		authentication: args.Authentication,
 		internalClient: http.Client{
-			Timeout: 0,
+			Timeout: timeout,
 			Jar:     args.CookieJar,
 			CheckRedirect: func(req *http.Request, via []*http.Request) error {
 				// Login validates destinations and replays the signed POST itself.
@@ -159,7 +172,7 @@ func (c *client[R]) Send(req Request) (Result[R], error) {
 
 	res, err := c.internalClient.Do(request)
 	if err != nil {
-		return Result[R]{}, fmt.Errorf("request failed: %w", err)
+		return Result[R]{}, fmt.Errorf("request failed: %w", &TransportError{Err: err})
 	}
 	defer res.Body.Close()
 
@@ -186,7 +199,7 @@ func (c *client[R]) Send(req Request) (Result[R], error) {
 func (c *client[R]) handleRawResponse(res *http.Response) (Result[R], error) {
 	body, err := io.ReadAll(res.Body)
 	if err != nil {
-		return Result[R]{}, fmt.Errorf("failed to read response body: %w", err)
+		return Result[R]{}, fmt.Errorf("failed to read response body: %w", &TransportError{Err: err, RetryAfter: res.Header.Get("Retry-After")})
 	}
 
 	data, ok := any(body).(R)
@@ -222,7 +235,7 @@ func (*client[R]) NewRequest(method, url string, body io.Reader) (*http.Request,
 func (c *client[R]) handleJSONResponse(res *http.Response) (Result[R], error) {
 	body, err := io.ReadAll(res.Body)
 	if err != nil {
-		return Result[R]{}, fmt.Errorf("failed to read response body: %w", err)
+		return Result[R]{}, fmt.Errorf("failed to read response body: %w", &TransportError{Err: err, RetryAfter: res.Header.Get("Retry-After")})
 	}
 
 	var data R
@@ -241,7 +254,7 @@ func (c *client[R]) handleJSONResponse(res *http.Response) (Result[R], error) {
 func (c *client[R]) handleXMLResponse(res *http.Response) (Result[R], error) {
 	body, err := io.ReadAll(res.Body)
 	if err != nil {
-		return Result[R]{}, fmt.Errorf("failed to read response body: %w", err)
+		return Result[R]{}, fmt.Errorf("failed to read response body: %w", &TransportError{Err: err, RetryAfter: res.Header.Get("Retry-After")})
 	}
 
 	if c.authentication && IsAuthenticationRedirect(res.StatusCode) && strings.TrimSpace(res.Header.Get("Location")) == "" {