Repository navigation
fix: migrate flag listing to REST API 20240415 with pagination - #251
Conversation
| if err != nil { | ||
| return []ldapi.FeatureFlag{}, err | ||
| } | ||
| // A short page does not guarantee that there are no more flags. |
There was a problem hiding this comment.
if this comment is true, why are are still returning flags?
There was a problem hiding this comment.
This is based on your migration guide:
To page through the results, repeat each request, increasing the
offsetby thelimiteach time, until one of the following is true:
the items array in the response is empty
And basically this is just my best guess from interpreting that. However, this depends on how your API actually behaves, and you all would know better than I would.
Also, re-reading that page again, it looks like we should be increasing offset by limit?
Ultimately this PR was intended to get the ball rolling. Let me know your thoughts, and please feel free to update this branch and/or request changes.
There was a problem hiding this comment.
Tested the change to offset by limit in CI with some extra logging, the flags fetched and values matched between the old version and this PR's changes. Also update the wording on this comment.
Following my PR #251, I noticed a higher rate of 429s from LaunchDarkly's API in our CI. Makes sense, pagination means more requests. So this PR adds retries. It's based on [your own documentation](https://launchdarkly.com/docs/api#rate-limiting) for rate limiting. I've run it in our CI for a few weeks, resulting in thousands of runs, and it handles 429s properly and the overall failure rate remains comparable to before the migration to pagination. Please feel free to do what you want with this PR, for example if lots of changes are needed and you want to do it yourself, take whatever you want from this PR. I only want to raise the issue and ensure it gets addressed somehow. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Replaces the paginated LaunchDarkly flag list client with a **context-aware `flagFetcher`** built on `go-retryablehttp`. **`GetAllFlags` now accepts `context.Context`** (wired from `main`), applies a **120s inventory timeout**, and only **retries HTTP 429** responses—using **`Retry-After`**, reset headers, and remaining-quota headers per LD’s rate-limiting docs, with exponential backoff when hints are missing. > > Between pages it can **pause proactively** when responses show exhausted quota (including before switching active → archived). Failures return structured **`flagFetchError`** reasons (canceled, rate-limit deadline/exhausted, decode, etc.), **avoid partial flag lists** on mid-inventory errors, and **surface cancellation/timeouts while reading response bodies**. **`LD_FLAG_FETCH` JSON diagnostics** log start/summary, rate limits, and intended waits for CI observability. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 357fdfd. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Context
The action's flag-list request still uses REST API version
20220603. LaunchDarkly's migration guide requires upgrading to20240415before the December 31, 2026 retirement deadline.This updates the request to
20240415and adds pagination for active and archived flags. The new API paginates flag listings, so changing the header alone would leave flag detection limited to the first page.Testing
HTTP fixture tests cover multiple pages of active and archived flags, short and empty pages, request headers and query parameters, preserved flag metadata, and errors on first and later pages without returning a partial flag inventory.
Passed locally with Go
1.25.14:go test ./...go test -race ./internal/ldclientgo vet -mod=vendor ./..., and the existinggolangci-lintpre-commit hookSecret-dependent end-to-end tests were not run locally and need maintainer involvement.
Note
Overview
Migrates LaunchDarkly flag listing from REST API
20220603to20240415and implements offset/limit pagination (100 per page) until an empty page, for both active flags and optional archived (state:archived) collections.getFlagsnow copies query params, drives the page loop, and delegates each request to newgetFlagPage;CHANGELOGdocuments the fix. Addsflags_test.gowith HTTP fixtures for multi-page active/archived flows, metadata preservation, query param handling, and error paths on first or later pages.Reviewed by Cursor Bugbot for commit 9749c1c. Bugbot is set up for automated code reviews on this repo. Configure here.