fix(azure): error instead of truncating silently at the pricing page cap

providers/azure/internal/pricing/retail_prices.go

@@ -42,10 +42,15 @@ type Page[T any] struct {
 // service clients use this default.
 const DefaultPageTimeout = 10 * time.Second
 
-// DefaultMaxPages caps the NextPageLink loop. The Azure Retail Prices API
-// paginates at 100 items per page, so 50 pages is 5000 items — more than
-// any realistic SKU/region/term query. The cap is purely a defence against
-// a server bug returning a NextPageLink that never empties.
+// DefaultMaxPages caps the NextPageLink loop. The Retail Prices API
+// (api-version 2023-01-01-preview) pages at about 1,000 items, so 50
+// pages is roughly 50,000 items. The largest filter any client issues
+// (a region-wide service catalogue, cosmosdb/search) measured 1 page on
+// 2026-09-08; the whole Virtual Machines catalogue for one region, which
+// no client requests, measured 16. The cap is a defence against a
+// server bug returning a NextPageLink that never empties. Reaching it
+// with a NextPageLink still pending is an error, never a truncated
+// result (#1963).
 const DefaultMaxPages = 50
 
 // FetchAll walks the Retail Prices API starting at initialURL, appending
@@ -54,7 +59,8 @@ const DefaultMaxPages = 50
 //   - a per-page timeout (pageTimeout) that's independent of the caller's
 //     ctx, so one slow page can't consume the caller's whole budget;
 //   - a max-pages cap (maxPages) against a server bug returning an
-//     infinite NextPageLink chain;
+//     infinite NextPageLink chain; hitting it with pages remaining is
+//     an error, not a truncated result;
 //   - a seen-URL guard against a self-referential NextPageLink.
 //
 // ctx's cancellation still propagates via context.WithTimeout(ctx, ...),
@@ -84,6 +90,9 @@ func FetchAll[T any](ctx context.Context, httpClient HTTPClient, initialURL stri
 		nextURL = page.NextPageLink
 	}
 
+	if nextURL != "" {
+		return nil, fmt.Errorf("pricing API result exceeds the %d-page cap (%d items read, NextPageLink still set)", maxPages, len(all))
+	}
 	return all, nil
 }
 

providers/azure/internal/pricing/retail_prices_test.go

@@ -60,6 +60,22 @@ func okJSONResponse(body string) *http.Response {
 	}
 }
 
+// scriptPageChain scripts n pages at https://prices.example/page<letter>,
+// each holding one item named after its letter and linking to the next;
+// the last page has an empty NextPageLink.
+func scriptPageChain(client *fakeHTTPClient, n int) {
+	for i := 0; i < n; i++ {
+		url := "https://prices.example/page" + string(rune('a'+i))
+		next := ""
+		if i < n-1 {
+			next = "https://prices.example/page" + string(rune('a'+i+1))
+		}
+		client.responses[url] = okJSONResponse(
+			`{"Items":[{"name":"` + string(rune('a'+i)) + `"}],"NextPageLink":"` + next + `"}`,
+		)
+	}
+}
+
 // TestFetchAll_MergesPages pins the multi-page walk: page 1 has a non-
 // empty NextPageLink, page 2 has an empty NextPageLink, all items are
 // merged into the returned slice in order.
@@ -94,29 +110,35 @@ func TestFetchAll_RejectsSelfReferentialNextPageLink(t *testing.T) {
 	assert.Contains(t, err.Error(), "self-referential")
 }
 
