From 6a2d11f473fa855a3524003e35d774a5a07234a7 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:37:37 +0000 Subject: [PATCH] fix(api): Keep per_page stable when listPage walks offset cursors listPage asked each later page for only the rows still missing. Sentry's OffsetPaginator reads a cursor's window with the page size the cursor encodes, so pairing 100:1:0 with per_page=50 returned rows 150-199 and dropped 100-149. listProjects, listTeams and listOrganizations lost rows whenever limit was above 100. listPage now walks through paginate, which keeps one page size per walk, reuses the size an offset cursor encodes, and returns a cursor that resumes mid-page. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- static/packages/api/lib/api-client/core.ts | 55 ++++++++-------- .../packages/api/test/api-client/core.test.ts | 66 +++++++++++++++++++ .../endpoints/organizations.test.ts | 2 +- 3 files changed, 94 insertions(+), 29 deletions(-) diff --git a/static/packages/api/lib/api-client/core.ts b/static/packages/api/lib/api-client/core.ts index c03821c4083b..79cbeae44f90 100644 --- a/static/packages/api/lib/api-client/core.ts +++ b/static/packages/api/lib/api-client/core.ts @@ -21,7 +21,7 @@ import { ConfigurationError, createApiError, } from "./errors.js"; -import { API_MAX_PER_PAGE } from "./pagination.js"; +import { API_MAX_PER_PAGE, paginate } from "./pagination.js"; import { type ErrorResponseBody, readErrorResponse, @@ -293,16 +293,17 @@ export function createApiClientCore({ /** * Fetch up to `limit` rows from a list endpoint, starting at `cursor`. * - * Each page asks for exactly the rows still missing, up to - * {@link API_MAX_PER_PAGE}, and is validated with `schema`. The walk stops - * once it has `limit` rows or the server has no next page, and returns the - * server's cursor for the page after the last one fetched. + * Walks pages with {@link paginate}, so every request in a walk uses the same + * `per_page`: a fresh walk splits `limit` evenly, and a page-number cursor + * (`OffsetPaginator`) keeps the page size it encodes. Each page is validated + * with `schema`. The walk stops once it has `limit` rows, the server has no + * next page, or it has fetched {@link MAX_LIST_PAGES} pages, and returns a + * cursor that resumes at the first row not returned. * * @throws {ApiValidationError} When `limit` is not between 1 and * `API_MAX_PER_PAGE * MAX_LIST_PAGES` * @throws {ApiResponseValidationError} When a page does not match `schema` - * @throws {Error} When a page has more rows than requested, a cursor - * repeats, or the walk passes {@link MAX_LIST_PAGES} + * @throws {Error} When a page has more rows than requested or a cursor repeats */ export async function listPage( fetchPage: ( @@ -322,26 +323,24 @@ export async function listPage( `limit must be an integer between 1 and ${API_MAX_PER_PAGE * MAX_LIST_PAGES}`, ); } - const items: T[] = []; const seenCursors = new Set(); - let cursor = initialCursor ?? undefined; - for (let page = 0; page < MAX_LIST_PAGES; page += 1) { - const perPage = Math.min(API_MAX_PER_PAGE, limit - items.length); - const { data, response } = await fetchPage(perPage, cursor); - const rows = validateResponse(schema, data); - if (rows.length > perPage) { - throw new Error("API returned more items than requested"); - } - items.push(...rows); - const nextCursor = nextPageCursor(response); - if (items.length >= limit || !nextCursor) { - return { items, nextCursor }; - } - if (seenCursors.has(nextCursor) || nextCursor === cursor) { - throw new Error("API returned a repeated pagination cursor"); - } - seenCursors.add(nextCursor); - cursor = nextCursor; - } - throw new Error("API pagination exceeded its page limit"); + const result = await paginate( + { limit, cursor: initialCursor ?? undefined, maxPages: MAX_LIST_PAGES }, + async (perPage, cursor) => { + const { data, response } = await fetchPage(perPage, cursor); + const rows = validateResponse(schema, data); + if (rows.length > perPage) { + throw new Error("API returned more items than requested"); + } + const nextCursor = nextPageCursor(response); + if (nextCursor) { + if (seenCursors.has(nextCursor) || nextCursor === cursor) { + throw new Error("API returned a repeated pagination cursor"); + } + seenCursors.add(nextCursor); + } + return nextCursor ? { data: rows, nextCursor } : { data: rows }; + }, + ); + return { items: result.data, nextCursor: result.nextCursor ?? null }; } diff --git a/static/packages/api/test/api-client/core.test.ts b/static/packages/api/test/api-client/core.test.ts index 6441ecc98ce4..748f177d9864 100644 --- a/static/packages/api/test/api-client/core.test.ts +++ b/static/packages/api/test/api-client/core.test.ts @@ -289,6 +289,72 @@ describe("listPage", () => { ).rejects.toThrow("API returned a repeated pagination cursor"); }); + /** + * Sentry's `OffsetPaginator` (`src/sentry/api/paginator.py`) over `total` + * rows. Its cursor is `::`; a request whose + * `per_page` differs from the cursor's page size reads the cursor's window + * and keeps only its last `per_page` rows. + */ + function offsetPaginator(total: number) { + const requests: Array<{ perPage: number; cursor: string | undefined }> = + []; + async function fetchPage(perPage: number, cursor: string | undefined) { + requests.push({ perPage, cursor }); + const [value = 0, pageIndex = 0] = (cursor ?? "0:0:0") + .split(":") + .map(Number); + const start = pageIndex * value; + const stop = Math.min(total, start + (value || perPage) + 1); + let window = ids(start, stop); + if (value !== perPage) { + window = window.slice(-(perPage + 1)); + } + return page( + window.slice(0, perPage), + window.length > perPage ? `${perPage}:${pageIndex + 1}:0` : undefined, + ); + } + return { fetchPage, requests }; + } + + function ids(start: number, stop: number) { + return Array.from({ length: stop - start }, (_, index) => start + index); + } + + function idsOf(result: { items: Array<{ id: number }> }) { + return result.items.map(({ id }) => id); + } + + it("keeps per_page stable across an OffsetPaginator walk", async () => { + const { fetchPage, requests } = offsetPaginator(300); + + const first = await listPage(fetchPage, rows, 150); + const second = await listPage(fetchPage, rows, 150, first.nextCursor); + + expect(idsOf(first)).toEqual(ids(0, 150)); + expect(idsOf(second)).toEqual(ids(150, 300)); + expect(second.nextCursor).toBeNull(); + expect(new Set(requests.map(({ perPage }) => perPage))).toEqual( + new Set([75]), + ); + }); + + it("resumes an OffsetPaginator cursor with the page size it encodes", async () => { + const { fetchPage, requests } = offsetPaginator(300); + // Pairing `100:1:0` with per_page=50 reads rows 150-199 and skips 100-149. + expect((await fetchPage(50, "100:1:0")).data).toEqual( + ids(150, 200).map((id) => ({ id })), + ); + requests.length = 0; + + const first = await listPage(fetchPage, rows, 50, "100:1:0"); + const second = await listPage(fetchPage, rows, 50, first.nextCursor); + + expect(idsOf(first)).toEqual(ids(100, 150)); + expect(idsOf(second)).toEqual(ids(150, 200)); + expect(requests.map(({ perPage }) => perPage)).toEqual([100, 100]); + }); + it("validates each page with the schema", async () => { await expect( listPage( diff --git a/static/packages/api/test/api-client/endpoints/organizations.test.ts b/static/packages/api/test/api-client/endpoints/organizations.test.ts index 49307514d7d4..1546b8d9d4e7 100644 --- a/static/packages/api/test/api-client/endpoints/organizations.test.ts +++ b/static/packages/api/test/api-client/endpoints/organizations.test.ts @@ -108,7 +108,7 @@ describe("listOrganizations", () => { ]); expect(urls[0]?.searchParams.get("per_page")).toBe("3"); expect(urls[0]?.searchParams.get("query")).toBe("ac"); - expect(urls[1]?.searchParams.get("per_page")).toBe("1"); + expect(urls[1]?.searchParams.get("per_page")).toBe("3"); expect(urls[1]?.searchParams.get("cursor")).toBe("0:2:0"); });