diff --git a/go/pkg/basecamp/account_service.go b/go/pkg/basecamp/account_service.go index 09333a6d4..62143e068 100644 --- a/go/pkg/basecamp/account_service.go +++ b/go/pkg/basecamp/account_service.go @@ -218,15 +218,15 @@ func (s *AccountService) UpdateLogo(ctx context.Context, logo io.Reader, filenam } // Delegate to the generated client — retry and auth are handled by the transport. - // Pass a rewindable reader so doWithRetry can replay the body on transient failures. + // Pass a *bytes.Reader so net/http snapshots it into GetBody and doWithRetry + // can replay the body on transient failures. bodyBytes := buf.Bytes() multipartContentType := writer.FormDataContentType() - rewindable := &rewindableReader{data: bodyBytes} resp, err := s.client.parent.gen.UpdateAccountLogoWithBodyWithResponse( ctx, s.client.accountID, multipartContentType, - rewindable, + bytes.NewReader(bodyBytes), ) if err != nil { return err diff --git a/go/pkg/basecamp/generated_withbody_replay_test.go b/go/pkg/basecamp/generated_withbody_replay_test.go new file mode 100644 index 000000000..abffed03d --- /dev/null +++ b/go/pkg/basecamp/generated_withbody_replay_test.go @@ -0,0 +1,717 @@ +package basecamp + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/basecamp/basecamp-sdk/go/pkg/generated" +) + +// These tests pin the generated client's request-body replay contract for +// idempotent raw *WithBody operations. buildRequest recreates each attempt's +// request from the caller's single-use io.Reader, so without replay support a +// retry ships an empty (or mid-stream) body. doWithRetry snapshots the first +// finalized request's body once and replays THAT body on every later attempt. +// They drive the loop through an exported idempotent *WithBody op +// (UpdateMessageWithBody, a PUT flagged idempotent), staying outside +// pkg/generated per the repo rule that generated code carries no hand-written +// tests. + +// attemptRecord captures what a fake transport observed for one attempt. +type attemptRecord struct { + body []byte // bytes read from req.Body on the wire + getBodyNil bool // whether req.GetBody was nil + getBodyData []byte // bytes produced by invoking req.GetBody (if present) + contentLen int64 // req.ContentLength +} + +// recordingDoer reads each request's body (as a real transport would), records +// what it saw, and replies with the status returned by statusFor(attemptNumber). +func recordingDoer(records *[]attemptRecord, statusFor func(attempt int) int) generated.HttpRequestDoer { + return doerFunc(func(req *http.Request) (*http.Response, error) { + rec := attemptRecord{contentLen: req.ContentLength, getBodyNil: req.GetBody == nil} + if req.GetBody != nil { + if rc, err := req.GetBody(); err == nil { + rec.getBodyData, _ = io.ReadAll(rc) + _ = rc.Close() + } + } + if req.Body != nil { + rec.body, _ = io.ReadAll(req.Body) + // net/http's Transport closes Request.Body after reading it; model + // that here so the tests exercise realistic Close behavior. + _ = req.Body.Close() + } + *records = append(*records, rec) + return &http.Response{ + StatusCode: statusFor(len(*records)), + Body: io.NopCloser(strings.NewReader("")), + Header: make(http.Header), + }, nil + }) +} + +// newBodyReplayClient builds a generated client with the given transport and a +// fast idempotent retry config allowing maxAttempts. +func newBodyReplayClient(t *testing.T, doer generated.HttpRequestDoer, maxAttempts int) *generated.Client { + t.Helper() + client, err := generated.NewClient( + "https://example.com", + generated.WithHTTPClient(doer), + generated.WithRetryConfig(fastRetryConfig(maxAttempts)), + ) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + return client +} + +// plainReader is an io.Reader whose concrete type is not one http.NewRequest +// recognizes, so no GetBody snapshot is synthesized for it (the no-GetBody, +// single-use stream path). It is intentionally not an io.ReadCloser. +type plainReader struct{ r *bytes.Reader } + +func newPlainReader(b []byte) *plainReader { return &plainReader{r: bytes.NewReader(b)} } +func (p *plainReader) Read(b []byte) (int, error) { return p.r.Read(b) } + +// closeTrackingReader is an io.ReadCloser that records whether Close was called, +// used to prove a discarded retry-editor body is closed rather than leaked. +type closeTrackingReader struct { + r io.Reader + closed bool +} + +func (c *closeTrackingReader) Read(b []byte) (int, error) { return c.r.Read(b) } +func (c *closeTrackingReader) Close() error { c.closed = true; return nil } + +// --- Red / contract proofs (must fail pre-fix, pass post-fix) --- + +// (1) A 503→200 idempotent *WithBody with a single-use reader must ship the FULL +// body on the retry, and the finalized retry request must expose a working +// GetBody (307/308-redirect safety). +func TestWithBodyReplay_RetryShipsFullBody(t *testing.T) { + const payload = `{"content":"the full replayable message body"}` + var records []attemptRecord + doer := recordingDoer(&records, func(attempt int) int { + if attempt == 1 { + return http.StatusServiceUnavailable + } + return http.StatusOK + }) + client := newBodyReplayClient(t, doer, 3) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(payload)) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 2 { + t.Fatalf("made %d attempts, want 2", len(records)) + } + if got := string(records[1].body); got != payload { + t.Errorf("retry shipped body %q, want the full body %q", got, payload) + } + if records[1].getBodyNil { + t.Error("retry request has a nil GetBody; a 307/308 redirect could not replay the body") + } + if got := string(records[1].getBodyData); got != payload { + t.Errorf("retry request GetBody produced %q, want %q", got, payload) + } +} + +// (2) A partial-read network failure (transport reads a prefix, then errors) +// must restart at byte 0 on the next attempt, not resume mid-body. +func TestWithBodyReplay_PartialReadRestartsAtByteZero(t *testing.T) { + const payload = `{"content":"partial-read restart proof body"}` + var attempt2Body []byte + var attempts int + doer := doerFunc(func(req *http.Request) (*http.Response, error) { + attempts++ + if attempts == 1 { + // Read only a prefix, then fail like a mid-stream network error. + _, _ = io.ReadFull(req.Body, make([]byte, 5)) + _ = req.Body.Close() + return nil, io.ErrUnexpectedEOF + } + attempt2Body, _ = io.ReadAll(req.Body) + _ = req.Body.Close() + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil + }) + client := newBodyReplayClient(t, doer, 3) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(payload)) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if got := string(attempt2Body); got != payload { + t.Errorf("after a partial-read failure the retry shipped %q, want the full body %q", got, payload) + } +} + +// (3) A body with no GetBody snapshot is an unrewindable stream: it is NOT +// buffered or retried (which would risk unbounded memory or an uninterruptible +// read). The op makes a single attempt shipping the full body once, even under a +// retryable 503, rather than retrying with a drained body. +func TestWithBodyReplay_NoGetBodyStreamSingleAttempt(t *testing.T) { + const payload = `{"content":"an unrewindable stream is sent once, not retried"}` + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusServiceUnavailable }) + client := newBodyReplayClient(t, doer, 3) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/octet-stream", newPlainReader([]byte(payload))) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("a no-GetBody stream made %d attempts, want exactly 1 (unrewindable, not replayed)", len(records)) + } + if got := string(records[0].body); got != payload { + t.Errorf("the single attempt shipped %q, want the full body %q", got, payload) + } +} + +// (4) The retry attempt's ContentLength must reflect the replayed body, not the +// drained reader's zero length. +func TestWithBodyReplay_RetryContentLength(t *testing.T) { + const payload = `{"content":"content-length restore proof"}` + var records []attemptRecord + doer := recordingDoer(&records, func(attempt int) int { + if attempt == 1 { + return http.StatusServiceUnavailable + } + return http.StatusOK + }) + client := newBodyReplayClient(t, doer, 3) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(payload)) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 2 { + t.Fatalf("made %d attempts, want 2", len(records)) + } + if want := int64(len(payload)); records[1].contentLen != want { + t.Errorf("retry ContentLength = %d, want %d", records[1].contentLen, want) + } +} + +// (5) Attempt-1 fidelity: an editor that REPLACES req.Body (as a compress/ +// encrypt editor does) without updating GetBody has its body sent verbatim on +// attempt 1 — never overwritten by the builder's stale GetBody. Because the +// finalized bytes can no longer be reproduced, the request is sent once (not +// retried with the wrong body). +func TestWithBodyReplay_EditorReplacingBodyIsSentThenNotRetried(t *testing.T) { + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusServiceUnavailable }) + client := newBodyReplayClient(t, doer, 3) + + editor := func(_ context.Context, req *http.Request) error { + req.Body = io.NopCloser(strings.NewReader("EDITOR-TRANSFORMED-BODY")) + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("caller-original"), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("made %d attempts, want 1 (a replaced body can't be reproduced → single attempt)", len(records)) + } + if got := string(records[0].body); got != "EDITOR-TRANSFORMED-BODY" { + t.Errorf("attempt 1 shipped %q, want the editor-finalized body %q (attempt-1 fidelity, not the builder GetBody)", got, "EDITOR-TRANSFORMED-BODY") + } +} + +// (6) Even when a body-replacing editor ALSO installs a matching GetBody, the +// request is conservatively sent once: the SDK cannot verify a replaced body's +// GetBody actually reproduces it, so it does not risk a wrong-body retry. The +// editor's body is still sent verbatim on attempt 1. +func TestWithBodyReplay_EditorReplacingBodyAndGetBodyIsSentThenNotRetried(t *testing.T) { + const editorBody = "EDITOR-BODY-WITH-GETBODY" + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusServiceUnavailable }) + client := newBodyReplayClient(t, doer, 3) + + editor := func(_ context.Context, req *http.Request) error { + req.Body = io.NopCloser(strings.NewReader(editorBody)) + req.GetBody = func() (io.ReadCloser, error) { return io.NopCloser(strings.NewReader(editorBody)), nil } + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("caller"), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("made %d attempts, want 1 (a replaced body is not retried, even with a new GetBody)", len(records)) + } + if got := string(records[0].body); got != editorBody { + t.Errorf("attempt 1 shipped %q, want the editor body %q", got, editorBody) + } +} + +// (7) In-place mutation is undetectable: an editor that Resets the caller's +// reader (rather than replacing req.Body) still has its finalized body sent on +// attempt 1. Since the builder's body reference is unchanged, the request is +// (best-effort, net/http-redirect-consistent) retried via GetBody — the point +// under test is that attempt 1 ships the editor's finalized bytes, not the +// builder's stale snapshot. +func TestWithBodyReplay_EditorInPlaceMutationSentOnAttempt1(t *testing.T) { + var records []attemptRecord + doer := recordingDoer(&records, func(attempt int) int { + if attempt == 1 { + return http.StatusServiceUnavailable + } + return http.StatusOK + }) + client := newBodyReplayClient(t, doer, 3) + + caller := strings.NewReader("CALLER-ORIGINAL-BODY") + editor := func(_ context.Context, _ *http.Request) error { + caller.Reset("EDITOR-FINALIZED-BODY") + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", caller, editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if got := string(records[0].body); got != "EDITOR-FINALIZED-BODY" { + t.Errorf("attempt 1 shipped %q, want the editor's in-place-mutated body %q (attempt-1 fidelity)", got, "EDITOR-FINALIZED-BODY") + } +} + +// (8) A large caller body with a native GetBody (a *bytes.Reader) replays fully +// on retry, regardless of size — the GetBody path never buffers, so it is not +// demoted to a single attempt. +func TestWithBodyReplay_LargeGetBodyBodyRetriesFully(t *testing.T) { + payload := bytes.Repeat([]byte("z"), (1<<20)+512) // large; size is irrelevant to GetBody replay + var records []attemptRecord + doer := recordingDoer(&records, func(attempt int) int { + if attempt == 1 { + return http.StatusServiceUnavailable + } + return http.StatusOK + }) + client := newBodyReplayClient(t, doer, 3) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/octet-stream", bytes.NewReader(payload)) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 2 { + t.Fatalf("made %d attempts, want 2 (a large body with a valid GetBody must not be demoted)", len(records)) + } + if !bytes.Equal(records[1].body, payload) { + t.Errorf("retry shipped %d body bytes, want the full %d", len(records[1].body), len(payload)) + } +} + +// (9) A well-behaved Digest/HMAC editor reads the body to sign via req.GetBody +// (not draining req.Body), and the digest it writes must match the body actually +// sent on both attempts. Attempt 1 sends req.Body and the retry replays GetBody; +// for an unmodified body these are byte-identical, so the digest matches. +func TestWithBodyReplay_SigningEditorViaGetBodyMatchesSentBody(t *testing.T) { + const payload = `{"content":"digest must match the body actually sent, on every attempt"}` + var attempts int32 + var mismatches []string + doer := doerFunc(func(req *http.Request) (*http.Response, error) { + n := atomic.AddInt32(&attempts, 1) + body, _ := io.ReadAll(req.Body) + _ = req.Body.Close() + sum := sha256.Sum256(body) + if want, got := hex.EncodeToString(sum[:]), req.Header.Get("X-Body-Digest"); want != got { + mismatches = append(mismatches, fmt.Sprintf("attempt %d: sent %d body bytes with sha256 %s, but the editor's digest header was %s", n, len(body), want, got)) + } + status := http.StatusOK + if n == 1 { + status = http.StatusServiceUnavailable + } + return &http.Response{StatusCode: status, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil + }) + client := newBodyReplayClient(t, doer, 3) + + editor := func(_ context.Context, req *http.Request) error { + if req.GetBody == nil { + return fmt.Errorf("no GetBody to sign") + } + rc, err := req.GetBody() + if err != nil { + return err + } + b, _ := io.ReadAll(rc) + _ = rc.Close() + sum := sha256.Sum256(b) + req.Header.Set("X-Body-Digest", hex.EncodeToString(sum[:])) + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(payload), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if atomic.LoadInt32(&attempts) != 2 { + t.Fatalf("made %d attempts, want 2", attempts) + } + if len(mismatches) > 0 { + t.Errorf("editor digest did not match the sent body:\n %s", strings.Join(mismatches, "\n ")) + } +} + +// (10) A GetBody snapshot failure must not fail an otherwise-sendable request: +// GetBody is only needed for replay, so a snapshot error disables retries and +// the request is still sent once with its valid body. +func TestWithBodyReplay_GetBodyFailureFallsBackToSingleAttempt(t *testing.T) { + const payload = "VALID-BODY" + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusOK }) + client := newBodyReplayClient(t, doer, 3) + + editor := func(_ context.Context, req *http.Request) error { + // A valid req.Body remains, but GetBody fails to snapshot. + req.GetBody = func() (io.ReadCloser, error) { return nil, fmt.Errorf("snapshot failed") } + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(payload), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody returned %v — a GetBody snapshot failure must not fail an otherwise-sendable request", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("made %d attempts, want 1 (GetBody failure disables retries)", len(records)) + } + if got := string(records[0].body); got != payload { + t.Errorf("shipped %q, want the valid body %q", got, payload) + } +} + +// (11) If an editor errors on a retry after installReplay has opened a fresh +// body (a custom GetBody may hold a file/pipe), that body must be closed — not +// leaked. +func TestWithBodyReplay_ClosesInstalledBodyOnEditorError(t *testing.T) { + doer := doerFunc(func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusServiceUnavailable, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil + }) + client := newBodyReplayClient(t, doer, 3) + + // Attempt 1 installs a custom GetBody returning close-tracking bodies; the + // retry editor fails after installReplay has opened one of them. + var created []*closeTrackingReader + var editorCalls int + editor := func(_ context.Context, req *http.Request) error { + editorCalls++ + if editorCalls == 1 { + req.GetBody = func() (io.ReadCloser, error) { + ctr := &closeTrackingReader{r: strings.NewReader("REPLAY")} + created = append(created, ctr) + return ctr, nil + } + return nil + } + return fmt.Errorf("editor failure on retry") + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("caller"), editor) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { + t.Fatal("expected the retry editor error to propagate") + } + if len(created) == 0 { + t.Fatal("custom GetBody was never invoked") + } + if last := created[len(created)-1]; !last.closed { + t.Error("the replay body installed before the failing retry editor was not closed (leak)") + } +} + +// (12) A 503→200 with an EMPTY body (http.NoBody): a body-aware editor reads/ +// signs the body on every attempt, so an explicitly empty body must be replayed +// as a non-nil http.NoBody rather than collapsed to nil (which would hand the +// retry editor a nil Body). +func TestWithBodyReplay_EmptyBodyReplayedNotNil(t *testing.T) { + var records []attemptRecord + doer := recordingDoer(&records, func(attempt int) int { + if attempt == 1 { + return http.StatusServiceUnavailable + } + return http.StatusOK + }) + client := newBodyReplayClient(t, doer, 3) + + var sawNil bool + var editorCalls int + editor := func(_ context.Context, req *http.Request) error { + editorCalls++ + if req.Body == nil { + sawNil = true + return fmt.Errorf("retry editor observed nil Body") + } + _, _ = io.ReadAll(req.Body) // a body-aware editor reads/signs the (empty) body + return nil + } + + // strings.NewReader("") → http.NewRequest sets req.Body = http.NoBody. + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(""), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if sawNil { + t.Error("a body-aware editor observed a nil Body on retry; an empty body must be replayed as http.NoBody") + } + if len(records) != 2 { + t.Fatalf("made %d attempts, want 2 (editor_calls=%d)", len(records), editorCalls) + } +} + +// (13) When a retry editor REPLACES req.Body, the replay body installed before +// the editor ran must not be orphaned — it is owned and closed regardless. Both +// the pre-editor body and the editor's replacement must end up closed. +func TestWithBodyReplay_RetryEditorReplacesBodyClosesPreEditor(t *testing.T) { + var records []attemptRecord + doer := recordingDoer(&records, func(attempt int) int { + if attempt == 1 { + return http.StatusServiceUnavailable + } + return http.StatusOK + }) + client := newBodyReplayClient(t, doer, 3) + + var created []*closeTrackingReader + newTracked := func(s string) *closeTrackingReader { + ctr := &closeTrackingReader{r: strings.NewReader(s)} + created = append(created, ctr) + return ctr + } + var editorCalls int + editor := func(_ context.Context, req *http.Request) error { + editorCalls++ + if editorCalls == 1 { + // Make the replay source produce trackable bodies (so the pre-editor + // body A is observable, not an opaque NopCloser). + req.GetBody = func() (io.ReadCloser, error) { return newTracked("REPLAY"), nil } + return nil + } + // Retry: replace req.Body, orphaning the pre-editor replay body. + req.Body = newTracked("EDITOR-REPLACEMENT") + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("caller"), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 2 { + t.Fatalf("made %d attempts, want 2", len(records)) + } + for i, ctr := range created { + if !ctr.closed { + t.Errorf("replay body #%d was leaked (not closed) — a retry editor replacing req.Body orphaned it", i) + } + } +} + +// (14) Same as (13) but the retry editor also ERRORS after replacing req.Body: +// both the pre-editor body and the replacement must be closed on the error path. +func TestWithBodyReplay_RetryEditorReplacesBodyThenErrorsClosesBoth(t *testing.T) { + // recordingDoer closes req.Body like a real transport, so attempt 1's body is + // released the normal way; the retry editor then errors before Client.Do. + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusServiceUnavailable }) + client := newBodyReplayClient(t, doer, 3) + + var created []*closeTrackingReader + newTracked := func(s string) *closeTrackingReader { + ctr := &closeTrackingReader{r: strings.NewReader(s)} + created = append(created, ctr) + return ctr + } + var editorCalls int + editor := func(_ context.Context, req *http.Request) error { + editorCalls++ + if editorCalls == 1 { + req.GetBody = func() (io.ReadCloser, error) { return newTracked("REPLAY"), nil } + return nil + } + req.Body = newTracked("EDITOR-REPLACEMENT") + return fmt.Errorf("editor failure after replacing the body") + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("caller"), editor) + if resp != nil { + _ = resp.Body.Close() + } + if err == nil { + t.Fatal("expected the retry editor error to propagate") + } + if len(created) == 0 { + t.Fatal("custom GetBody was never invoked") + } + for i, ctr := range created { + if !ctr.closed { + t.Errorf("replay body #%d was leaked (not closed) on the replace-then-error path", i) + } + } +} + +// (15) Empty-body transfer framing must be identical across attempts. net/http +// keys request framing off body identity: a wrapped http.NoBody has +// ContentLength 0 but is not the sentinel, so Request.outgoingLength() reports +// -1 and the retry would switch to chunked Transfer-Encoding. This runs through +// a REAL net/http Transport (httptest.Server) — a fake doer cannot observe +// transfer framing. +func TestWithBodyReplay_EmptyBodyIdenticalFramingAcrossRetries(t *testing.T) { + type framing struct { + contentLength string + transferEncoding string + reqContentLength int64 + } + var framings []framing + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + framings = append(framings, framing{ + contentLength: r.Header.Get("Content-Length"), + transferEncoding: strings.Join(r.TransferEncoding, ","), + reqContentLength: r.ContentLength, + }) + if len(framings) == 1 { + w.WriteHeader(http.StatusServiceUnavailable) // retryable + return + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // A real *http.Client (no fake doer), so the real Transport computes framing. + client, err := generated.NewClient(server.URL, generated.WithRetryConfig(fastRetryConfig(3))) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("")) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(framings) != 2 { + t.Fatalf("made %d requests, want 2", len(framings)) + } + if framings[0] != framings[1] { + t.Errorf("empty-body transfer framing differs across attempts:\n attempt 1: %+v\n attempt 2: %+v", framings[0], framings[1]) + } + if framings[1].transferEncoding == "chunked" { + t.Error("retry used chunked Transfer-Encoding for an empty body while attempt 1 did not (http.NoBody sentinel not preserved)") + } +} + +// --- Controls (pass both pre- and post-fix) --- + +// Happy path: a body with a native GetBody snapshot on a first-try success is +// sent once, unchanged. +func TestWithBodyReplay_HappyPathSingleAttempt(t *testing.T) { + const payload = `{"content":"happy path body"}` + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusOK }) + client := newBodyReplayClient(t, doer, 3) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader(payload)) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("made %d attempts, want 1", len(records)) + } + if got := string(records[0].body); got != payload { + t.Errorf("shipped body %q, want %q", got, payload) + } +} + +// Retry-ineligible (single attempt): a no-GetBody body is streamed untouched — +// not replayed — so no GetBody snapshot is synthesized. +func TestWithBodyReplay_RetryIneligibleBuffersNothing(t *testing.T) { + const payload = `{"content":"streamed untouched body"}` + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusOK }) + // maxAttempts == 1 → retry-ineligible. + client := newBodyReplayClient(t, doer, 1) + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", newPlainReader([]byte(payload))) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("made %d attempts, want 1", len(records)) + } + if !records[0].getBodyNil { + t.Error("a retry-ineligible no-GetBody body had a GetBody synthesized; it should stream untouched") + } + if got := string(records[0].body); got != payload { + t.Errorf("shipped body %q, want %q", got, payload) + } +} + +// Retry-ineligible with a native GetBody caller: a non-retrying call must send +// whatever body the editors finalized (here, an editor that replaced req.Body), +// NOT the caller's GetBody snapshot. The retry-ineligible check runs before the +// GetBody normalization, so no rewrite happens on a single-attempt request. +func TestWithBodyReplay_RetryIneligibleHonorsEditorBody(t *testing.T) { + var records []attemptRecord + doer := recordingDoer(&records, func(int) int { return http.StatusOK }) + // maxAttempts == 1 → retry-ineligible. + client := newBodyReplayClient(t, doer, 1) + + editor := func(_ context.Context, req *http.Request) error { + req.Body = io.NopCloser(strings.NewReader("EDITOR-BODY")) + return nil + } + + resp, err := client.UpdateMessageWithBody(context.Background(), "1", 100, "application/json", strings.NewReader("CALLER-BODY"), editor) + if err != nil { + t.Fatalf("UpdateMessageWithBody: %v", err) + } + _ = resp.Body.Close() + + if len(records) != 1 { + t.Fatalf("made %d attempts, want 1", len(records)) + } + if got := string(records[0].body); got != "EDITOR-BODY" { + t.Errorf("retry-ineligible call shipped %q, want the editor body %q (no GetBody normalization when not retrying)", got, "EDITOR-BODY") + } +} diff --git a/go/pkg/basecamp/helpers.go b/go/pkg/basecamp/helpers.go index 6de8d503e..db0996033 100644 --- a/go/pkg/basecamp/helpers.go +++ b/go/pkg/basecamp/helpers.go @@ -1,6 +1,7 @@ package basecamp import ( + "bytes" "encoding/json" "fmt" "io" @@ -9,7 +10,11 @@ import ( ) // marshalBody encodes a map as JSON and returns an io.Reader suitable for the -// generated client's *WithBodyWithResponse methods. +// generated client's *WithBodyWithResponse methods. It returns a *bytes.Reader +// so net/http snapshots it into req.GetBody, which the generated client's +// doWithRetry uses to replay the body across retry attempts (these SDK-owned +// serialized bodies must stay retryable — see the naturally-idempotent PUT/ +// DELETE conformance case). // // This is an intentional architectural exception to the normal pattern of using // generated typed request bodies. It exists because the generated structs for @@ -38,31 +43,7 @@ func marshalBody(m map[string]any) (io.Reader, error) { if err != nil { return nil, fmt.Errorf("failed to marshal request body: %w", err) } - return &rewindableReader{data: b}, nil -} - -// rewindableReader wraps a byte slice as an io.Reader that auto-rewinds -// after returning EOF, so the generated client's doWithRetry can replay -// the body on each retry attempt. Safe against partial reads: once EOF -// is returned, the next Read starts from position 0. -type rewindableReader struct { - data []byte - pos int - atEOF bool -} - -func (r *rewindableReader) Read(p []byte) (int, error) { - if r.atEOF { - r.pos = 0 - r.atEOF = false - } - if r.pos >= len(r.data) { - r.atEOF = true - return 0, io.EOF - } - n := copy(p, r.data[r.pos:]) - r.pos += n - return n, nil + return bytes.NewReader(b), nil } // checkResponse converts HTTP response errors to SDK errors for non-2xx responses. diff --git a/go/pkg/basecamp/helpers_test.go b/go/pkg/basecamp/helpers_test.go index 0ff0e9ee5..56503ac11 100644 --- a/go/pkg/basecamp/helpers_test.go +++ b/go/pkg/basecamp/helpers_test.go @@ -1,6 +1,7 @@ package basecamp import ( + "context" "io" "net/http" "testing" @@ -169,14 +170,30 @@ func TestMarshalBody_ReturnsReplayableReader(t *testing.T) { t.Fatalf("marshalBody returned error: %v", err) } + // The body must be snapshotable by net/http so the generated client's + // doWithRetry can replay it via GetBody across retries — otherwise these + // SDK-owned serialized bodies would be sent once and never retried. + req, err := http.NewRequestWithContext(context.Background(), http.MethodPut, "https://example.com", reader) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + if req.GetBody == nil { + t.Fatal("marshalBody body is not snapshotable: req.GetBody is nil (retries would be disabled)") + } + const want = `{"content":"Updated content"}` for attempt := 1; attempt <= 2; attempt++ { - got, err := io.ReadAll(reader) + rc, err := req.GetBody() + if err != nil { + t.Fatalf("attempt %d: GetBody: %v", attempt, err) + } + got, err := io.ReadAll(rc) + _ = rc.Close() if err != nil { t.Fatalf("attempt %d: failed reading body: %v", attempt, err) } if string(got) != want { - t.Fatalf("attempt %d: body = %q, want %q", attempt, got, want) + t.Fatalf("attempt %d: replayed body = %q, want %q", attempt, got, want) } } } diff --git a/go/pkg/basecamp/projects_test.go b/go/pkg/basecamp/projects_test.go index 2872b5896..c07a4b872 100644 --- a/go/pkg/basecamp/projects_test.go +++ b/go/pkg/basecamp/projects_test.go @@ -3,11 +3,13 @@ package basecamp import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "os" "path/filepath" "testing" + "time" ) // fixturesDir returns the path to the fixtures directory. @@ -322,3 +324,45 @@ func TestProjectsService_UpdateEmptyScheduleAttributes(t *testing.T) { t.Errorf("expected schedule_attributes to be omitted for empty struct, but it was present: %v", receivedBody["schedule_attributes"]) } } + +// TestProjectsService_UpdateRetriesOn503WithFullBody is a PUBLIC-service retry +// proof: the typed ProjectsService.Update serializes its body via marshalBody +// (a *bytes.Reader, which net/http snapshots into GetBody), so the generated +// client's doWithRetry replays the FULL body on a transient 503. Guards against +// SDK-owned serialized bodies losing retries (the naturally-idempotent PUT +// conformance case). +func TestProjectsService_UpdateRetriesOn503WithFullBody(t *testing.T) { + var bodies []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + bodies = append(bodies, string(b)) + if len(bodies) == 1 { + w.WriteHeader(http.StatusServiceUnavailable) // retryable + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(200) + _, _ = w.Write([]byte(`{"id":12345,"name":"x"}`)) + })) + t.Cleanup(server.Close) + + cfg := DefaultConfig() + cfg.BaseURL = server.URL + token := &StaticTokenProvider{Token: "test-token"} + client := NewClient(cfg, token, WithMaxRetries(3), WithBaseDelay(time.Millisecond)) + svc := client.ForAccount("99999").Projects() + + if _, err := svc.Update(context.Background(), 12345, &UpdateProjectRequest{Name: "x"}); err != nil { + t.Fatalf("Update: %v", err) + } + + if len(bodies) != 2 { + t.Fatalf("public UpdateProject made %d requests, want 2 (idempotent PUT must retry on 503)", len(bodies)) + } + const want = `{"name":"x"}` + for i, b := range bodies { + if b != want { + t.Errorf("request %d body = %q, want the full serialized body %q (the retry must replay it)", i+1, b, want) + } + } +} diff --git a/go/pkg/generated/client.gen.go b/go/pkg/generated/client.gen.go index a9e62cdee..290065d48 100644 --- a/go/pkg/generated/client.gen.go +++ b/go/pkg/generated/client.gen.go @@ -3974,6 +3974,102 @@ func isRetryableStatus(statusCode int) bool { } } +// captureReplayBody inspects the finalized first-attempt request and returns a +// closure that reproduces its body on later attempts, plus the ContentLength to +// restore each attempt. When retriable is false the body cannot be safely +// replayed, so the caller makes a single attempt streaming the body as-is. +// +// Replay contract (documented; net/http-consistent): req.GetBody, when present, +// DEFINES the request body for replay — exactly as the standard library's own +// redirect handling uses it. There is no reliable way to detect whether an +// editor mutated req.Body out of sync with req.GetBody (an editor can Reset the +// underlying reader in place, which no identity check can see), so we make +// GetBody authoritative: this attempt is ALSO normalized to send GetBody()'s +// bytes, so every attempt — first and retries — ships identical bytes. A +// retrying request's body is therefore changed by an editor only through +// req.GetBody (set req.Body and req.GetBody together, as net/http requires). +// +// A body WITHOUT a GetBody snapshot is an arbitrary, single-use stream. It is +// NOT buffered or retried: buffering would risk unbounded memory, and a blocking +// reader cannot be interrupted (there is no general way to abort an io.Reader), +// so such a request is sent exactly once. In-memory bodies that net/http can +// snapshot (*bytes.Reader/*bytes.Buffer/*strings.Reader — the readers the typed +// and generated builders use) already carry a GetBody and so replay normally; +// only a caller passing a genuinely streaming reader to a raw *WithBody method +// forgoes retries, which is the safe behavior for an unrewindable stream. +// sameReader reports whether two request bodies are the same value. Comparing +// io.ReadCloser interface values panics if their dynamic type is not comparable +// (e.g. a struct with a slice field used directly as the body); such a body was +// necessarily supplied by an editor, so recovering to false — "not the same", +// which disables replay — is the safe answer. +func sameReader(a, b io.ReadCloser) (same bool) { + defer func() { _ = recover() }() + return a == b +} + +// captureReplayBody decides, once, how a retry reproduces the FIRST finalized +// request body. It never rewrites req.Body: attempt 1 always sends the body +// exactly as buildRequest + the request editors left it, so an editor that +// compresses/encrypts/transforms the payload (and sets matching headers) is +// honored — even when no retry occurs. builderBody is req.Body as buildRequest +// produced it, BEFORE the editors ran. +// +// A retry replays via req.GetBody, exactly as net/http replays a body on a +// 307/308 redirect. That is only sound when the builder's body survived the +// editors unchanged (so its GetBody still describes req.Body). If an editor +// REPLACED req.Body, the builder's GetBody is stale and the finalized bytes +// cannot be reproduced, so the request is sent once rather than retried with the +// wrong body. (An editor that mutates the body IN PLACE without updating GetBody +// is undetectable and, like a net/http redirect, would replay GetBody's +// snapshot; editors should replace the body, or keep req.GetBody in sync.) +func captureReplayBody(req *http.Request, builderBody io.ReadCloser, retryEligible bool) (replay func() (io.ReadCloser, error), contentLength int64, retriable bool, err error) { + // A nil body (GET/HEAD): a retry reproduces the nil body faithfully. + if req.Body == nil { + return nil, req.ContentLength, true, nil + } + // An explicitly empty body (http.NoBody): always reproducible — replay the + // stateless sentinel so retries send a non-nil empty body with identical + // (empty) transfer framing. + if req.Body == http.NoBody { + return func() (io.ReadCloser, error) { return http.NoBody, nil }, req.ContentLength, true, nil + } + if !retryEligible { + return nil, req.ContentLength, false, nil + } + // Replayable only if the builder's body survived the editors unchanged and + // carries a usable GetBody. Probe GetBody (without disturbing the finalized + // req.Body sent on attempt 1); on failure or an editor replacement, send once. + if req.GetBody != nil && sameReader(req.Body, builderBody) { + rc, gErr := req.GetBody() + if gErr != nil { + return nil, req.ContentLength, false, nil + } + _ = rc.Close() + return req.GetBody, req.ContentLength, true, nil + } + return nil, req.ContentLength, false, nil +} + +// closeOnceBody wraps a request body so Close is idempotent. doWithRetry installs +// a replay body before the request editors run (so a body-aware editor observes +// it), then owns that handle: it closes it after the editor phase regardless of +// whether an editor replaced req.Body — closing it here plus the routine close of +// the current req.Body would otherwise double-close the same handle when the +// editor left it in place, or leak it when the editor swapped it out. +type closeOnceBody struct { + rc io.ReadCloser + closed bool +} + +func (b *closeOnceBody) Read(p []byte) (int, error) { return b.rc.Read(p) } +func (b *closeOnceBody) Close() error { + if b.closed { + return nil + } + b.closed = true + return b.rc.Close() +} + // doWithRetry executes a request with retry logic for idempotent operations. func (c *Client) doWithRetry(ctx context.Context, buildRequest func() (*http.Request, error), isIdempotent bool, operationId string, reqEditors ...RequestEditorFn) (*http.Response, error) { // MaxRetries is a TOTAL attempt count. Re-validate here (not just in @@ -3998,14 +4094,113 @@ func (c *Client) doWithRetry(ctx context.Context, buildRequest func() (*http.Req var lastErr error delay := c.RetryConfig.BaseDelay + // Replay state captured from the first finalized request (see + // captureReplayBody). buildRequest recreates the request from the caller's + // now-drained io.Reader on every attempt, so the raw *WithBody methods would + // otherwise ship an empty (or mid-stream) body on retries. + var bodyReplay func() (io.ReadCloser, error) + var replayContentLength int64 + retryEligible := isIdempotent && maxAttempts > 1 + + // installReplay closes the current req.Body and swaps in a fresh replay of the + // captured first-attempt body (or nil when there is no body), restoring + // GetBody and ContentLength. It returns the installed handle (wrapped so Close + // is idempotent) so a retry's pre-editor caller can own and close it after the + // editor phase. On a retry it runs BOTH before applyEditors — so body-aware + // editors (e.g. one computing a Digest/HMAC header) observe and sign the bytes + // that will actually be sent — AND after, so the first finalized body stays + // authoritative (an editor cannot change a retry's body). + installReplay := func(req *http.Request) (io.ReadCloser, error) { + if req.Body != nil { + _ = req.Body.Close() + } + if bodyReplay == nil { + req.Body = nil + req.GetBody = nil + req.ContentLength = replayContentLength + return nil, nil + } + rc, bErr := bodyReplay() + if bErr != nil { + return nil, bErr + } + // Preserve the http.NoBody sentinel unwrapped: net/http keys request + // framing off body identity — a wrapped NoBody has ContentLength 0 but is + // not the sentinel, so Request.outgoingLength() reports -1 (unknown) and + // the retry would switch to Transfer-Encoding: chunked, differing from the + // first attempt's empty framing. http.NoBody's Close is already a no-op, + // so it needs no close-once wrapper. + var installed io.ReadCloser = rc + if rc != http.NoBody { + installed = &closeOnceBody{rc: rc} + } + req.Body = installed + req.GetBody = bodyReplay + req.ContentLength = replayContentLength + return installed, nil + } + for attempt := 1; attempt <= maxAttempts; attempt++ { req, err := buildRequest() if err != nil { return nil, err } req = req.WithContext(ctx) - if err := c.applyEditors(ctx, req, reqEditors); err != nil { - return nil, err + + // Capture the builder's body BEFORE the editors run so captureReplayBody + // can tell (on attempt 1) whether an editor replaced it — a replaced body + // is not safely reproducible for retries. + var builderBody io.ReadCloser + if attempt == 1 { + builderBody = req.Body + } + + // On a retry, install the replay body BEFORE editors so body-aware editors + // sign the bytes that will actually be sent. We OWN this handle and close + // it after the editor phase — even if an editor replaces req.Body (which + // would otherwise orphan it). closeOnceBody makes the routine close below + // a no-op when the editor left it in place. + var preEditorBody io.ReadCloser + if attempt > 1 { + preEditorBody, err = installReplay(req) + if err != nil { + return nil, err + } + } + + editErr := c.applyEditors(ctx, req, reqEditors) + if preEditorBody != nil { + _ = preEditorBody.Close() + } + if editErr != nil { + // Release whatever body the editors left (a replacement, or the + // already-closed pre-editor handle — closeOnceBody makes this safe). + if req.Body != nil { + _ = req.Body.Close() + } + return nil, editErr + } + + if attempt == 1 { + // Snapshot the finalized first request's body exactly once; every + // later attempt replays THIS body. + replay, cl, retriable, capErr := captureReplayBody(req, builderBody, retryEligible) + if capErr != nil { + return nil, capErr + } + bodyReplay = replay + replayContentLength = cl + if !retriable { + // The body is not safely replayable (an unrewindable stream with + // no GetBody): send it once and do not retry. + maxAttempts = 1 + } + } else { + // Re-install AFTER editors: the first finalized body wins, and any + // editor body change (now closed) is discarded and refreshed. + if _, err := installReplay(req); err != nil { + return nil, err + } } resp, err := c.Client.Do(req) diff --git a/go/templates/client.tmpl b/go/templates/client.tmpl index a72f19c77..215490aa9 100644 --- a/go/templates/client.tmpl +++ b/go/templates/client.tmpl @@ -158,6 +158,102 @@ func isRetryableStatus(statusCode int) bool { } } +// captureReplayBody inspects the finalized first-attempt request and returns a +// closure that reproduces its body on later attempts, plus the ContentLength to +// restore each attempt. When retriable is false the body cannot be safely +// replayed, so the caller makes a single attempt streaming the body as-is. +// +// Replay contract (documented; net/http-consistent): req.GetBody, when present, +// DEFINES the request body for replay — exactly as the standard library's own +// redirect handling uses it. There is no reliable way to detect whether an +// editor mutated req.Body out of sync with req.GetBody (an editor can Reset the +// underlying reader in place, which no identity check can see), so we make +// GetBody authoritative: this attempt is ALSO normalized to send GetBody()'s +// bytes, so every attempt — first and retries — ships identical bytes. A +// retrying request's body is therefore changed by an editor only through +// req.GetBody (set req.Body and req.GetBody together, as net/http requires). +// +// A body WITHOUT a GetBody snapshot is an arbitrary, single-use stream. It is +// NOT buffered or retried: buffering would risk unbounded memory, and a blocking +// reader cannot be interrupted (there is no general way to abort an io.Reader), +// so such a request is sent exactly once. In-memory bodies that net/http can +// snapshot (*bytes.Reader/*bytes.Buffer/*strings.Reader — the readers the typed +// and generated builders use) already carry a GetBody and so replay normally; +// only a caller passing a genuinely streaming reader to a raw *WithBody method +// forgoes retries, which is the safe behavior for an unrewindable stream. +// sameReader reports whether two request bodies are the same value. Comparing +// io.ReadCloser interface values panics if their dynamic type is not comparable +// (e.g. a struct with a slice field used directly as the body); such a body was +// necessarily supplied by an editor, so recovering to false — "not the same", +// which disables replay — is the safe answer. +func sameReader(a, b io.ReadCloser) (same bool) { + defer func() { _ = recover() }() + return a == b +} + +// captureReplayBody decides, once, how a retry reproduces the FIRST finalized +// request body. It never rewrites req.Body: attempt 1 always sends the body +// exactly as buildRequest + the request editors left it, so an editor that +// compresses/encrypts/transforms the payload (and sets matching headers) is +// honored — even when no retry occurs. builderBody is req.Body as buildRequest +// produced it, BEFORE the editors ran. +// +// A retry replays via req.GetBody, exactly as net/http replays a body on a +// 307/308 redirect. That is only sound when the builder's body survived the +// editors unchanged (so its GetBody still describes req.Body). If an editor +// REPLACED req.Body, the builder's GetBody is stale and the finalized bytes +// cannot be reproduced, so the request is sent once rather than retried with the +// wrong body. (An editor that mutates the body IN PLACE without updating GetBody +// is undetectable and, like a net/http redirect, would replay GetBody's +// snapshot; editors should replace the body, or keep req.GetBody in sync.) +func captureReplayBody(req *http.Request, builderBody io.ReadCloser, retryEligible bool) (replay func() (io.ReadCloser, error), contentLength int64, retriable bool, err error) { + // A nil body (GET/HEAD): a retry reproduces the nil body faithfully. + if req.Body == nil { + return nil, req.ContentLength, true, nil + } + // An explicitly empty body (http.NoBody): always reproducible — replay the + // stateless sentinel so retries send a non-nil empty body with identical + // (empty) transfer framing. + if req.Body == http.NoBody { + return func() (io.ReadCloser, error) { return http.NoBody, nil }, req.ContentLength, true, nil + } + if !retryEligible { + return nil, req.ContentLength, false, nil + } + // Replayable only if the builder's body survived the editors unchanged and + // carries a usable GetBody. Probe GetBody (without disturbing the finalized + // req.Body sent on attempt 1); on failure or an editor replacement, send once. + if req.GetBody != nil && sameReader(req.Body, builderBody) { + rc, gErr := req.GetBody() + if gErr != nil { + return nil, req.ContentLength, false, nil + } + _ = rc.Close() + return req.GetBody, req.ContentLength, true, nil + } + return nil, req.ContentLength, false, nil +} + +// closeOnceBody wraps a request body so Close is idempotent. doWithRetry installs +// a replay body before the request editors run (so a body-aware editor observes +// it), then owns that handle: it closes it after the editor phase regardless of +// whether an editor replaced req.Body — closing it here plus the routine close of +// the current req.Body would otherwise double-close the same handle when the +// editor left it in place, or leak it when the editor swapped it out. +type closeOnceBody struct { + rc io.ReadCloser + closed bool +} + +func (b *closeOnceBody) Read(p []byte) (int, error) { return b.rc.Read(p) } +func (b *closeOnceBody) Close() error { + if b.closed { + return nil + } + b.closed = true + return b.rc.Close() +} + // doWithRetry executes a request with retry logic for idempotent operations. func (c *{{ $clientTypeName }}) doWithRetry(ctx context.Context, buildRequest func() (*http.Request, error), isIdempotent bool, operationId string, reqEditors ...RequestEditorFn) (*http.Response, error) { // MaxRetries is a TOTAL attempt count. Re-validate here (not just in @@ -182,14 +278,113 @@ func (c *{{ $clientTypeName }}) doWithRetry(ctx context.Context, buildRequest fu var lastErr error delay := c.RetryConfig.BaseDelay + // Replay state captured from the first finalized request (see + // captureReplayBody). buildRequest recreates the request from the caller's + // now-drained io.Reader on every attempt, so the raw *WithBody methods would + // otherwise ship an empty (or mid-stream) body on retries. + var bodyReplay func() (io.ReadCloser, error) + var replayContentLength int64 + retryEligible := isIdempotent && maxAttempts > 1 + + // installReplay closes the current req.Body and swaps in a fresh replay of the + // captured first-attempt body (or nil when there is no body), restoring + // GetBody and ContentLength. It returns the installed handle (wrapped so Close + // is idempotent) so a retry's pre-editor caller can own and close it after the + // editor phase. On a retry it runs BOTH before applyEditors — so body-aware + // editors (e.g. one computing a Digest/HMAC header) observe and sign the bytes + // that will actually be sent — AND after, so the first finalized body stays + // authoritative (an editor cannot change a retry's body). + installReplay := func(req *http.Request) (io.ReadCloser, error) { + if req.Body != nil { + _ = req.Body.Close() + } + if bodyReplay == nil { + req.Body = nil + req.GetBody = nil + req.ContentLength = replayContentLength + return nil, nil + } + rc, bErr := bodyReplay() + if bErr != nil { + return nil, bErr + } + // Preserve the http.NoBody sentinel unwrapped: net/http keys request + // framing off body identity — a wrapped NoBody has ContentLength 0 but is + // not the sentinel, so Request.outgoingLength() reports -1 (unknown) and + // the retry would switch to Transfer-Encoding: chunked, differing from the + // first attempt's empty framing. http.NoBody's Close is already a no-op, + // so it needs no close-once wrapper. + var installed io.ReadCloser = rc + if rc != http.NoBody { + installed = &closeOnceBody{rc: rc} + } + req.Body = installed + req.GetBody = bodyReplay + req.ContentLength = replayContentLength + return installed, nil + } + for attempt := 1; attempt <= maxAttempts; attempt++ { req, err := buildRequest() if err != nil { return nil, err } req = req.WithContext(ctx) - if err := c.applyEditors(ctx, req, reqEditors); err != nil { - return nil, err + + // Capture the builder's body BEFORE the editors run so captureReplayBody + // can tell (on attempt 1) whether an editor replaced it — a replaced body + // is not safely reproducible for retries. + var builderBody io.ReadCloser + if attempt == 1 { + builderBody = req.Body + } + + // On a retry, install the replay body BEFORE editors so body-aware editors + // sign the bytes that will actually be sent. We OWN this handle and close + // it after the editor phase — even if an editor replaces req.Body (which + // would otherwise orphan it). closeOnceBody makes the routine close below + // a no-op when the editor left it in place. + var preEditorBody io.ReadCloser + if attempt > 1 { + preEditorBody, err = installReplay(req) + if err != nil { + return nil, err + } + } + + editErr := c.applyEditors(ctx, req, reqEditors) + if preEditorBody != nil { + _ = preEditorBody.Close() + } + if editErr != nil { + // Release whatever body the editors left (a replacement, or the + // already-closed pre-editor handle — closeOnceBody makes this safe). + if req.Body != nil { + _ = req.Body.Close() + } + return nil, editErr + } + + if attempt == 1 { + // Snapshot the finalized first request's body exactly once; every + // later attempt replays THIS body. + replay, cl, retriable, capErr := captureReplayBody(req, builderBody, retryEligible) + if capErr != nil { + return nil, capErr + } + bodyReplay = replay + replayContentLength = cl + if !retriable { + // The body is not safely replayable (an unrewindable stream with + // no GetBody): send it once and do not retry. + maxAttempts = 1 + } + } else { + // Re-install AFTER editors: the first finalized body wins, and any + // editor body change (now closed) is discarded and refreshed. + if _, err := installReplay(req); err != nil { + return nil, err + } } resp, err := c.Client.Do(req)