-// TestFetchAll_HonoursMaxPagesCap covers the defensive cap: if the server
-// returns a genuinely unbounded chain of fresh NextPageLinks, the walker
-// must stop after maxPages instead of running forever.
-func TestFetchAll_HonoursMaxPagesCap(t *testing.T) {
+// TestFetchAll_ErrorsWhenCapReachedWithPagesRemaining is the regression
+// test for #1963: a chain longer than maxPages must produce an error, not
+// the first maxPages pages with a nil error. The walker must still stop
+// fetching at the cap.
+func TestFetchAll_ErrorsWhenCapReachedWithPagesRemaining(t *testing.T) {
 	client := newFakeHTTPClient()
-	for i := 0; i < 10; i++ {
-		url := "https://prices.example/page" + string(rune('a'+i))
-		next := ""
-		if i < 9 {
-			next = "https://prices.example/page" + string(rune('a'+i+1))
-		}
-		client.responses[url] = okJSONResponse(
-			`{"Items":[{"name":"` + string(rune('a'+i)) + `"}],"NextPageLink":"` + next + `"}`,
-		)
-	}
+	scriptPageChain(client, 10)
+
+	items, err := FetchAll[fakeItem](context.Background(), client, "https://prices.example/pagea", DefaultPageTimeout, 3)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "3-page cap")
+	assert.Contains(t, err.Error(), "3 items read")
+	assert.Nil(t, items, "a truncated result must not be returned alongside the error")
+	assert.Len(t, client.calls, 3, "walker must not fetch beyond maxPages")
+}
+
+// TestFetchAll_ExactlyMaxPagesSucceeds pins the boundary: a chain of
+// exactly maxPages pages whose last page has an empty NextPageLink is
+// complete and must not be reported as truncated.
+func TestFetchAll_ExactlyMaxPagesSucceeds(t *testing.T) {
+	client := newFakeHTTPClient()
+	scriptPageChain(client, 3)
 
-	// Cap at 3 — walker should read pages a, b, c only.
 	items, err := FetchAll[fakeItem](context.Background(), client, "https://prices.example/pagea", DefaultPageTimeout, 3)
 	require.NoError(t, err)
 	require.Len(t, items, 3)
 	assert.Equal(t, "a", items[0].Name)
 	assert.Equal(t, "c", items[2].Name)
-	assert.Len(t, client.calls, 3, "walker must not fetch beyond maxPages")
+	assert.Len(t, client.calls, 3)
 }
 
 // TestFetchAll_PerPageTimeout proves the per-page timeout is applied

providers/azure/services/cache/client.go

@@ -567,7 +567,7 @@ func (c *CacheClient) getRedisPricing(ctx context.Context, sku, region string, t
 }
 
 // fetchAzurePricing fetches pricing data from Azure Retail Prices API,
-// following NextPageLink until exhausted or the shared safety cap fires.
+// following NextPageLink until exhausted; hitting the shared page cap is an error.
 // Delegates pagination to pricing.FetchAll.
 func (c *CacheClient) fetchAzurePricing(ctx context.Context, serviceName, sku, region string) (*AzureRetailPrice, error) {
 	filter := fmt.Sprintf("serviceName eq '%s' and armRegionName eq '%s' and contains(armSkuName, '%s')",

providers/azure/services/compute/client.go

@@ -728,8 +728,8 @@ func (c *ComputeClient) getVMPricing(ctx context.Context, vmSize, region string,
 }
 
 // fetchAzurePricing fetches pricing data from Azure Retail Prices API,
-// following NextPageLink until exhausted (or the shared safety cap is
-// hit). Delegates the pagination walk to pricing.FetchAll so every
+// following NextPageLink until exhausted (hitting the shared page cap
+// is an error). Delegates the pagination walk to pricing.FetchAll so every
 // service client shares the same per-page timeout, seen-URL guard, and
 // max-pages cap — see providers/azure/internal/pricing for those
 // invariants.

providers/azure/services/cosmosdb/client.go

@@ -567,7 +567,7 @@ func (c *CosmosDBClient) getCosmosPricing(ctx context.Context, sku, region strin
 }
 
 // fetchAzurePricing fetches pricing data from Azure Retail Prices API,
-// following NextPageLink until exhausted or the shared safety cap fires.
+// following NextPageLink until exhausted; hitting the shared page cap is an error.
 // Delegates pagination to pricing.FetchAll.
 func (c *CosmosDBClient) fetchAzurePricing(ctx context.Context, filter string) (*AzureRetailPrice, error) {
 	baseURL := "https://prices.azure.com/api/retail/prices"

providers/azure/services/database/client.go

@@ -586,7 +586,7 @@ func (c *DatabaseClient) getSQLPricing(ctx context.Context, sku, region string,
 }
 
 // fetchAzurePricing fetches pricing data from Azure Retail Prices API,
-// following NextPageLink until exhausted or the shared safety cap fires.
+// following NextPageLink until exhausted; hitting the shared page cap is an error.
 // Delegates pagination to pricing.FetchAll — see
 // providers/azure/internal/pricing for the per-page timeout, max-pages
 // cap, and seen-URL guard invariants.