diff --git a/docs/errors.md b/docs/errors.md index fccab13..61b88ac 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -3,15 +3,16 @@ Every failure the SDK gives up on is typed, and which type tells you what you know about the request: -| Thrown | Means | -| ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| `BrewApiError` | The server answered with a non-2xx status. `status`, `code` and the rest of the envelope say what it said. | -| `BrewTimeoutError` (a `BrewTransportError`) | No complete answer within `timeoutMs` — waiting for the headers or reading the body. **Outcome unknown.** | -| `BrewConnectionError` (a `BrewTransportError`) | The connection failed, or dropped while the body streamed. **Outcome unknown.** The original error is on `cause`. | -| `BrewParseError` | The server answered 2xx, but the body was not JSON. Report it with `requestId`. | -| your signal's `reason` | You cancelled — the `signal` you passed (request- or client-level) aborted. | - -All four classes extend `Error`, so the two-branch pattern below — +| Thrown | Means | +| ---------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `BrewApiError` | The server answered with a non-2xx status. `status`, `code` and the rest of the envelope say what it said. | +| `BrewTimeoutError` (a `BrewTransportError`) | No complete answer within `timeoutMs` — waiting for the headers or reading the body. **Outcome unknown.** | +| `BrewConnectionError` (a `BrewTransportError`) | The connection failed, or dropped while the body streamed. **Outcome unknown.** The original error is on `cause`. | +| `BrewParseError` | The server answered 2xx, but the body was not JSON. Report it with `requestId`. | +| `BrewPaginationError` | A list endpoint repeated a cursor. The iterator stops with `code: 'PAGINATION_CURSOR_CYCLE'`, `cursor`, `pagesFetched`, and `itemsYielded`. Already-yielded items are incomplete; do not treat them as a complete list. | +| your signal's `reason` | You cancelled — the `signal` you passed (request- or client-level) aborted. | + +All SDK error classes extend `Error`, so the two-branch pattern below — `BrewApiError` for API errors, `Error` for everything else — covers every case. @@ -338,3 +339,29 @@ interrupted write without doing it twice, replay it with the same idempotency key. A cancel rejects with your own `reason`, which carries no key, so on a write you might cancel, pass your own `RequestOptions.idempotencyKey` and keep it. + +## Pagination protocol failures + +`autoPaginate` rejects an identical or cyclic next cursor before fetching a +known page again. Empty pages with an advancing cursor continue normally; +`hasMore: false` or a missing cursor ends iteration. Its documented cancellation +behavior is unchanged: an abort between pages ends iteration, while an abort +from an in-flight request propagates that request's signal reason. + +```ts +import { BrewPaginationError } from '@brew.new/sdk' + +try { + for await (const contact of brew.contacts.searchAll()) { + // Consume each contact. + } +} catch (error) { + if (error instanceof BrewPaginationError) { + console.error(`Incomplete list after ${error.itemsYielded} items`) + } + throw error +} +``` + +The repeated cursor is diagnostic evidence, not a safe resume cursor. Report +the failure instead of automatically retrying the same cursor. diff --git a/src/core/pagination.ts b/src/core/pagination.ts index 9a97bec..cfd3453 100644 --- a/src/core/pagination.ts +++ b/src/core/pagination.ts @@ -40,6 +40,26 @@ export type Page = { readonly pagination: Pagination | undefined } +/** The server returned a cursor already fetched by this iterator. */ +export class BrewPaginationError extends Error { + readonly code = 'PAGINATION_CURSOR_CYCLE' + readonly cursor: string + readonly pagesFetched: number + readonly itemsYielded: number + + constructor(input: { + readonly cursor: string + readonly pagesFetched: number + readonly itemsYielded: number + }) { + super('Pagination returned a previously fetched cursor.') + this.name = 'BrewPaginationError' + this.cursor = input.cursor + this.pagesFetched = input.pagesFetched + this.itemsYielded = input.itemsYielded + } +} + /** * Generic async-iterator that walks every page of a cursor-paginated * list, yielding one row at a time. Resources build a thin `listAll` @@ -67,21 +87,44 @@ export async function* autoPaginate( * before its cursor is known, so the next fetch cannot start early. */ let cursor: string | null = null + const fetched = new Set() + let pagesFetched = 0 + let itemsYielded = 0 + const isAborted = () => options?.signal?.aborted === true while (true) { - if (options?.signal?.aborted === true) { + if (isAborted()) { return } const page = await fetchPage(cursor) + pagesFetched += 1 + if (cursor !== null) { + fetched.add(cursor) + } for (const item of page.items) { + itemsYielded += 1 yield item } + // A caller may abort while consuming the page. Preserve quiet termination + // between pages even if the server also returned a cyclic next cursor. + if (isAborted()) { + return + } + const next = page.pagination if (!next || !next.hasMore || next.cursor === null) { return } + if (fetched.has(next.cursor)) { + throw new BrewPaginationError({ + cursor: next.cursor, + pagesFetched, + itemsYielded, + }) + } + cursor = next.cursor } /* eslint-enable no-await-in-loop */ diff --git a/src/index.ts b/src/index.ts index f1de7c1..709c1d7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,7 +22,7 @@ export { } from './core/errors' export type { Pagination, PaginationInput } from './core/pagination' // ---------- Pagination ---------- -export { autoPaginate } from './core/pagination' +export { autoPaginate, BrewPaginationError } from './core/pagination' // Recursive JSON payload value for `emails.send` / `automations.triggers.fire` // (replaces the removed `TransactionalPayloadValue`). export type { SendPayloadValue } from './generated/openapi-types' diff --git a/src/resources/contacts/search-all.ts b/src/resources/contacts/search-all.ts index 84e4913..73ab1d8 100644 --- a/src/resources/contacts/search-all.ts +++ b/src/resources/contacts/search-all.ts @@ -38,9 +38,8 @@ export type SearchAllContactsInput = Readonly< * `search` are still yielded by the loop, but no further fetches are * issued. * - * This is the filtered counterpart to `brew.contacts.listAll`: filtering - * moved off the list query onto the search endpoint, so any iteration - * that needs filters/search/sort goes through here. + * Filtering lives on the search endpoint, so iteration that needs + * filters/search/sort goes through here. * * Example: * ```ts diff --git a/tests/core/pagination.test.ts b/tests/core/pagination.test.ts index 0c7ab45..b1f8da5 100644 --- a/tests/core/pagination.test.ts +++ b/tests/core/pagination.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { BrewPaginationError } from '../../src' import { autoPaginate, @@ -7,6 +8,66 @@ import { } from '../../src/core/pagination' describe('autoPaginate', () => { + it.each([ + { cursors: ['a', 'a'], calls: [null, 'a'], itemsYielded: 2 }, + { cursors: ['a', 'b', 'a'], calls: [null, 'a', 'b'], itemsYielded: 3 }, + ])( + 'rejects a cursor cycle before refetching a known page', + async ({ cursors, calls, itemsYielded }) => { + const requested: Array = [] + const yielded: Array = [] + const drain = async () => { + for await (const item of autoPaginate((cursor) => { + requested.push(cursor) + // Bound the old implementation so the regression fails without hanging. + if (requested.length > cursors.length) { + return Promise.reject(new Error('refetched a known page')) + } + return Promise.resolve({ + items: [requested.length], + pagination: { + limit: 1, + cursor: cursors[requested.length - 1]!, + hasMore: true, + }, + }) + })) { + yielded.push(item) + } + } + const error = await drain().catch((caught: unknown) => caught) + expect(error).toBeInstanceOf(BrewPaginationError) + expect(error).toMatchObject({ + code: 'PAGINATION_CURSOR_CYCLE', + cursor: 'a', + pagesFetched: calls.length, + itemsYielded, + }) + expect(requested).toEqual(calls) + expect(yielded.length).toBe(itemsYielded) + } + ) + + it('advances through an empty page and accepts a terminal cursor that was seen earlier', async () => { + const cursors: Array = [] + const output: Array = [] + for await (const item of autoPaginate((cursor) => { + cursors.push(cursor) + return Promise.resolve( + cursor === null + ? { items: [], pagination: { limit: 1, cursor: 'a', hasMore: true } } + : { + items: [7], + pagination: { limit: 1, cursor: 'a', hasMore: false }, + } + ) + })) { + output.push(item) + } + expect(output).toEqual([7]) + expect(cursors).toEqual([null, 'a']) + }) + it('yields every row across pages, following the cursor', async () => { const pages: Record> = { first: { @@ -61,6 +122,31 @@ describe('autoPaginate', () => { expect(calls).toBe(1) expect(out).toEqual([1]) }) + + it('finishes the fetched page and stops quietly when an abort coincides with a cursor cycle', async () => { + const controller = new AbortController() + const requested: Array = [] + const output: Array = [] + + for await (const item of autoPaginate( + (cursor) => { + requested.push(cursor) + return Promise.resolve({ + items: cursor === null ? [1] : [2, 3], + pagination: { limit: 2, cursor: 'a', hasMore: true }, + }) + }, + { signal: controller.signal } + )) { + output.push(item) + if (item === 2) { + controller.abort() + } + } + + expect(requested).toEqual([null, 'a']) + expect(output).toEqual([1, 2, 3]) + }) }) describe('pageReadOptions', () => {