From bd8498188f97b72ce2db5fc17ff3e752cc545ed5 Mon Sep 17 00:00:00 2001 From: Andrea Cappelletti Date: Tue, 29 Sep 2026 13:48:17 -0400 Subject: [PATCH] fix(pricing): page through the whole catalog in pricing sync The enumerator stopped at the first page shorter than it requested and stepped the offset by the requested size. Once the pricing API capped requests at 1,000 products, every page was short, so a sync kept only the first 1,000 products per service and region and offline estimates fell back to the stub. Advance by the rows returned; stop on an empty page. --- CHANGELOG.md | 9 +++++ internal/pricing/enumerate.go | 22 ++++++---- internal/pricing/enumerate_paging_test.go | 49 +++++++++++++++++++++++ 3 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 internal/pricing/enumerate_paging_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ea76117..72042c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,15 @@ uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). region-fallback caveat. Catalog expressions can now read the region a resource is priced in as `region` when the resource does not set that attribute itself. +- `c3x pricing sync` fetched only the first 1,000 products of each + service and region, so `c3x estimate --offline` priced most resources + from the stub ($0 for an m5.xlarge). The enumerator treated a page + shorter than it asked for as the last page and stepped the offset by + the requested size; since the pricing API began capping requests at + 1,000 products, every page came back short. It now advances by the rows + returned and stops only on an empty page. A synced us-east-1 AWS cache + went from 4,455 to 22,364 prices, and offline estimates match live + ones. ## [0.3.15] - 2026-09-29 diff --git a/internal/pricing/enumerate.go b/internal/pricing/enumerate.go index 9a09939..e2fc580 100644 --- a/internal/pricing/enumerate.go +++ b/internal/pricing/enumerate.go @@ -10,9 +10,11 @@ import ( "strings" ) -// enumPageSize is the per-request product window. It matches the -// server's product cap so each page is one round-trip, and — crucially -// — every request stays bounded. The v1 failure mode (one ~8 GB +// enumPageSize is the per-request product window the enumerator asks +// for. A server may cap it lower (pricing.c3x.dev's cap is +// MAX_PRODUCTS_PER_REQUEST), so [productEnumerator.each] advances by the +// rows actually returned and stops only on an empty page. Every request +// stays bounded. The v1 failure mode (one ~8 GB // streamed JSON that truncated into "unexpected end of JSON input") // cannot recur here: a stalled or partial page fails that page only // and retries cheaply. @@ -60,7 +62,12 @@ func (e *productEnumerator) each( provider, service, region string, fn func(enumProduct) error, ) error { - for offset := 0; ; offset += enumPageSize { + // Advance by what came back and stop on an empty page: a server + // that caps the page below enumPageSize returns short pages that are + // not the last one. Treating a short page as the end, and stepping by + // the requested size, truncated every sync to the first page once the + // API capped requests at 1,000 products. + for offset := 0; ; { if err := ctx.Err(); err != nil { return err } @@ -68,14 +75,15 @@ func (e *productEnumerator) each( if err != nil { return err } + if len(page) == 0 { + return nil + } for _, p := range page { if err := fn(p); err != nil { return err } } - if len(page) < enumPageSize { - return nil - } + offset += len(page) } } diff --git a/internal/pricing/enumerate_paging_test.go b/internal/pricing/enumerate_paging_test.go new file mode 100644 index 0000000..31d9936 --- /dev/null +++ b/internal/pricing/enumerate_paging_test.go @@ -0,0 +1,49 @@ +package pricing + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "regexp" + "strconv" + "strings" + "testing" +) + +// A server that caps each page below enumPageSize, as pricing.c3x.dev +// does with MAX_PRODUCTS_PER_REQUEST, must still be enumerated in full. +// Before the fix the enumerator treated the first short page as the last +// one, so `c3x pricing sync` kept only the first 1,000 products of each +// service and region. +func TestEnumeratorPagesThroughAServerCap(t *testing.T) { + t.Parallel() + const total, capPerPage = 7, 3 + offsetRe := regexp.MustCompile(`offset:(\d+)`) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + m := offsetRe.FindStringSubmatch(string(raw)) + offset, _ := strconv.Atoi(m[1]) + var items []string + for i := offset; i < total && i < offset+capPerPage; i++ { + items = append(items, fmt.Sprintf( + `{"productFamily":"Compute Instance","attributes":[{"key":"instanceType","value":"t%d"}],"prices":[{"USD":"0.1","unit":"Hrs","purchaseOption":"on_demand"}]}`, i)) + } + _, _ = io.WriteString(w, `{"data":{"products":[`+strings.Join(items, ",")+`]}}`) + })) + t.Cleanup(srv.Close) + + enum := newProductEnumerator(srv.Client(), srv.URL, "") + seen := map[string]bool{} + err := enum.each(context.Background(), "aws", "AmazonEC2", "us-east-1", func(p enumProduct) error { + seen[p.Attributes["instanceType"]] = true + return nil + }) + if err != nil { + t.Fatal(err) + } + if len(seen) != total { + t.Fatalf("enumerated %d distinct products, want all %d: %v", len(seen), total, seen) + } +}