Skip to content

Commit cfb436e

Browse files
jar-stripeclaude
andauthored
Fix V2ListIterator: concurrency guard and empty page handling (#2747)
Fix V2ListIterator: add concurrency guard and handle empty pages V2ListIterator had two bugs: 1. No concurrency guard — parallel .next() calls could independently trigger page fetches, producing duplicate or out-of-order results. 2. Empty intermediate pages terminated iteration — if the API returned a page with data:[] but a valid next_page_url, iteration stopped instead of continuing to the next page. Committed-By-Agent: claude Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8056da8 commit cfb436e

2 files changed

Lines changed: 63 additions & 2 deletions

File tree

src/autoPagination.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ class V2ListIterator<T> implements AsyncIterator<T> {
173173
private firstPagePromise: Promise<PageResult<T>> | null;
174174
private currentPageIterator: Iterator<T> | null;
175175
private nextPageUrl: string | null;
176+
private promiseCache: PromiseCache;
176177
private options: RequestOptions | undefined;
177178
private spec: MakeRequestSpec | undefined;
178179
private stripeResource: StripeResourceObject;
@@ -185,6 +186,7 @@ class V2ListIterator<T> implements AsyncIterator<T> {
185186
this.firstPagePromise = firstPagePromise;
186187
this.currentPageIterator = null;
187188
this.nextPageUrl = null;
189+
this.promiseCache = {currentPromise: null};
188190
this.options = options;
189191
this.spec = spec;
190192
this.stripeResource = stripeResource;
@@ -210,19 +212,45 @@ class V2ListIterator<T> implements AsyncIterator<T> {
210212
this.currentPageIterator = page.data[Symbol.iterator]();
211213
return this.currentPageIterator;
212214
}
213-
async next(): Promise<IteratorResult<T>> {
215+
private async _next(): Promise<IteratorResult<T>> {
214216
await this.initFirstPage();
215217
if (this.currentPageIterator) {
216218
const result = this.currentPageIterator.next();
217219
if (!result.done) return {done: false, value: result.value};
218220
}
221+
return this.nextFromNewPage();
222+
}
223+
private async nextFromNewPage(): Promise<IteratorResult<T>> {
219224
const nextPageIterator = await this.turnPage();
220225
if (!nextPageIterator) {
221226
return {done: true, value: undefined};
222227
}
223228
const result = nextPageIterator.next();
224229
if (!result.done) return {done: false, value: result.value};
225-
return {done: true, value: undefined};
230+
// Empty intermediate page — recurse to try the next page.
231+
return this.nextFromNewPage();
232+
}
233+
next(): Promise<IteratorResult<T>> {
234+
/**
235+
* If a user calls `.next()` multiple times in parallel,
236+
* return the same result until something has resolved
237+
* to prevent page-turning race conditions.
238+
*/
239+
if (this.promiseCache.currentPromise) {
240+
return this.promiseCache.currentPromise;
241+
}
242+
243+
const nextPromise = (async (): Promise<IteratorResult<T>> => {
244+
try {
245+
return await this._next();
246+
} finally {
247+
this.promiseCache.currentPromise = null;
248+
}
249+
})();
250+
251+
this.promiseCache.currentPromise = nextPromise;
252+
253+
return nextPromise;
226254
}
227255
}
228256

test/autoPagination.spec.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,39 @@ describe('auto pagination', () => {
879879
'Should not have any unhandled promise rejections'
880880
).to.have.length(0);
881881
});
882+
883+
it('iterates through empty intermediate pages', () => {
884+
return testCaseV2List({
885+
pages: [
886+
{ids: [1, 2], next_page_url: '/v2/items?page=a'},
887+
{ids: [], next_page_url: '/v2/items?page=b'},
888+
{ids: [], next_page_url: '/v2/items?page=c'},
889+
{ids: [3, 4]},
890+
],
891+
limit: 10,
892+
expectedIds: [1, 2, 3, 4],
893+
expectedParamsLog: ['?page=a', '?page=b', '?page=c'],
894+
});
895+
});
896+
897+
it('coalesces parallel next() calls at a page boundary into one request', async () => {
898+
const {paginator, paramsLog} = mockPaginationV2List(
899+
[{ids: [1], next_page_url: '/v2/items?page=foo'}, {ids: [2, 3]}],
900+
{}
901+
);
902+
903+
// Drain the first page
904+
const first = await paginator.next();
905+
expect((first as any).value.id).to.equal(1);
906+
907+
// Now at page boundary — fire parallel next() calls
908+
const [r1, r2] = await Promise.all([paginator.next(), paginator.next()]);
909+
expect(r1).to.deep.equal(r2);
910+
expect((r1 as any).value.id).to.equal(2);
911+
912+
// Only one page request should have been made
913+
expect(paramsLog).to.deep.equal(['?page=foo']);
914+
});
882915
});
883916
describe('stack traces', () => {
884917
// Builds a paginator whose first page succeeds but whose second page

0 commit comments

Comments
 (0)