fix(azure): discarded io.ReadAll error decides retry-vs-abort on the reservation purchase path #1766

Found while triaging LeanerCloud/cloud-commitments-platform#174's production errcheck findings. Filed separately from that issue: this is a behavioural defect on the Azure reservation purchase path, not lint chore, and it should not wait behind ~1242 cosmetic findings.

The defect

providers/azure/services/internal/reservations/purchase.go:475, in doPurchase:

resp, err := httpClient.Do(req)
if err != nil {
    return fmt.Errorf("failed to purchase reservation: %w", err)
}
body, _ := io.ReadAll(resp.Body)          // <-- read error discarded
resp.Body.Close()

if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusCreated || resp.StatusCode == http.StatusAccepted {
    return nil
}
return fmt.Errorf("reservation purchase failed with status %d: %s", resp.StatusCode, string(body))

That error string is the only input to the retry decision:

func IsSessionTimeout(err error) bool {
    if err == nil {
        return false
    }
    return strings.Contains(err.Error(), sessionTimeoutFragment)
}

and DoPurchaseTwoStep branches on it:

purchaseErr := doPurchase(ctx, httpClient, PurchaseURL(orderID), bodyBytes, bearerToken)
if purchaseErr == nil {
    return orderID, nil
}
if IsSessionTimeout(purchaseErr) && attempt < purchaseMaxAttempts {
    // re-run calculatePrice and retry
    continue
}
return "", purchaseErr

So a discarded io.ReadAll error decides retry versus abort. If the body read fails or truncates before sessionTimeoutFragment on Azure's 400 "Session timed out" response, IsSessionTimeout returns false and the purchase is abandoned on the first attempt — the exact condition purchaseMaxAttempts exists to survive. The operator sees reservation purchase failed with status 400: with an empty body and no indication why.

Severity

Fails closed. It abandons a purchase; it does not double-buy. That is why this is severity/medium rather than higher.

But it is a real behavioural defect: a designed-retryable failure silently becomes permanent, and the diagnostic that would explain it is the very thing that went missing. Azure session timeouts on purchase are common enough that the two-step retry exists specifically for them (see the DoPurchaseTwoStep doc comment).

This is the only one of LeanerCloud/cloud-commitments-platform#174's 49 production findings where a discarded error feeds control flow rather than a message — see the triage.

Not affected

Checked rather than assumed — the two sibling io.ReadAll discards in the same file are safe:

• :287 doCalculatePrice — a truncated body fails json.Unmarshal, and an empty ReservationOrderID is separately guarded. Fails closed with a real error.
• :390 fetchReservationOrdersPage — backs the idempotency-token lookup. Truncation fails json.Unmarshal, and the caller DoIdempotentPurchaseTwoStep explicitly refuses to purchase on a lookup error rather than falling through, so there is no double-buy path.

Fix

Check the read error and surface it, or determine session-timeout status from something that does not depend on a successful body read. Either way the retry decision must not silently depend on a discarded error.

effort/xs.

Regression test

Must pin that the retry decision survives a failed body read: a mock whose Do returns a 400 with a body reader that errors partway, asserting that either the purchase is retried or the failure is surfaced as a read error — not silently classified as non-retryable. Confirm it fails against the current code before the fix.

Related

• ci: lint job is root-only — 1291 golangci-lint findings across five unlinted workspace modules cloud-commitments-platform#174 — the parent lint-scope issue; this was found in its triage and deliberately not folded into it.
• ci: unit and integration test jobs only run the root module — providers/*, pkg and tests/e2e are never tested #1751 / PR ci: run unit and integration tests in every workspace module #1755 — providers/azure has never been run by CI's test jobs, which is part of why this sat unnoticed.