diff --git a/.oxlintrc.json b/.oxlintrc.json index 233e8d2e0..f9096c72a 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -71,6 +71,7 @@ "sentry-cli/no-direct-target-resolution": "error", "sentry-cli/no-generic-is-record": "error", "sentry-cli/prefer-paginate-helper": "error", + "sentry-cli/no-manual-pagination": "error", "sentry-cli/no-namespace-import": "error", "no-restricted-imports": [ "error", diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 2e8d0c163..0d6925176 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -411,24 +411,32 @@ that fits your use case): **Auto-pagination for large limits:** When `--limit` exceeds `API_MAX_PER_PAGE` (100), list commands MUST transparently -fetch multiple pages to fill the requested limit. Cap `perPage` at -`Math.min(flags.limit, API_MAX_PER_PAGE)` and loop until `results.length >= limit` -or pages are exhausted. This matches the `listIssuesAllPages` pattern. +fetch multiple pages to fill the requested limit. Always do this with +`paginate()` from `src/lib/api/infrastructure.ts`; never write the cursor loop +yourself. The `sentry-cli/no-manual-pagination` lint rule rejects loops that +follow cursors and count toward a limit. ```typescript -const perPage = Math.min(flags.limit, API_MAX_PER_PAGE); -for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { - const { data, nextCursor } = await listPaginated(org, { perPage, cursor }); - results.push(...data); - if (results.length >= flags.limit || !nextCursor) break; - cursor = nextCursor; -} +const { data, nextCursor } = await paginate( + { limit: flags.limit, cursor }, + (perPage, pageCursor) => + listPaginated(org, { perPage, cursor: pageCursor }), +); ``` -Never pass a `per_page` value larger than `API_MAX_PER_PAGE` to the API — the -server silently caps it, causing the command to return fewer items than requested. - -Reference template: `trace/list.ts`, `span/list.ts`, `dashboard/list.ts` +Pass `perPage` to the request unchanged. Sentry's page-number paginators +(`OffsetPaginator`, used by projects, teams, members, repositories, and +dashboard revisions) encode the page size in the cursor, so a cursor is only +valid with the `per_page` that produced it. `paginate()` therefore returns +opaque CLI cursors (`cli1~~~`) that record the +walk's page size and how many rows of the current page were already returned. +A later `-c next` reuses that page size even when `--limit` changes, and +resumes mid-page by refetching the page and skipping returned rows. Always +pass these cursors back through `paginate()`; never send them to an API +request directly. Test pagination changes against the paginator models in +`test/lib/paginate.property.test.ts`. + +Reference template: `trace/list.ts`, `span/list.ts`, `dashboard/revisions.ts` ### ID Validation diff --git a/packages/cli/lint-rules/cli-oxlint-plugin.js b/packages/cli/lint-rules/cli-oxlint-plugin.js index d1035dd58..e9f68d55f 100644 --- a/packages/cli/lint-rules/cli-oxlint-plugin.js +++ b/packages/cli/lint-rules/cli-oxlint-plugin.js @@ -226,18 +226,42 @@ const rules = { "prefer-paginate-helper": rule( "Use paginate() from infrastructure.js instead of a raw limit.", (_context, file, report) => - file.includes("/src/lib/api/") && !file.endsWith("/infrastructure.ts") + file.includes("/src/") && !file.endsWith("/lib/api/infrastructure.ts") ? { CallExpression(node) { + const limit = node.arguments[1]; if ( node.callee.name === "autoPaginate" && - node.arguments[1]?.name === "limit" + (limit?.name === "limit" || member(limit, "limit")) ) report(node); }, } : {}, ), + "no-manual-pagination": rule( + "Use paginate() from infrastructure.js instead of a limit-bounded cursor loop.", + (_context, file, report) => { + if ( + !file.includes("/src/") || + file.endsWith("/lib/api/infrastructure.ts") + ) + return {}; + // A loop that follows cursors and also counts toward a limit is + // re-deriving per_page and cursor-after-trim rules that paginate() owns. + const check = (node) => { + const followsCursors = + containsIdentifier(node, "MAX_PAGINATION_PAGES") || + containsIdentifier(node, "nextCursor"); + if (followsCursors && containsIdentifier(node, "limit")) report(node); + }; + return { + ForStatement: check, + WhileStatement: check, + DoWhileStatement: check, + }; + }, + ), "no-namespace-import": rule( "Import only the names used by production code.", (_context, file, report) => diff --git a/packages/cli/src/commands/alert/issues/list.ts b/packages/cli/src/commands/alert/issues/list.ts index a7778109e..f154afa74 100644 --- a/packages/cli/src/commands/alert/issues/list.ts +++ b/packages/cli/src/commands/alert/issues/list.ts @@ -13,11 +13,8 @@ import type { SentryContext } from "../../../context.js"; import { buildProjectAliasMap } from "../../../lib/alias.js"; import type { IssueAlertRule } from "../../../lib/api/alerts.js"; -import { MAX_PAGINATION_PAGES } from "../../../lib/api/infrastructure.js"; -import { - API_MAX_PER_PAGE, - listIssueAlertsPaginated, -} from "../../../lib/api-client.js"; +import { paginate } from "../../../lib/api/infrastructure.js"; +import { listIssueAlertsPaginated } from "../../../lib/api-client.js"; import { parseOrgProjectArg } from "../../../lib/arg-parsing.js"; import { openInBrowser } from "../../../lib/browser.js"; import { @@ -137,43 +134,15 @@ async function fetchRulesForTarget( options: { limit: number; startCursor?: string }, ): Promise { const result = await withAuthGuard(async () => { - const rules: IssueAlertRule[] = []; - let serverCursor = options.startCursor; - - for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { - const { data, nextCursor } = await listIssueAlertsPaginated( - target.org, - target.project, - { - perPage: Math.min(options.limit - rules.length, API_MAX_PER_PAGE), - cursor: serverCursor, - }, - ); - - for (const rule of data) { - rules.push(rule); - if (rules.length >= options.limit) { - return { - target, - rules, - hasMore: !!nextCursor, - nextCursor: nextCursor ?? undefined, - }; - } - } - - if (!nextCursor) { - return { target, rules, hasMore: false }; - } - serverCursor = nextCursor; - } - - return { - target, - rules, - hasMore: !!serverCursor, - nextCursor: serverCursor ?? undefined, - }; + const { data: rules, nextCursor } = await paginate( + { limit: options.limit, cursor: options.startCursor }, + (perPage, cursor) => + listIssueAlertsPaginated(target.org, target.project, { + perPage, + cursor, + }), + ); + return { target, rules, hasMore: !!nextCursor, nextCursor }; }); if (!result.ok) { diff --git a/packages/cli/src/commands/alert/metrics/list.ts b/packages/cli/src/commands/alert/metrics/list.ts index f5f01fe17..9394a2e2e 100644 --- a/packages/cli/src/commands/alert/metrics/list.ts +++ b/packages/cli/src/commands/alert/metrics/list.ts @@ -12,11 +12,8 @@ import type { SentryContext } from "../../../context.js"; import type { MetricAlertRule } from "../../../lib/api/alerts.js"; -import { MAX_PAGINATION_PAGES } from "../../../lib/api/infrastructure.js"; -import { - API_MAX_PER_PAGE, - listMetricAlertsPaginated, -} from "../../../lib/api-client.js"; +import { paginate } from "../../../lib/api/infrastructure.js"; +import { listMetricAlertsPaginated } from "../../../lib/api-client.js"; import { parseOrgProjectArg } from "../../../lib/arg-parsing.js"; import { openInBrowser } from "../../../lib/browser.js"; import { @@ -143,39 +140,12 @@ async function fetchRulesForOrg( options: { limit: number; startCursor?: string }, ): Promise { const result = await withAuthGuard(async () => { - const rules: MetricAlertRule[] = []; - let serverCursor = options.startCursor; - - for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { - const { data, nextCursor } = await listMetricAlertsPaginated(orgSlug, { - perPage: Math.min(options.limit - rules.length, API_MAX_PER_PAGE), - cursor: serverCursor, - }); - - for (const rule of data) { - rules.push(rule); - if (rules.length >= options.limit) { - return { - orgSlug, - rules, - hasMore: !!nextCursor, - nextCursor: nextCursor ?? undefined, - }; - } - } - - if (!nextCursor) { - return { orgSlug, rules, hasMore: false }; - } - serverCursor = nextCursor; - } - - return { - orgSlug, - rules, - hasMore: !!serverCursor, - nextCursor: serverCursor, - }; + const { data: rules, nextCursor } = await paginate( + { limit: options.limit, cursor: options.startCursor }, + (perPage, cursor) => + listMetricAlertsPaginated(orgSlug, { perPage, cursor }), + ); + return { orgSlug, rules, hasMore: !!nextCursor, nextCursor }; }); if (!result.ok) { diff --git a/packages/cli/src/commands/dashboard/list.ts b/packages/cli/src/commands/dashboard/list.ts index 6dfd52213..134267003 100644 --- a/packages/cli/src/commands/dashboard/list.ts +++ b/packages/cli/src/commands/dashboard/list.ts @@ -293,6 +293,9 @@ async function fetchDashboards( const allTitles: string[] = []; let cursorToStore: string | undefined; + // Filters titles client-side, so it cannot count rows with paginate(). It + // always uses one page size and resumes mid-page via afterId bookmarks. + // oxlint-disable-next-line sentry-cli/no-manual-pagination for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { const { data, nextCursor } = await listDashboardsPaginated(orgSlug, { perPage: opts.perPage, @@ -439,11 +442,10 @@ export const listCommand = buildListCommand("dashboard", { ? picomatch(titleFilter.toLowerCase(), { dot: true }) : undefined; - // When filtering, fetch max-size pages to minimize round trips. - // When not filtering, cap at the smaller of limit and API max. - const perPage = glob - ? API_MAX_PER_PAGE - : Math.min(flags.limit, API_MAX_PER_PAGE); + // Dashboards use a page-number paginator whose cursor is only valid with + // the page size that produced it, so every call uses the same size, even + // when `-c next` changes --limit. afterId bookmarks resume mid-page. + const perPage = API_MAX_PER_PAGE; const { dashboards: results, diff --git a/packages/cli/src/commands/dashboard/revisions.ts b/packages/cli/src/commands/dashboard/revisions.ts index 7cc551bac..5f034e05f 100644 --- a/packages/cli/src/commands/dashboard/revisions.ts +++ b/packages/cli/src/commands/dashboard/revisions.ts @@ -5,11 +5,8 @@ */ import type { SentryContext } from "../../context.js"; -import { MAX_PAGINATION_PAGES } from "../../lib/api/infrastructure.js"; -import { - API_MAX_PER_PAGE, - listDashboardRevisionsPaginated, -} from "../../lib/api-client.js"; +import { paginate } from "../../lib/api/infrastructure.js"; +import { listDashboardRevisionsPaginated } from "../../lib/api-client.js"; import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; import { parseOrgProjectTarget } from "../../lib/completion-target.js"; @@ -169,11 +166,7 @@ export const revisionsCommand = buildCommand({ contextKey, ); - const results: DashboardRevision[] = []; - let cursor = rawCursor; - let nextCursor: string | undefined; - - await withDashboardErrors( + const { data: revisions, nextCursor } = await withDashboardErrors( { orgSlug, dashboardId, @@ -182,50 +175,29 @@ export const revisionsCommand = buildCommand({ () => withProgress( { message: "Fetching revisions...", json: flags.json }, - async () => { - for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { - const { data, nextCursor: nc } = - await listDashboardRevisionsPaginated(orgSlug, dashboardId, { - // Ask only for the remaining budget so the last page's - // cursor points at the first row we did not return. - perPage: Math.min( - flags.limit - results.length, - API_MAX_PER_PAGE, - ), - cursor, - }); - results.push(...data); - nextCursor = nc; - if (results.length >= flags.limit || !nc) { - break; - } - cursor = nc; - } - }, + () => + paginate( + { limit: flags.limit, cursor: rawCursor }, + (perPage, pageCursor) => + listDashboardRevisionsPaginated(orgSlug, dashboardId, { + perPage, + cursor: pageCursor, + }), + ), ), ); + const hasMore = nextCursor !== undefined; - const trimmed = results.slice(0, flags.limit); - // A page larger than requested would make the server cursor skip the - // trimmed rows, so drop it (same as autoPaginate). - const cursorToStore = results.length > flags.limit ? undefined : nextCursor; - const hasMore = cursorToStore !== undefined; - - advancePaginationState( - PAGINATION_KEY, - contextKey, - direction, - cursorToStore, - ); + advancePaginationState(PAGINATION_KEY, contextKey, direction, nextCursor); const hasPrev = hasPreviousPage(PAGINATION_KEY, contextKey); const outputData: RevisionsResult = { - revisions: trimmed, + revisions, orgSlug, dashboardId, hasMore, hasPrev: hasPrev || undefined, - nextCursor: cursorToStore, + nextCursor, }; yield new CommandOutput(outputData); @@ -238,13 +210,13 @@ export const revisionsCommand = buildCommand({ }); const navStr = nav ? ` ${nav}` : ""; - if (trimmed.length === 0) { + if (revisions.length === 0) { return { hint: nav ? `No revisions found.${navStr}` : undefined }; } return { hint: - `Showing ${trimmed.length} revision(s) for dashboard ${dashboardId}.${navStr}\n` + + `Showing ${revisions.length} revision(s) for dashboard ${dashboardId}.${navStr}\n` + `Restore: sentry dashboard restore ${orgSlug}/ ${dashboardId} --revision \n` + `Dashboard: ${url}`, }; diff --git a/packages/cli/src/commands/issue/list.ts b/packages/cli/src/commands/issue/list.ts index c5be1e075..0e0758256 100644 --- a/packages/cli/src/commands/issue/list.ts +++ b/packages/cli/src/commands/issue/list.ts @@ -8,12 +8,10 @@ import type { SentryContext } from "../../context.js"; import { buildProjectAliasMap } from "../../lib/alias.js"; import { - API_MAX_PER_PAGE, buildIssueListCollapse, type IssueCollapseField, type IssuesPage, listIssuesAllPages, - listIssuesPaginated, } from "../../lib/api-client.js"; import { extractRequiredScopes } from "../../lib/api-scope.js"; import { @@ -744,8 +742,7 @@ function pageHint( /** * Fetch issues for an organization, optionally restricted to a project set. * - * When `cursor` is provided (--cursor resume), fetches a single page to keep the - * cursor chain intact. Otherwise auto-paginates up to the requested limit. + * Auto-paginates up to the requested limit, resuming from `cursor` when given. */ async function fetchOrganizationIssues( org: string, @@ -762,29 +759,10 @@ async function fetchOrganizationIssues( const timeParams = timeRangeToApiParams(timeRange); const { cursor, onPage, projects } = options; - // When resuming with --cursor, fetch a single page so the cursor chain stays intact. - if (cursor) { - const perPage = Math.min(flags.limit, API_MAX_PER_PAGE); - const response = await listIssuesPaginated(org, "", { - query: flags.query, - cursor, - perPage, - sort: flags.sort, - ...timeParams, - projects, - groupStatsPeriod: apiOpts.groupStatsPeriod, - collapse: apiOpts.collapse, - }); - return { - issues: applyQueryScopedCounts(response.data), - nextCursor: response.nextCursor, - }; - } - - // No cursor — auto-paginate from the beginning via the shared helper. const { issues, nextCursor } = await listIssuesAllPages(org, "", { query: flags.query, limit: flags.limit, + startCursor: cursor, sort: flags.sort, ...timeParams, projects, diff --git a/packages/cli/src/lib/api/events.ts b/packages/cli/src/lib/api/events.ts index 5ea97360f..8b08dca80 100644 --- a/packages/cli/src/lib/api/events.ts +++ b/packages/cli/src/lib/api/events.ts @@ -25,6 +25,7 @@ import { MAX_PAGINATION_PAGES, ORG_FANOUT_CONCURRENCY, type PaginatedResponse, + paginate, unwrapPaginatedResult, unwrapResult, } from "./infrastructure.js"; @@ -242,21 +243,9 @@ export type ListIssueEventsOptions = { /** * List events for a specific issue. * - * Uses the SDK's `listOrganizationIssueEvents` endpoint with region-aware routing. - * When `limit` exceeds {@link API_MAX_PER_PAGE} (100), auto-paginates through - * multiple API calls to fill the requested limit, bounded by - * {@link MAX_PAGINATION_PAGES}. - * - * Page size is capped at `min(limit, API_MAX_PER_PAGE)` via `per_page`, which - * Sentry accepts on this route at runtime even though it is absent from the - * OpenAPI spec. Capping page size keeps the server-issued `nextCursor` aligned - * to a page boundary, preventing the skip bug where trim + keep-cursor would - * jump past items. - * - * When trimming to `limit`, `nextCursor` is PRESERVED: the events cursor is - * offset-based, so resuming from it re-includes any trimmed tail rather than - * skipping it. This prevents both the original skip bug and the stall - * (drop-cursor) regression. + * Uses the SDK's `listOrganizationIssueEvents` endpoint with region-aware routing + * and {@link paginate} for limits above one page. `per_page` is accepted on + * this route at runtime even though it is absent from the OpenAPI spec. * * @param orgSlug - Organization slug for region routing * @param issueId - Numeric issue ID @@ -271,13 +260,8 @@ export async function listIssueEvents( const { limit = 25, query, full, cursor, statsPeriod, start, end } = options; const config = await getOrgSdkConfig(orgSlug); - const perPage = Math.min(limit, API_MAX_PER_PAGE); - - const allEvents: IssueEvent[] = []; - let currentCursor = cursor; - let nextCursor: string | undefined; - for (let page = 0; page < MAX_PAGINATION_PAGES; page += 1) { + return await paginate({ limit, cursor }, async (perPage, pageCursor) => { const result = await listOrganizationIssueEvents({ ...config, path: { @@ -287,7 +271,7 @@ export async function listIssueEvents( query: { query: query || undefined, full, - cursor: currentCursor, + cursor: pageCursor, statsPeriod, start, end, @@ -296,25 +280,9 @@ export async function listIssueEvents( per_page: perPage, } as Parameters[0]["query"], }); - - const paginated = unwrapPaginatedResult( + return unwrapPaginatedResult( result, "Failed to list issue events", ); - - allEvents.push(...(paginated.data as IssueEvent[])); - nextCursor = paginated.nextCursor; - - if (allEvents.length >= limit || !nextCursor) { - break; - } - currentCursor = nextCursor; - } - - // Trim to limit but PRESERVE nextCursor. The events cursor is offset-based, - // so resuming from it re-includes any trimmed tail rather than skipping it. - // Preserving the cursor lets `-c next` advance; dropping it would strand all - // events past the first page. - const data = allEvents.length > limit ? allEvents.slice(0, limit) : allEvents; - return { data, nextCursor }; + }); } diff --git a/packages/cli/src/lib/api/explore.ts b/packages/cli/src/lib/api/explore.ts index fc5ce1e7d..aa13574b4 100644 --- a/packages/cli/src/lib/api/explore.ts +++ b/packages/cli/src/lib/api/explore.ts @@ -15,6 +15,7 @@ import { apiRequestToRegion, MAX_PAGINATION_PAGES, type PaginatedResponse, + paginate, parseLinkHeader, } from "./infrastructure.js"; @@ -163,9 +164,8 @@ export async function queryMetricsMeta( * query, sort, and time range. Supports all standard Sentry Explore fields * including aggregates like `count()`, `count_unique(user)`, `p50(span.duration)`. * - * When `limit` exceeds {@link API_MAX_PER_PAGE}, transparently fetches multiple - * pages using cursor-based pagination (bounded by {@link MAX_PAGINATION_PAGES}). - * Meta is taken from the first page — it is identical across pages. + * Limits above one page are fetched with {@link paginate}. Meta is taken from + * the first page — it is identical across pages. * * @param orgSlug - Organization slug * @param options - Query options @@ -176,52 +176,21 @@ export async function queryEvents( options: ExploreQueryOptions, ): Promise> { const regionUrl = await resolveOrgRegion(orgSlug); - const limit = options.limit ?? 25; - const perPage = Math.min(limit, API_MAX_PER_PAGE); - - // Fast path: single-page fetch when limit fits in one API page - if (limit <= API_MAX_PER_PAGE) { - return await fetchEventsPage(regionUrl, orgSlug, options, perPage); - } - - // Multi-page: accumulate rows across pages up to the requested limit - const allRows: Record[] = []; - let meta: EventsTableResponse["meta"] | undefined; - let cursor: string | undefined = options.cursor; - - for (let page = 0; page < MAX_PAGINATION_PAGES; page += 1) { - const result = await fetchEventsPage( - regionUrl, - orgSlug, - { ...options, cursor }, - perPage, - ); - - if (page === 0) { - meta = result.data.meta; - } - - allRows.push(...result.data.data); - - // Stop when we've reached the requested limit or there are no more pages - if (allRows.length >= limit || !result.nextCursor) { - // Overshot — trim and drop nextCursor (cursor would skip items) - if (allRows.length > limit) { - return { - data: { data: allRows.slice(0, limit), meta }, - }; - } - return { - data: { data: allRows, meta }, - nextCursor: result.nextCursor, - }; - } - - cursor = result.nextCursor; - } - - // Safety limit reached — return what we have, no nextCursor - return { - data: { data: allRows.slice(0, limit), meta }, - }; + let meta: EventsTableResponse["meta"]; + const { data, nextCursor } = await paginate( + options, + async (perPage, cursor) => { + const page = await fetchEventsPage( + regionUrl, + orgSlug, + { ...options, cursor }, + perPage, + ); + // Meta is identical across pages; keep the first page's. + meta ??= page.data.meta; + return { data: page.data.data, nextCursor: page.nextCursor }; + }, + 25, + ); + return { data: { data, meta }, nextCursor }; } diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index af00846bb..3d13761f2 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -25,7 +25,12 @@ import { isEnvTokenActive, } from "../db/auth.js"; import { getEnv } from "../env.js"; -import { ApiError, AuthError, stringifyUnknown } from "../errors.js"; +import { + ApiError, + AuthError, + stringifyUnknown, + ValidationError, +} from "../errors.js"; import { logger } from "../logger.js"; import { resolveOrgRegion } from "../region.js"; import { @@ -477,23 +482,85 @@ export async function autoPaginate( return { data: allRows.slice(0, limit) }; } +/** Marks cursors produced by {@link paginate}; Sentry cursors never use it. */ +const PAGE_CURSOR_PREFIX = "cli1~"; + +/** + * Where a {@link paginate} walk resumes: the server cursor that fetches the + * page, the page size it must be fetched with, and how many of that page's + * rows were already returned. + */ +type PagePosition = { + cursor: string | undefined; + perPage: number; + skip: number; +}; + +function encodePageCursor({ cursor, perPage, skip }: PagePosition): string { + return `${PAGE_CURSOR_PREFIX}${perPage}~${skip}~${cursor ?? ""}`; +} + +/** + * Decode a cursor from {@link paginate}, or treat anything else as a raw + * server cursor (older stored cursors, or one pasted from an API response). + */ +function decodePageCursor( + raw: string | undefined, +): Partial & { cursor: string | undefined } { + if (!raw?.startsWith(PAGE_CURSOR_PREFIX)) { + return { cursor: raw }; + } + const [perPage, skip, ...server] = raw + .slice(PAGE_CURSOR_PREFIX.length) + .split("~"); + const position = { + perPage: Number(perPage), + skip: Number(skip), + cursor: server.join("~") || undefined, + }; + if ( + !Number.isInteger(position.perPage) || + position.perPage < 1 || + position.perPage > API_MAX_PER_PAGE || + !Number.isInteger(position.skip) || + position.skip < 0 + ) { + throw new ValidationError(`Invalid pagination cursor '${raw}'.`, "cursor"); + } + return position; +} + /** - * Wire a single-page fetcher into a limit-bounded, auto-paginating list call. + * Fetch up to `limit` rows and a cursor that resumes after the last one. + * + * This is the only supported way to build a limit-bounded list; the + * `sentry-cli/no-manual-pagination` lint rule rejects hand-written loops. It + * owns the decisions those loops kept getting wrong (#1458, #1608): * - * Centralizes the two things every list endpoint kept re-deriving by hand and - * occasionally got wrong (see #1458): capping `per_page` at - * {@link API_MAX_PER_PAGE} and threading `limit` plus the initial cursor into - * {@link autoPaginate}. Each request asks only for the remaining item budget, - * so a partial final page can return a cursor without skipping trimmed rows. - * Callers still own region resolution and - * endpoint-specific query building inside `fetchPage`. + * - Every request of a walk, including later `-c next` calls with a different + * `--limit`, uses the `per_page` of its first request. Sentry's page-number + * paginators (`OffsetPaginator`, used by projects, teams, members, + * repositories, and dashboard revisions) put the page size in the cursor + * (`::`), and pairing it with another `per_page` + * reads the wrong window and silently skips rows. + * - The returned cursor always resumes at the first row the caller has not + * received. When a call stops inside a page (a smaller `--limit`, or a + * server that ignored `per_page`), the cursor refetches that page and skips + * the rows already returned. + * + * Returned cursors are opaque CLI cursors; pass them back through this + * function, never directly to an API request. Raw server cursors are accepted + * as a starting point. + * + * Callers own region resolution and query building inside `fetchPage`, and + * must send `perPage` as the request's `per_page` unchanged. * * @param options - Caller list options; `limit` bounds total rows, `cursor` is the start cursor - * @param fetchPage - Fetches one page given the capped `perPage` and a page cursor + * @param fetchPage - Fetches one page given the page size and a server cursor * @param defaultLimit - Applied when `options.limit` is undefined - * @returns Accumulated items with optional nextCursor + * @returns Up to `limit` items with optional nextCursor */ -export function paginate( +export async function paginate( options: { limit?: number; cursor?: string }, fetchPage: ( perPage: number, @@ -502,19 +569,50 @@ export function paginate( defaultLimit = 10, ): Promise> { const limit = options.limit ?? defaultLimit; - let remaining = limit; - return autoPaginate( - async (cursor) => { - const result = await fetchPage( - Math.min(remaining, API_MAX_PER_PAGE), - cursor, - ); - remaining -= result.data.length; - return result; - }, - limit, - options.cursor, - ); + const start = decodePageCursor(options.cursor); + const perPage = + start.perPage ?? Math.max(1, Math.min(limit, API_MAX_PER_PAGE)); + let cursor = start.cursor; + let skip = start.skip ?? 0; + const rows: T[] = []; + + for (let page = 0; page < MAX_PAGINATION_PAGES; page += 1) { + const { data, nextCursor } = await fetchPage(perPage, cursor); + const unseen = data.slice(skip); + const remaining = limit - rows.length; + + if (unseen.length > remaining) { + rows.push(...unseen.slice(0, remaining)); + return { + data: rows, + nextCursor: encodePageCursor({ + cursor, + perPage, + skip: skip + remaining, + }), + }; + } + rows.push(...unseen); + if (!nextCursor) { + return { data: rows }; + } + cursor = nextCursor; + skip = 0; + if (rows.length === limit) { + break; + } + } + + if (rows.length < limit) { + logger.warn( + `Pagination limit reached (${MAX_PAGINATION_PAGES} pages, ${rows.length} items). ` + + "Use the returned cursor to continue.", + ); + } + return { + data: rows, + nextCursor: encodePageCursor({ cursor, perPage, skip: 0 }), + }; } /** diff --git a/packages/cli/src/lib/api/issues.ts b/packages/cli/src/lib/api/issues.ts index 00b95afc5..40a4132c1 100644 --- a/packages/cli/src/lib/api/issues.ts +++ b/packages/cli/src/lib/api/issues.ts @@ -26,8 +26,8 @@ import { apiRequest, apiRequestToRegion, getOrgSdkConfig, - MAX_PAGINATION_PAGES, type PaginatedResponse, + paginate, unwrapPaginatedResult, } from "./infrastructure.js"; @@ -213,10 +213,8 @@ export type IssuesPage = { * Auto-paginate through issues up to the requested limit. * * The Sentry API caps `per_page` at {@link API_MAX_PER_PAGE} server-side. When the caller - * requests more than that, this function transparently fetches multiple - * pages using cursor-based pagination and returns the combined result. - * - * Safety-bounded by {@link MAX_PAGINATION_PAGES} to prevent runaway requests. + * requests more than that, {@link paginate} transparently fetches multiple + * pages and returns the combined result. * * @param orgSlug - Organization slug * @param projectSlug - Single-slug fallback used when `options.projects` is @@ -255,45 +253,28 @@ export async function listIssuesAllPages( ); } - const allResults: SentryIssue[] = []; - let cursor: string | undefined = options.startCursor; - - for (let page = 0; page < MAX_PAGINATION_PAGES; page++) { - const remaining = options.limit - allResults.length; - const perPage = Math.min(remaining, API_MAX_PER_PAGE); - const response = await listIssuesPaginated(orgSlug, projectSlug, { - query: options.query, - cursor, - perPage, - sort: options.sort, - statsPeriod: options.statsPeriod, - start: options.start, - end: options.end, - projects: options.projects, - groupStatsPeriod: options.groupStatsPeriod, - collapse: options.collapse, - }); - - allResults.push(...response.data); - options.onPage?.(Math.min(allResults.length, options.limit), options.limit); - - // Request only the remaining number of items on each page. A conforming - // API therefore reaches the limit exactly and its cursor remains usable. - // Keep the defensive overshoot branch for servers that ignore `limit`. - if (allResults.length >= options.limit || !response.nextCursor) { - if (allResults.length > options.limit) { - // The cursor resumes after the oversized page; exposing it would skip - // the trimmed items that the caller has not received. - return { issues: allResults.slice(0, options.limit) }; - } - return { issues: allResults, nextCursor: response.nextCursor }; - } - - cursor = response.nextCursor; - } - - // Safety limit reached — return what we have, no nextCursor - return { issues: allResults.slice(0, options.limit) }; + let fetched = 0; + const { data: issues, nextCursor } = await paginate( + { limit: options.limit, cursor: options.startCursor }, + async (perPage, cursor) => { + const response = await listIssuesPaginated(orgSlug, projectSlug, { + query: options.query, + cursor, + perPage, + sort: options.sort, + statsPeriod: options.statsPeriod, + start: options.start, + end: options.end, + projects: options.projects, + groupStatsPeriod: options.groupStatsPeriod, + collapse: options.collapse, + }); + fetched += response.data.length; + options.onPage?.(Math.min(fetched, options.limit), options.limit); + return response; + }, + ); + return { issues, nextCursor }; } /** diff --git a/packages/cli/test/commands/dashboard/list.test.ts b/packages/cli/test/commands/dashboard/list.test.ts index 1f03d72d6..1d6f322e7 100644 --- a/packages/cli/test/commands/dashboard/list.test.ts +++ b/packages/cli/test/commands/dashboard/list.test.ts @@ -294,7 +294,7 @@ describe("dashboard list command", () => { // --limit flag // ------------------------------------------------------------------------- - test("passes limit as perPage to API", async () => { + test("requests full pages regardless of limit", async () => { resolveOrgSpy.mockResolvedValue("test-org"); listDashboardsPaginatedSpy.mockResolvedValue({ data: [DASHBOARD_A], @@ -306,12 +306,12 @@ describe("dashboard list command", () => { await func.call(context, defaultFlags({ json: true, limit: 10 })); expect(withProgressSpy).toHaveBeenCalled(); - // perPage is Math.min(flags.limit, API_MAX_PER_PAGE). Verify org and cursor; - // perPage derivation tested via integration in the command's own tests. - expect(listDashboardsPaginatedSpy).toHaveBeenCalledWith( - "test-org", - expect.objectContaining({ cursor: undefined }), - ); + // The page size stays fixed so a cursor resumed with another --limit + // still matches the page size the server encoded in it. + expect(listDashboardsPaginatedSpy).toHaveBeenCalledWith("test-org", { + cursor: undefined, + perPage: apiClient.API_MAX_PER_PAGE, + }); }); // ------------------------------------------------------------------------- diff --git a/packages/cli/test/commands/dashboard/revisions.test.ts b/packages/cli/test/commands/dashboard/revisions.test.ts index 1d6b3a442..700392871 100644 --- a/packages/cli/test/commands/dashboard/revisions.test.ts +++ b/packages/cli/test/commands/dashboard/revisions.test.ts @@ -268,79 +268,28 @@ describe("dashboard revisions command", () => { expect(output).toContain("-c prev"); }); - test("auto-pagination: --limit larger than page size fetches multiple pages", async () => { - // First page returns 2 items + nextCursor, second page returns 1 item + test("auto-pagination: --limit above one page keeps one per_page", async () => { + const page = Array.from({ length: 100 }, () => REVISION_A); listDashboardRevisionsPaginatedSpy - .mockResolvedValueOnce({ - data: [REVISION_A, REVISION_B], - nextCursor: "cursor-page-2", - }) - .mockResolvedValueOnce({ - data: [REVISION_C], - nextCursor: undefined, - }); + .mockResolvedValueOnce({ data: page, nextCursor: "100:1:0" }) + .mockResolvedValueOnce({ data: page, nextCursor: "100:2:0" }); const { context, stdoutWrite } = createMockContext(); const func = await revisionsCommand.loader(); - // Request 3 items, which exceeds a single page of 2 - await func.call(context, defaultFlags({ json: true, limit: 3 }), "123"); - - // Should have called API twice - expect(listDashboardRevisionsPaginatedSpy).toHaveBeenCalledTimes(2); - - const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(output); - expect(parsed.data).toHaveLength(3); - expect(parsed.hasMore).toBe(false); - }); - - test("auto-pagination: later pages request only the remaining budget", async () => { - listDashboardRevisionsPaginatedSpy - .mockResolvedValueOnce({ - data: [REVISION_A, REVISION_B], - nextCursor: "cursor-page-2", - }) - .mockResolvedValueOnce({ - data: [REVISION_C], - nextCursor: "cursor-page-3", - }); - - const { context, stdoutWrite } = createMockContext(); - const func = await revisionsCommand.loader(); - await func.call(context, defaultFlags({ json: true, limit: 3 }), "123"); - - expect(listDashboardRevisionsPaginatedSpy).toHaveBeenLastCalledWith( - "test-org", - "123", - { perPage: 1, cursor: "cursor-page-2" }, - ); + await func.call(context, defaultFlags({ json: true, limit: 200 }), "123"); + + // The revisions endpoint uses OffsetPaginator, whose cursor encodes the + // page size, so every request must send the same per_page. + expect( + listDashboardRevisionsPaginatedSpy.mock.calls.map((call) => call[2]), + ).toEqual([ + { perPage: 100, cursor: undefined }, + { perPage: 100, cursor: "100:1:0" }, + ]); const parsed = JSON.parse(stdoutWrite.mock.calls.map((c) => c[0]).join("")); - expect(parsed.data).toHaveLength(3); + expect(parsed.data).toHaveLength(200); expect(parsed.hasMore).toBe(true); - expect(parsed.nextCursor).toBe("cursor-page-3"); - }); - - test("auto-pagination: drops the cursor when a page overshoots the limit", async () => { - // An endpoint that ignores per_page returns more rows than asked for; its - // cursor would skip the trimmed rows on `-c next`. - listDashboardRevisionsPaginatedSpy - .mockResolvedValueOnce({ - data: [REVISION_A, REVISION_B], - nextCursor: "cursor-page-2", - }) - .mockResolvedValueOnce({ - data: [REVISION_C, REVISION_A], - nextCursor: "cursor-page-3", - }); - - const { context, stdoutWrite } = createMockContext(); - const func = await revisionsCommand.loader(); - await func.call(context, defaultFlags({ json: true, limit: 3 }), "123"); - - const parsed = JSON.parse(stdoutWrite.mock.calls.map((c) => c[0]).join("")); - expect(parsed.data).toHaveLength(3); - expect(parsed.hasMore).toBe(false); - expect(parsed.nextCursor).toBeUndefined(); + expect(parsed.nextCursor).toContain("100:2:0"); }); // ------------------------------------------------------------------------- diff --git a/packages/cli/test/commands/dsn/list.test.ts b/packages/cli/test/commands/dsn/list.test.ts index 1cc8c73f8..9ad1c0ad7 100644 --- a/packages/cli/test/commands/dsn/list.test.ts +++ b/packages/cli/test/commands/dsn/list.test.ts @@ -554,7 +554,7 @@ describe("dsn list", () => { expect(first).toMatchObject({ hasMore: true, hasPrev: false, - nextCursor: "0:1:0", + nextCursor: expect.stringContaining("0:1:0"), data: [{ org: "test-org", project: "test-project" }], }); const second = await invokeJson({ limit: 1, cursor: "next" }); diff --git a/packages/cli/test/commands/integration/list.test.ts b/packages/cli/test/commands/integration/list.test.ts index e383578e4..a337f3449 100644 --- a/packages/cli/test/commands/integration/list.test.ts +++ b/packages/cli/test/commands/integration/list.test.ts @@ -98,11 +98,11 @@ describe("integration list", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { limit: 25, json: true }, "my-org/"); + await func.call(context, { limit: 2, json: true }, "my-org/"); const parsed = JSON.parse(stdoutWrite.mock.calls.map((c) => c[0]).join("")); expect(parsed).toHaveProperty("hasMore", true); - expect(parsed).toHaveProperty("nextCursor", "0:25:0"); + expect(parsed.nextCursor).toContain("0:25:0"); expect(parsed.data).toHaveLength(2); expect(parsed.data[0]).toMatchObject({ id: "101", diff --git a/packages/cli/test/commands/issue/list.test.ts b/packages/cli/test/commands/issue/list.test.ts index aaa674bac..66ed0aaf8 100644 --- a/packages/cli/test/commands/issue/list.test.ts +++ b/packages/cli/test/commands/issue/list.test.ts @@ -1097,8 +1097,8 @@ describe("issue list: org-all mode (cursor pagination)", () => { cursor: "cached:cursor:789", direction: "next", }); - listIssuesPaginatedMock.mockResolvedValue({ - data: [sampleIssue], + listIssuesAllPagesMock.mockResolvedValue({ + issues: [sampleIssue], nextCursor: undefined, }); @@ -1121,10 +1121,10 @@ describe("issue list: org-all mode (cursor pagination)", () => { "my-org/", ); - expect(listIssuesPaginatedMock).toHaveBeenCalledWith( + expect(listIssuesAllPagesMock).toHaveBeenCalledWith( "my-org", "", - expect.objectContaining({ cursor: "cached:cursor:789" }), + expect.objectContaining({ limit: 10, startCursor: "cached:cursor:789" }), ); }); @@ -1165,8 +1165,8 @@ describe("issue list: org-all mode (cursor pagination)", () => { cursor: "explicit:cursor:val", direction: "next", }); - listIssuesPaginatedMock.mockResolvedValue({ - data: [sampleIssue], + listIssuesAllPagesMock.mockResolvedValue({ + issues: [sampleIssue], nextCursor: undefined, }); @@ -1189,10 +1189,13 @@ describe("issue list: org-all mode (cursor pagination)", () => { "my-org/", ); - expect(listIssuesPaginatedMock).toHaveBeenCalledWith( + expect(listIssuesAllPagesMock).toHaveBeenCalledWith( "my-org", "", - expect.objectContaining({ cursor: "explicit:cursor:val" }), + expect.objectContaining({ + limit: 10, + startCursor: "explicit:cursor:val", + }), ); }); }); @@ -1235,6 +1238,12 @@ describe("issue list: Phase 2 budget redistribution", () => { // Surplus: 6 - 4 = 2, expandable = [org-one] // Phase 2: fetch 2 more from org-one via cursor resume. + beforeEach(() => { + // Exercise the real pagination helpers, not overrides left by earlier tests. + listIssuesPaginatedMock.mockReset(); + listIssuesAllPagesMock.mockReset(); + }); + test("redistributes surplus to expandable targets", async () => { setOrgRegion("org-one", DEFAULT_SENTRY_URL); setOrgRegion("org-two", DEFAULT_SENTRY_URL); @@ -2198,10 +2207,6 @@ describe("issue list: comma-separated project slugs", () => { ], nextCursor: "1735689600:0:1", }); - listIssuesPaginatedMock.mockResolvedValue({ - data: [], - nextCursor: undefined, - }); await func.call(createContext().context, baseFlags, "test-org/web,api"); await func.call( @@ -2210,10 +2215,13 @@ describe("issue list: comma-separated project slugs", () => { "test-org/api,web", ); - expect(listIssuesPaginatedMock).toHaveBeenCalledWith( + expect(listIssuesAllPagesMock).toHaveBeenLastCalledWith( "test-org", "", - expect.objectContaining({ cursor: "1735689600:0:1", projects: [2, 1] }), + expect.objectContaining({ + startCursor: "1735689600:0:1", + projects: [2, 1], + }), ); }); diff --git a/packages/cli/test/commands/project/list.test.ts b/packages/cli/test/commands/project/list.test.ts index 4fcb85e15..5eaf6f2ec 100644 --- a/packages/cli/test/commands/project/list.test.ts +++ b/packages/cli/test/commands/project/list.test.ts @@ -668,14 +668,14 @@ describe("handleOrgAll", () => { const result = await handleOrgAll({ org: "test-org", - flags: { limit: 30, json: true, fresh: false }, + flags: { limit: 2, json: true, fresh: false }, contextKey: "type:org:test-org", cursor: undefined, direction: "next", }); expect(result.hasMore).toBe(true); - expect(result.nextCursor).toBe("1735689600000:100:0"); + expect(result.nextCursor).toContain("1735689600000:100:0"); expect(result.items).toHaveLength(2); }); @@ -702,7 +702,7 @@ describe("handleOrgAll", () => { await handleOrgAll({ org: "test-org", - flags: { limit: 30, json: false, fresh: false }, + flags: { limit: 2, json: false, fresh: false }, contextKey: "type:org:test-org", cursor: undefined, direction: "next", @@ -710,7 +710,7 @@ describe("handleOrgAll", () => { const state = getPaginationState(PAGINATION_KEY, "type:org:test-org"); expect(state).toBeDefined(); - expect(state!.stack).toContain("1735689600000:100:0"); + expect(state!.stack.at(-1)).toContain("1735689600000:100:0"); }); test("no hasMore truncates pagination stack", async () => { @@ -1485,7 +1485,7 @@ describe("handleAutoDetect", () => { }); const result = await handleAutoDetect("/tmp/test-project", { - limit: 30, + limit: 2, json: false, fresh: false, }); @@ -1503,7 +1503,7 @@ describe("handleAutoDetect", () => { }); const result = await handleAutoDetect("/tmp/test-project", { - limit: 30, + limit: 2, json: true, fresh: false, }); diff --git a/packages/cli/test/commands/repo/list.test.ts b/packages/cli/test/commands/repo/list.test.ts index 2e05adcac..6836d8715 100644 --- a/packages/cli/test/commands/repo/list.test.ts +++ b/packages/cli/test/commands/repo/list.test.ts @@ -414,12 +414,12 @@ describe("listCommand.func — org-all mode (cursor pagination)", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { limit: 25, json: true }, "my-org/"); + await func.call(context, { limit: 2, json: true }, "my-org/"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); expect(parsed).toHaveProperty("hasMore", true); - expect(parsed).toHaveProperty("nextCursor", "cursor:abc:123"); + expect(parsed.nextCursor).toContain("cursor:abc:123"); expect(advancePaginationStateSpy).toHaveBeenCalled(); }); diff --git a/packages/cli/test/commands/team/list.test.ts b/packages/cli/test/commands/team/list.test.ts index fd3575a86..ebe02e60c 100644 --- a/packages/cli/test/commands/team/list.test.ts +++ b/packages/cli/test/commands/team/list.test.ts @@ -400,12 +400,12 @@ describe("listCommand.func — org-all mode (cursor pagination)", () => { const { context, stdoutWrite } = createMockContext(); const func = await listCommand.loader(); - await func.call(context, { limit: 25, json: true }, "my-org/"); + await func.call(context, { limit: 2, json: true }, "my-org/"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); expect(parsed).toHaveProperty("hasMore", true); - expect(parsed).toHaveProperty("nextCursor", "cursor:abc:123"); + expect(parsed.nextCursor).toContain("cursor:abc:123"); expect(advancePaginationStateSpy).toHaveBeenCalled(); }); diff --git a/packages/cli/test/e2e/feedback.test.ts b/packages/cli/test/e2e/feedback.test.ts index bca4e7d81..4c08b5c73 100644 --- a/packages/cli/test/e2e/feedback.test.ts +++ b/packages/cli/test/e2e/feedback.test.ts @@ -285,7 +285,8 @@ describe("sentry feedback list", () => { "--json", ]); expect(firstPage.exitCode, firstPage.stderr + firstPage.stdout).toBe(0); - expect(JSON.parse(firstPage.stdout).nextCursor).toBe( + // paginate() wraps the server cursor with the walk's page size. + expect(JSON.parse(firstPage.stdout).nextCursor).toContain( `feedback-limit-${limit}-next`, ); } diff --git a/packages/cli/test/lib/api-client.coverage.test.ts b/packages/cli/test/lib/api-client.coverage.test.ts index 65affaed6..9c53edd89 100644 --- a/packages/cli/test/lib/api-client.coverage.test.ts +++ b/packages/cli/test/lib/api-client.coverage.test.ts @@ -349,36 +349,6 @@ describe("issues.ts", () => { expect(callCount).toBe(2); }); - test("trims when overshooting limit (no nextCursor)", async () => { - // Return 5 items but we only want 3 - globalThis.fetch = mockFetch( - async () => - new Response( - JSON.stringify([ - mockIssue({ id: "1" }), - mockIssue({ id: "2" }), - mockIssue({ id: "3" }), - mockIssue({ id: "4" }), - mockIssue({ id: "5" }), - ]), - { - status: 200, - headers: { - "Content-Type": "application/json", - Link: linkHeader("next", true), - }, - }, - ), - ); - - const result = await listIssuesAllPages("test-org", "test-project", { - limit: 3, - }); - expect(result.issues).toHaveLength(3); - // Trimmed — no nextCursor to prevent skipping - expect(result.nextCursor).toBeUndefined(); - }); - test("throws when limit < 1", async () => { await expect( listIssuesAllPages("test-org", "test-project", { limit: 0 }), @@ -413,61 +383,6 @@ describe("issues.ts", () => { expect(onPageCalls[0]).toEqual([2, 10]); }); - test("returns nextCursor when last page has more results but limit reached", async () => { - // Return exactly limit items with a next cursor - globalThis.fetch = mockFetch( - async () => - new Response( - JSON.stringify([mockIssue({ id: "1" }), mockIssue({ id: "2" })]), - { - status: 200, - headers: { - "Content-Type": "application/json", - Link: linkHeader("has-more", true), - }, - }, - ), - ); - - const result = await listIssuesAllPages("test-org", "test-project", { - limit: 2, - }); - expect(result.issues).toHaveLength(2); - // Exactly at limit with more pages → keep nextCursor - expect(result.nextCursor).toBe("has-more"); - }); - - test("uses the remaining limit on the final page and preserves its cursor", async () => { - const requestedLimits: string[] = []; - let callCount = 0; - - globalThis.fetch = mockFetch(async (input, init) => { - callCount += 1; - const req = new Request(input!, init); - const url = new URL(req.url); - requestedLimits.push(url.searchParams.get("limit") ?? ""); - const count = callCount === 3 ? 50 : 100; - const issues = Array.from({ length: count }, (_, index) => - mockIssue({ id: `${callCount}-${index}` }), - ); - return new Response(JSON.stringify(issues), { - status: 200, - headers: { - "Content-Type": "application/json", - Link: linkHeader(`cursor-${callCount + 1}`, true), - }, - }); - }); - - const result = await listIssuesAllPages("test-org", "test-project", { - limit: 250, - }); - - expect(result.issues).toHaveLength(250); - expect(requestedLimits).toEqual(["100", "100", "50"]); - expect(result.nextCursor).toBe("cursor-4"); - }); - test("accepts startCursor option", async () => { let capturedUrl = ""; @@ -1832,10 +1747,12 @@ describe("traces.ts (transactions)", () => { }); }); - const result = await listTransactions("test-org", "test-project"); + const result = await listTransactions("test-org", "test-project", { + limit: 1, + }); expect(result.data).toHaveLength(1); expect(result.data[0]!.transaction).toBe("GET /api/users"); - expect(result.nextCursor).toBe("txn-next"); + expect(result.nextCursor).toContain("txn-next"); }); test("uses numeric project ID via project param", async () => { diff --git a/packages/cli/test/lib/api-client.test.ts b/packages/cli/test/lib/api-client.test.ts index 0fd11195a..2f103a1fe 100644 --- a/packages/cli/test/lib/api-client.test.ts +++ b/packages/cli/test/lib/api-client.test.ts @@ -1489,10 +1489,12 @@ describe("listTransactions", () => { return new Response(JSON.stringify({}), { status: 200 }); }; - const result = await listTransactions("my-org", "my-project"); + const result = await listTransactions("my-org", "my-project", { + limit: 1, + }); expect(Array.isArray(result.data)).toBe(true); expect(result.data).toHaveLength(1); - expect(result.nextCursor).toBe("1735689600:0:0"); + expect(result.nextCursor).toContain("1735689600:0:0"); }); test("returns no nextCursor when link header has results=false", async () => { diff --git a/packages/cli/test/lib/api/conversations.test.ts b/packages/cli/test/lib/api/conversations.test.ts index c284a96e2..0aadb3d07 100644 --- a/packages/cli/test/lib/api/conversations.test.ts +++ b/packages/cli/test/lib/api/conversations.test.ts @@ -208,7 +208,7 @@ describe("listConversations", () => { const result = await listConversations(ORG); - expect(result.nextCursor).toBe("1735689600000:0:0"); + expect(result.nextCursor).toContain("1735689600000:0:0"); }); test("returns undefined nextCursor when no more pages", async () => { diff --git a/packages/cli/test/lib/api/events-overshoot.test.ts b/packages/cli/test/lib/api/events-overshoot.test.ts index 6fec75cdc..a62afb4c6 100644 --- a/packages/cli/test/lib/api/events-overshoot.test.ts +++ b/packages/cli/test/lib/api/events-overshoot.test.ts @@ -1,16 +1,9 @@ /** * Regression tests for listIssueEvents pagination. * - * Two bugs framed this behavior: - * 1. The original skip bug: trimming an overshooting page while returning the - * server's next-page cursor skipped the trimmed tail on `-c next`. - * 2. The overcorrection: dropping the cursor on overshoot stranded all events - * past the first `limit`, so `sentry issue events` could never page forward. - * - * The fix caps page size with `per_page = min(limit, API_MAX_PER_PAGE)` so the - * server cursor is page-aligned, and trims defensively while PRESERVING the - * cursor (the events cursor is offset-based, so resuming re-includes any trimmed - * tail). These tests pin both the `per_page` request and the cursor preservation. + * Without `per_page`, every page overshot the limit. listIssueEvents now sends + * the walk's page size; trimming and cursor continuity are the shared + * `paginate()` contract, covered by test/lib/paginate.property.test.ts. */ import { afterEach, beforeEach, describe, expect, test } from "vitest"; @@ -32,11 +25,6 @@ afterEach(() => { globalThis.fetch = originalFetch; }); -/** Build a Sentry pagination Link header advertising a next page. */ -function nextLinkHeader(cursor: string): string { - return `; rel="next"; results="true"; cursor="${cursor}"`; -} - function makeEvents(count: number): Array<{ id: string }> { return Array.from({ length: count }, (_unused, i) => ({ id: `evt-${i}` })); } @@ -71,60 +59,4 @@ describe("listIssueEvents pagination", () => { expect(new URL(capturedUrl).searchParams.get("per_page")).toBe("100"); }); - - test("trims to limit but PRESERVES nextCursor when a page overshoots", async () => { - // Server ignores per_page and returns 5 with a next cursor; caller wants 2. - globalThis.fetch = mockFetch( - async () => - new Response(JSON.stringify(makeEvents(5)), { - status: 200, - headers: { - "Content-Type": "application/json", - Link: nextLinkHeader("page2"), - }, - }), - ); - - const result = await listIssueEvents("test-org", "123", { limit: 2 }); - - expect(result.data).toHaveLength(2); - expect(result.data[0]?.id).toBe("evt-0"); - expect(result.data[1]?.id).toBe("evt-1"); - // Cursor preserved so `-c next` can advance; the offset-based events cursor - // re-includes the trimmed tail rather than skipping it. - expect(result.nextCursor).toBe("page2"); - }); - - test("preserves nextCursor when the page exactly fills the limit", async () => { - globalThis.fetch = mockFetch( - async () => - new Response(JSON.stringify(makeEvents(2)), { - status: 200, - headers: { - "Content-Type": "application/json", - Link: nextLinkHeader("page2"), - }, - }), - ); - - const result = await listIssueEvents("test-org", "123", { limit: 2 }); - - expect(result.data).toHaveLength(2); - expect(result.nextCursor).toBe("page2"); - }); - - test("returns all events with no cursor when under the limit", async () => { - globalThis.fetch = mockFetch( - async () => - new Response(JSON.stringify(makeEvents(3)), { - status: 200, - headers: { "Content-Type": "application/json" }, - }), - ); - - const result = await listIssueEvents("test-org", "123", { limit: 25 }); - - expect(result.data).toHaveLength(3); - expect(result.nextCursor).toBeUndefined(); - }); }); diff --git a/packages/cli/test/lib/api/explore.test.ts b/packages/cli/test/lib/api/explore.test.ts index e9cf7d1a8..9514c68ca 100644 --- a/packages/cli/test/lib/api/explore.test.ts +++ b/packages/cli/test/lib/api/explore.test.ts @@ -194,6 +194,7 @@ describe("queryEvents", () => { const result = await queryEvents("my-org", { fields: ["title", "count()"], + limit: 1, }); expect(result.data.data).toHaveLength(1); @@ -201,7 +202,7 @@ describe("queryEvents", () => { title: "string", "count()": "integer", }); - expect(result.nextCursor).toBe(cursor); + expect(result.nextCursor).toContain(cursor); }); test("returns undefined nextCursor when results=false in Link header", async () => { @@ -272,7 +273,7 @@ describe("queryEvents", () => { }, }, { - body: { data: makeRows(50), meta: PAGE_META }, + body: { data: makeRows(100), meta: PAGE_META }, headers: { Link: `; rel="next"; results="false"; cursor=""`, }, @@ -281,38 +282,16 @@ describe("queryEvents", () => { const result = await queryEvents("my-org", { fields: ["title", "count()"], - limit: 150, + limit: 200, }); - expect(result.data.data).toHaveLength(150); + expect(result.data.data).toHaveLength(200); expect(result.data.meta?.fields).toEqual(PAGE_META.fields); expect(result.nextCursor).toBeUndefined(); expect(getCapturedUrls()).toHaveLength(2); - }); - - test("trims results and drops nextCursor when overshoot", async () => { - mockSequential([ - { - body: { data: makeRows(100), meta: PAGE_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:100:0"`, - }, - }, - { - body: { data: makeRows(100), meta: PAGE_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:200:0"`, - }, - }, - ]); - - const result = await queryEvents("my-org", { - fields: ["title", "count()"], - limit: 120, - }); - - expect(result.data.data).toHaveLength(120); - expect(result.nextCursor).toBeUndefined(); + for (const url of getCapturedUrls()) { + expect(url).toContain("per_page=100"); + } }); test("single-page fast path when limit <= API_MAX_PER_PAGE", async () => { diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 95f212f09..6f84d1a3d 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -13,7 +13,11 @@ import { unwrapResult, } from "../../../src/lib/api/infrastructure.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; -import { ApiError, HostScopeError } from "../../../src/lib/errors.js"; +import { + ApiError, + HostScopeError, + ValidationError, +} from "../../../src/lib/errors.js"; import { mockFetch, useEnvSandbox, useTestConfigDir } from "../../helpers.js"; describe("SDK error compatibility", () => { @@ -888,45 +892,44 @@ describe("paginate", () => { expect(spy).toHaveBeenCalledWith(5, undefined); }); - test("accumulates across pages when limit exceeds cap", async () => { + // Cursor continuity across servers and limits is covered by + // test/lib/paginate.property.test.ts; this pins the request sequence. + test("keeps the walk's page size and resumes mid-page across -c next", async () => { const spy = vi.fn((perPage: number, cursor: string | undefined) => { const offset = cursor ? Number(cursor) : 0; const data = Array.from({ length: perPage }, (_, i) => offset + i); - return Promise.resolve({ - data, - nextCursor: String(offset + perPage), - }); + return Promise.resolve({ data, nextCursor: String(offset + perPage) }); }); - const result = await paginate({ limit: 150 }, spy); - expect(result.data).toEqual(Array.from({ length: 150 }, (_, i) => i)); - expect(spy.mock.calls.map(([perPage]) => perPage)).toEqual([ - API_MAX_PER_PAGE, - 50, + const first = await paginate({ limit: 150 }, spy); + expect(first.data).toEqual(Array.from({ length: 150 }, (_, i) => i)); + const second = await paginate({ limit: 10, cursor: first.nextCursor }, spy); + expect(second.data).toEqual(Array.from({ length: 10 }, (_, i) => 150 + i)); + + // The first call stops inside page 2, so the second refetches it with the + // walk's page size rather than one derived from its own --limit. + expect(spy.mock.calls).toEqual([ + [100, undefined], + [100, "100"], + [100, "100"], ]); - expect(result.nextCursor).toBe("150"); }); - test("drops nextCursor when a page exceeds the remaining budget", async () => { - const spy = vi - .fn() - .mockResolvedValueOnce({ - data: Array.from({ length: 100 }, (_, i) => i), - nextCursor: "c1", - }) - .mockResolvedValueOnce({ - data: Array.from({ length: 100 }, (_, i) => 100 + i), - nextCursor: "c2", - }); - - const result = await paginate({ limit: 150 }, spy); - expect(result.data).toHaveLength(150); - expect(result.nextCursor).toBeUndefined(); - expect(spy.mock.calls.map(([perPage]) => perPage)).toEqual([ - API_MAX_PER_PAGE, - 50, - ]); + test("returns no cursor once the server is exhausted", async () => { + const spy = vi.fn().mockResolvedValue({ data: [1, 2] }); + expect(await paginate({ limit: 5 }, spy)).toEqual({ data: [1, 2] }); }); + + test.each(["cli1~0~0~abc", "cli1~101~0~abc", "cli1~10~-1~abc", "cli1~x~0~"])( + "rejects the malformed CLI cursor %s", + async (cursor) => { + const spy = vi.fn().mockResolvedValue({ data: [] }); + await expect(paginate({ limit: 5, cursor }, spy)).rejects.toThrow( + ValidationError, + ); + expect(spy).not.toHaveBeenCalled(); + }, + ); }); describe("fetchAllPages", () => { diff --git a/packages/cli/test/lib/api/logs.test.ts b/packages/cli/test/lib/api/logs.test.ts index 220c24399..31e9e4031 100644 --- a/packages/cli/test/lib/api/logs.test.ts +++ b/packages/cli/test/lib/api/logs.test.ts @@ -210,7 +210,7 @@ describe("listLogs", () => { link: `; rel="next"; results="true"; cursor="0:100:0"`, }, { - body: { data: makeRows(50, 100), meta: { fields: {} } }, + body: { data: makeRows(100, 100), meta: { fields: {} } }, link: `; rel="next"; results="false"; cursor=""`, }, ]; @@ -226,10 +226,13 @@ describe("listLogs", () => { }); }); - const logs = await listLogs("test-org", "my-project", { limit: 150 }); + const logs = await listLogs("test-org", "my-project", { limit: 200 }); - expect(logs).toHaveLength(150); + expect(logs).toHaveLength(200); expect(urls).toHaveLength(2); + for (const url of urls) { + expect(url).toContain("per_page=100"); + } }); }); diff --git a/packages/cli/test/lib/api/project-dsns.test.ts b/packages/cli/test/lib/api/project-dsns.test.ts index 5ee0e6320..6ec9e8e37 100644 --- a/packages/cli/test/lib/api/project-dsns.test.ts +++ b/packages/cli/test/lib/api/project-dsns.test.ts @@ -144,7 +144,7 @@ describe.each([ expect(projectIds.sort()).toEqual(organization ? ["42", "7"] : []); }); - test("forwards page budgets and cursors, reusing project lookups across pages", async () => { + test("keeps the walk's page size across calls and reuses project lookups", async () => { const { requests, projectIds } = mockKeys((url) => { expect(url.pathname).toBe(path); expect(url.searchParams.get("project")).toBe(organization ? "-1" : null); @@ -163,7 +163,7 @@ describe.each([ const first = await list({ limit: 150, cursor: "0:25:0" }); expect(projectIds).toEqual(organization ? ["42"] : []); expect(first.data).toHaveLength(150); - expect(first.nextCursor).toBe("0:175:0"); + expect(first.nextCursor).toContain("0:125:0"); const second = await list({ limit: 50, cursor: first.nextCursor }); expect( @@ -173,8 +173,9 @@ describe.each([ }), ).toEqual([ ["100", "0:25:0"], - ["50", "0:125:0"], - ["50", "0:175:0"], + ["100", "0:125:0"], + // -c next refetches the partly returned page with the walk's page size. + ["100", "0:125:0"], ]); expect(second.nextCursor).toBeUndefined(); expect([...first.data, ...second.data].map((key) => key.name)).toEqual( diff --git a/packages/cli/test/lib/api/replays.test.ts b/packages/cli/test/lib/api/replays.test.ts index 381234e24..aed2a3d8b 100644 --- a/packages/cli/test/lib/api/replays.test.ts +++ b/packages/cli/test/lib/api/replays.test.ts @@ -69,7 +69,8 @@ describe("listReplays", () => { globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input!, init); capturedUrl = req.url; - return new Response(JSON.stringify({ data: [replayRow()] }), { + const data = Array.from({ length: 25 }, () => replayRow()); + return new Response(JSON.stringify({ data }), { status: 200, headers: { "Content-Type": "application/json", @@ -95,8 +96,8 @@ describe("listReplays", () => { expect(url.searchParams.get("per_page")).toBe("25"); expect(url.searchParams.getAll("field")).toContain("id"); expect(url.searchParams.getAll("field")).toContain("user"); - expect(result.data).toHaveLength(1); - expect(result.nextCursor).toBe("0:25:0"); + expect(result.data).toHaveLength(25); + expect(result.nextCursor).toContain("0:25:0"); }); test("passes replay environment filters and custom field selection", async () => { @@ -143,7 +144,7 @@ describe("listReplays", () => { ), } : { - data: Array.from({ length: 50 }, (_, index) => + data: Array.from({ length: 100 }, (_, index) => replayRow(index.toString(16).padStart(32, "b").slice(-32)), ), }; @@ -163,13 +164,13 @@ describe("listReplays", () => { }); }); - const result = await listReplays("test-org", { limit: 150 }); + const result = await listReplays("test-org", { limit: 200 }); - expect(result.data).toHaveLength(150); + expect(result.data).toHaveLength(200); expect(result.nextCursor).toBeUndefined(); expect(capturedUrls).toHaveLength(2); expect(capturedUrls[0]).toContain("per_page=100"); - expect(capturedUrls[1]).toContain("per_page=50"); + expect(capturedUrls[1]).toContain("per_page=100"); }); }); diff --git a/packages/cli/test/lib/api/traces.test.ts b/packages/cli/test/lib/api/traces.test.ts index 9b6cb5ac0..0d0b8bb11 100644 --- a/packages/cli/test/lib/api/traces.test.ts +++ b/packages/cli/test/lib/api/traces.test.ts @@ -115,28 +115,6 @@ describe("listTransactions", () => { expect(decodeURIComponent(capturedUrl)).toContain("is_transaction:true"); }); - test("uses the remaining item budget for the final page", async () => { - const { getCapturedUrls } = mockSequential([ - { - body: { data: makeTxnRows(100), meta: TX_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:100:0"`, - }, - }, - { - body: { data: makeTxnRows(50), meta: TX_META }, - headers: { - Link: `; rel="next"; results="false"; cursor=""`, - }, - }, - ]); - - await listTransactions("my-org", "my-project", { limit: 150 }); - - expect(getCapturedUrls()[0]).toContain("per_page=100"); - expect(getCapturedUrls()[1]).toContain("per_page=50"); - }); - test("sends sort=-timestamp by default", async () => { mockOk({ data: [], meta: TX_META }); @@ -206,7 +184,7 @@ describe("listTransactions", () => { }, }, { - body: { data: makeTxnRows(50), meta: TX_META }, + body: { data: makeTxnRows(100), meta: TX_META }, headers: { Link: `; rel="next"; results="false"; cursor=""`, }, @@ -214,38 +192,14 @@ describe("listTransactions", () => { ]); const result = await listTransactions("my-org", "my-project", { - limit: 150, + limit: 200, }); - expect(result.data).toHaveLength(150); + expect(result.data).toHaveLength(200); expect(result.nextCursor).toBeUndefined(); expect(getCapturedUrls()).toHaveLength(2); }); - test("trims results and drops nextCursor when overshoot", async () => { - mockSequential([ - { - body: { data: makeTxnRows(100), meta: TX_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:100:0"`, - }, - }, - { - body: { data: makeTxnRows(100), meta: TX_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:200:0"`, - }, - }, - ]); - - const result = await listTransactions("my-org", "my-project", { - limit: 120, - }); - - expect(result.data).toHaveLength(120); - expect(result.nextCursor).toBeUndefined(); - }); - test("single-page fast path for limit <= 100", async () => { const { getCapturedUrls } = mockSequential([ { @@ -309,7 +263,7 @@ describe("listTransactions", () => { limit: 10, }); - expect(result.nextCursor).toBe(cursor); + expect(result.nextCursor).toContain(cursor); }); test("returns undefined nextCursor when results=false", async () => { @@ -427,28 +381,6 @@ describe("listSpans", () => { expect(capturedUrl).toContain("dataset=spans"); }); - test("uses the remaining item budget for the final page", async () => { - const { getCapturedUrls } = mockSequential([ - { - body: { data: makeSpanRows(100), meta: SPAN_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:100:0"`, - }, - }, - { - body: { data: makeSpanRows(50), meta: SPAN_META }, - headers: { - Link: `; rel="next"; results="false"; cursor=""`, - }, - }, - ]); - - await listSpans("my-org", "my-project", { limit: 150 }); - - expect(getCapturedUrls()[0]).toContain("per_page=100"); - expect(getCapturedUrls()[1]).toContain("per_page=50"); - }); - test("sends sort=-timestamp by default", async () => { mockOk({ data: [], meta: SPAN_META }); @@ -495,42 +427,20 @@ describe("listSpans", () => { }, }, { - body: { data: makeSpanRows(50), meta: SPAN_META }, + body: { data: makeSpanRows(100), meta: SPAN_META }, headers: { Link: `; rel="next"; results="false"; cursor=""`, }, }, ]); - const result = await listSpans("my-org", "my-project", { limit: 150 }); + const result = await listSpans("my-org", "my-project", { limit: 200 }); - expect(result.data).toHaveLength(150); + expect(result.data).toHaveLength(200); expect(result.nextCursor).toBeUndefined(); expect(getCapturedUrls()).toHaveLength(2); }); - test("trims results when overshoot", async () => { - mockSequential([ - { - body: { data: makeSpanRows(100), meta: SPAN_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:100:0"`, - }, - }, - { - body: { data: makeSpanRows(100), meta: SPAN_META }, - headers: { - Link: `; rel="next"; results="true"; cursor="0:200:0"`, - }, - }, - ]); - - const result = await listSpans("my-org", "my-project", { limit: 120 }); - - expect(result.data).toHaveLength(120); - expect(result.nextCursor).toBeUndefined(); - }); - test("single-page fast path for limit <= 100", async () => { const { getCapturedUrls } = mockSequential([ { @@ -639,7 +549,7 @@ describe("listSpans", () => { const result = await listSpans("my-org", "my-project", { limit: 10 }); - expect(result.nextCursor).toBe(cursor); + expect(result.nextCursor).toContain(cursor); }); test("returns undefined nextCursor when results=false", async () => { diff --git a/packages/cli/test/lib/auto-paginate.property.test.ts b/packages/cli/test/lib/auto-paginate.property.test.ts index df22c9f3b..c5e4995f3 100644 --- a/packages/cli/test/lib/auto-paginate.property.test.ts +++ b/packages/cli/test/lib/auto-paginate.property.test.ts @@ -5,9 +5,8 @@ * * - Fast path (`limit <= API_MAX_PER_PAGE`): returns a single page verbatim, * WITHOUT trimming — so a page that overshoots `limit` is returned as-is with - * its nextCursor. Callers whose endpoint can overshoot a single page (e.g. - * `listIssueEvents`, which has no per-page param) must trim-and-drop-cursor - * themselves; that behavior is covered by `events-overshoot.test.ts`. + * its nextCursor. Limit-bounded lists use `paginate()` instead, which trims + * and drops the cursor (see `paginate.property.test.ts`). * - Multi-page path (`limit > API_MAX_PER_PAGE`): accumulates across pages up to * `limit`. On true overshoot (a page pushes the total strictly past `limit`), * it trims to `limit` AND drops nextCursor, so cursor navigation never skips diff --git a/packages/cli/test/lib/org-list.test.ts b/packages/cli/test/lib/org-list.test.ts index e8a16b636..d74029db1 100644 --- a/packages/cli/test/lib/org-list.test.ts +++ b/packages/cli/test/lib/org-list.test.ts @@ -312,14 +312,15 @@ describe("handleOrgAll", () => { const result = await handleOrgAll({ config, org: "my-org", - flags: { limit: 10, json: true }, + flags: { limit: 1, json: true }, contextKey: "key", cursor: undefined, direction: "next", }); expect(result.hasMore).toBe(true); - expect(result.nextCursor).toBe("next:123"); + // paginate() wraps the server cursor with the walk's page size. + expect(result.nextCursor).toContain("next:123"); expect(result.items).toHaveLength(1); expect(result.items[0]!.orgSlug).toBe("my-org"); }); @@ -453,7 +454,7 @@ describe("handleOrgAll", () => { "test-list", "ctx", "next", - "cursor:abc", + expect.stringContaining("cursor:abc"), ); }); @@ -511,16 +512,32 @@ describe("handleOrgAll", () => { }); expect(result.items).toHaveLength(250); - expect(listPaginated).toHaveBeenCalledTimes(3); - // Each request uses only the remaining item budget, capped at API_MAX_PER_PAGE, - // so the final page requests only the remaining 50 items (250 - 100 - 100). + // Full pages keep requests to a minimum; the cursor carries the page size + // because team and repo lists use page-number cursors. expect(listPaginated.mock.calls.map((call) => call[1].perPage)).toEqual([ - 100, 100, 50, + 100, 100, 100, ]); // The mock always returns a nextCursor, so hasMore is true and // the cursor points to the first item beyond the fetched 250. expect(result.hasMore).toBe(true); - expect(result.nextCursor).toBe("250"); + + // `-c next` with another --limit keeps the walk's page size and refetches + // the third page, skipping the 50 rows already returned. + listPaginated.mockClear(); + const next = await handleOrgAll({ + config, + org: "my-org", + flags: { limit: 30, json: true }, + contextKey: "key", + cursor: result.nextCursor, + direction: "next", + }); + expect(next.items.map((item) => item.id)).toEqual( + Array.from({ length: 30 }, (_, i) => String(250 + i)), + ); + expect(listPaginated.mock.calls.map((call) => call[1])).toEqual([ + { cursor: "200", perPage: 100 }, + ]); }); }); diff --git a/packages/cli/test/lib/paginate.property.test.ts b/packages/cli/test/lib/paginate.property.test.ts new file mode 100644 index 000000000..d48253732 --- /dev/null +++ b/packages/cli/test/lib/paginate.property.test.ts @@ -0,0 +1,140 @@ +/** + * Property-based tests for paginate() against models of Sentry's paginators. + * + * Earlier pagination tests modeled every cursor as an absolute row offset, + * which hid a skip bug: Sentry's page-number paginators put the page size in + * the cursor, so changing `per_page` mid-walk reads the wrong rows. These + * models port the cursor math from `src/sentry/api/paginator.py`, and the + * properties walk `-c next` the way the CLI does — with a different + * `--limit` on every call — asserting that every row is returned exactly + * once, in order. + */ + +import { + array, + asyncProperty, + assert as fcAssert, + integer, + nat, +} from "fast-check"; +import { describe, expect, test } from "vitest"; +import { + API_MAX_PER_PAGE, + type PaginatedResponse, + paginate, +} from "../../src/lib/api/infrastructure.js"; +import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; + +type FetchPage = ( + perPage: number, + cursor: string | undefined, +) => Promise>; + +function parseCursor(cursor: string | undefined): [number, number] { + const [value = 0, offset = 0] = (cursor ?? "0:0:0").split(":").map(Number); + return [value, offset]; +} + +/** `GenericOffsetPaginator`: cursor offset is an absolute row offset. */ +function genericOffsetPaginator(total: number): FetchPage { + return (perPage, cursor) => { + const [, offset] = parseCursor(cursor); + const end = Math.min(offset + perPage, total); + const data = Array.from({ length: end - offset }, (_, i) => offset + i); + return Promise.resolve({ + data, + nextCursor: end < total ? `0:${end}:0` : undefined, + }); + }; +} + +/** + * `OffsetPaginator`: cursor is `::`. + * + * Port of `OffsetPaginator.get_result`, including the window it reads when + * the cursor's page size differs from the request's `per_page`. + */ +function offsetPaginator(total: number): FetchPage { + const rows = Array.from({ length: total }, (_, i) => i); + return (perPage, cursor) => { + const [value, page] = parseCursor(cursor); + const limit = Math.min(perPage, API_MAX_PER_PAGE); + const offset = page * value; + const stop = offset + (value || limit) + 1; + let results = rows.slice(offset, stop); + if (value !== limit) { + results = results.slice(-(limit + 1)); + } + return Promise.resolve({ + data: results.slice(0, limit), + nextCursor: results.length > limit ? `${limit}:${page + 1}:0` : undefined, + }); + }; +} + +/** A server that ignores `per_page` and always serves pages of 30 rows. */ +function fixedPageServer(total: number): FetchPage { + return (_perPage, cursor) => genericOffsetPaginator(total)(30, cursor); +} + +const paginators = [ + { name: "GenericOffsetPaginator", make: genericOffsetPaginator }, + { name: "OffsetPaginator", make: offsetPaginator }, + { name: "a server that ignores per_page", make: fixedPageServer }, +]; + +/** + * Follow `-c next` until the cursor runs out, like repeated CLI calls, using + * the next limit from `limits` (cycled) on each call. + */ +async function walk(fetchPage: FetchPage, total: number, limits: number[]) { + const seen: number[] = []; + let cursor: string | undefined; + // Each call returns at least one row while rows remain. + for (let call = 0; call <= total; call += 1) { + const limit = limits[call % limits.length] ?? 1; + const result = await paginate({ limit, cursor }, fetchPage); + // A resumed walk keeps its first page size, so a much larger later limit + // can stop at MAX_PAGINATION_PAGES; it must still make progress. + expect(result.data.length).toBeLessThanOrEqual(limit); + if (seen.length < total) { + expect(result.data.length).toBeGreaterThan(0); + } + seen.push(...result.data); + if (!result.nextCursor) { + return seen; + } + cursor = result.nextCursor; + } + throw new Error("walk did not finish"); +} + +const totalArb = nat(API_MAX_PER_PAGE * 5); +const limitsArb = array(integer({ min: 1, max: API_MAX_PER_PAGE * 4 }), { + minLength: 1, + maxLength: 4, +}); + +describe.each(paginators)("property: paginate over $name", ({ make }) => { + test("fills the limit on a fresh walk", async () => { + await fcAssert( + asyncProperty(totalArb, limitsArb, async (total, [limit = 1]) => { + const result = await paginate({ limit }, make(total)); + expect(result.data).toEqual( + Array.from({ length: Math.min(limit, total) }, (_, i) => i), + ); + }), + { numRuns: DEFAULT_NUM_RUNS }, + ); + }); + + test("returns every row exactly once, in order, across -c next", async () => { + await fcAssert( + asyncProperty(totalArb, limitsArb, async (total, limits) => { + const seen = await walk(make(total), total, limits); + expect(seen).toEqual(Array.from({ length: total }, (_, i) => i)); + }), + { numRuns: DEFAULT_NUM_RUNS }, + ); + }); +});