Preserve request body when retrying AssociateAssets request (#40515)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> Resolves #40593 This PR attempts to fix this error: ``` {"component":"http","err":"associating asset with adamID <adamId> to host <hostId>: making request to Apple VPP endpoint: making request to Apple VPP endpoint: Post \"https://vpp.itunes.apple.com/mdm/v2/assets/associate\": http: ContentLength=111 with Body length 0","host_id":<hostId>,"ip_addr":"<ip_addr>","level":"error","method":"POST","took":"20.748056032s","ts":"2026-02-25T09:53:32.10267006Z","uri":"/api/latest/fleet/device/<deviceId>/software/install/<id>","x_for_ip_addr":"<ip_addr>"} ``` Per my troubleshooting: `client.Do(req)` consumes the request body. When retrying, the same `req` is reused but its body is not there -- so, the retry sends `ContentLength=108` with an empty body, producing the `Body length 0` error. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [ ] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually Ran the test I added without the code fix, and was able to see the exact same error <img width="1188" height="567" alt="Screenshot 2026-02-25 at 3 26 12 PM" src="https://github.com/user-attachments/assets/d7bdfee7-de33-43d0-92c6-e77fa46329d6" /> After: <img width="852" height="140" alt="Screenshot 2026-02-25 at 3 26 55 PM" src="https://github.com/user-attachments/assets/e7ec3ea5-2b29-463a-9038-e5530d654a4d" />
This commit is contained in:
@@ -291,7 +291,19 @@ func GetAssignments(token string, filter *AssignmentFilter) ([]Assignment, error
|
||||
}
|
||||
|
||||
func do[T any](req *http.Request, token string, dest *T) error {
|
||||
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token))
|
||||
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
|
||||
|
||||
// Reset the request body for retries. After client.Do reads the body,
|
||||
// it's consumed. GetBody (set by http.NewRequest for *bytes.Buffer)
|
||||
// returns a fresh reader over the original bytes.
|
||||
if req.GetBody != nil {
|
||||
body, err := req.GetBody()
|
||||
if err != nil {
|
||||
return fmt.Errorf("resetting request body for VPP retry: %w", err)
|
||||
}
|
||||
req.Body = body
|
||||
}
|
||||
|
||||
resp, err := client.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("making request to Apple VPP endpoint: %w", err)
|
||||
|
||||
@@ -366,6 +366,93 @@ func TestDoRetryAfter(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoRetry(t *testing.T) {
|
||||
t.Run("retries after 500 with Retry-After", func(t *testing.T) {
|
||||
var calls int
|
||||
setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
calls++
|
||||
|
||||
// Verify Authorization header appears exactly once
|
||||
authHeaders := r.Header.Values("Authorization")
|
||||
require.Len(t, authHeaders, 1,
|
||||
"expected exactly 1 Authorization header on attempt %d, got %d: %v",
|
||||
calls, len(authHeaders), authHeaders)
|
||||
require.Equal(t, "Bearer test-token", authHeaders[0])
|
||||
|
||||
// Verify POST body is intact
|
||||
body, err := io.ReadAll(r.Body)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, body, "request body should not be empty on attempt %d", calls)
|
||||
|
||||
var reqParams AssociateAssetsRequest
|
||||
err = json.Unmarshal(body, &reqParams)
|
||||
require.NoError(t, err, "request body should be valid JSON on attempt %d, got: %q", calls, string(body))
|
||||
require.Equal(t, "462054704", reqParams.Assets[0].AdamID)
|
||||
require.Equal(t, "GXH409KH7X", reqParams.SerialNumbers[0])
|
||||
|
||||
if calls == 1 {
|
||||
// First call: return 500 with Retry-After to trigger retry
|
||||
w.Header().Set("Retry-After", "1")
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
_, _ = w.Write([]byte("{}"))
|
||||
return
|
||||
}
|
||||
|
||||
// Second call: success
|
||||
_, _ = w.Write([]byte(`{"eventId": "evt-123"}`))
|
||||
})
|
||||
|
||||
eventID, err := AssociateAssets("test-token", &AssociateAssetsRequest{
|
||||
Assets: []Asset{{AdamID: "462054704", PricingParam: "STDQ"}},
|
||||
SerialNumbers: []string{"GXH409KH7X"},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "evt-123", eventID)
|
||||
require.Equal(t, 2, calls)
|
||||
})
|
||||
|
||||
t.Run("retries after error 9646", func(t *testing.T) {
|
||||
var calls int
|
||||
setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
calls++
|
||||
|
||||
// Verify Authorization header appears exactly once
|
||||
authHeaders := r.Header.Values("Authorization")
|
||||
require.Len(t, authHeaders, 1,
|
||||
"expected exactly 1 Authorization header on attempt %d, got %d: %v",
|
||||
calls, len(authHeaders), authHeaders)
|
||||
|
||||
// Verify POST body is intact
|
||||
body, err := io.ReadAll(r.Body)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, body, "request body should not be empty on attempt %d", calls)
|
||||
|
||||
var reqParams AssociateAssetsRequest
|
||||
err = json.Unmarshal(body, &reqParams)
|
||||
require.NoError(t, err, "request body should be valid JSON on attempt %d, got: %q", calls, string(body))
|
||||
require.Equal(t, "462054704", reqParams.Assets[0].AdamID)
|
||||
|
||||
if calls == 1 {
|
||||
// First call: return rate-limit error 9646
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(`{"errorMessage":"Too many requests","errorNumber":9646}`))
|
||||
return
|
||||
}
|
||||
|
||||
// Second call: success
|
||||
_, _ = w.Write([]byte(`{"eventId": "evt-456"}`))
|
||||
})
|
||||
|
||||
eventID, err := AssociateAssets("test-token", &AssociateAssetsRequest{
|
||||
Assets: []Asset{{AdamID: "462054704", PricingParam: "STDQ"}},
|
||||
SerialNumbers: []string{"GXH409KH7X"},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "evt-456", eventID)
|
||||
require.GreaterOrEqual(t, calls, 2)
|
||||
})
|
||||
}
|
||||
|
||||
func TestGetBaseURL(t *testing.T) {
|
||||
t.Run("Default URL", func(t *testing.T) {
|
||||
require.Equal(t, "https://vpp.itunes.apple.com/mdm/v2", getBaseURL())
|
||||
|
||||
Reference in New Issue
Block a user