Skip to content

fix: paginate listEntries with scroll #3384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hellofanny merged 1 commit into from
Jun 11, 2026
Merged

fix: paginate listEntries with scroll #3384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"@parcel/watcher": "^2.4.0",
"@types/react": "^18.2.42",
"@vtex/client-cms": "^0.2.16",
"@vtex/client-cp": "0.3.5",
"@vtex/client-cp": "0.6.0",
"@vtex/prettier-config": "1.0.0",
"autoprefixer": "^10.4.0",
"cookie": "^0.7.0",
Expand Down
35 changes: 33 additions & 2 deletions packages/core/src/server/content/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,29 @@ export class ContentService {

if (isContentPlatformSource()) {
const serviceParams = this.convertOptionsToParams(options)
const { entries } = await this.clientCP.listEntries(serviceParams)
return this.fillEntriesWithData(entries, serviceParams, options.isPreview)
// TODO: This should be temporary until the cp data-plane have a search engine.
const MAX_PAGES = 25
const allEntries: ContentEntry[] = []
let scroll: string | undefined
let pages = 0

do {
const result = await this.clientCP.listEntries(serviceParams, scroll)
allEntries.push(...result.entries)
const next = result.scroll ?? undefined
if (next && next === scroll) break
scroll = next
} while (scroll && ++pages < MAX_PAGES)

if (pages >= MAX_PAGES) {
console.warn('listEntries pagination hit MAX_PAGES cap', serviceParams)
}
Comment on lines +61 to +65

@coderabbitai coderabbitai Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't continue with truncated results after the page cap.

If this loop exits with scroll still set, allEntries is incomplete but the code still runs template selection on it. That recreates the original bug for large CP datasets, just at a higher threshold, and can pick the wrong PLP/PDP content.

Suggested change 
-      if (pages >= MAX_PAGES) {
-        console.warn('listEntries pagination hit MAX_PAGES cap', serviceParams)
-      }
+      if (scroll) {
+        throw new Error(
+          `listEntries pagination exceeded MAX_PAGES (${MAX_PAGES})`
+        )
+      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} while (scroll && ++pages < MAX_PAGES)
if (pages >= MAX_PAGES) {
console.warn('listEntries pagination hit MAX_PAGES cap', serviceParams)
}
} while (scroll && ++pages < MAX_PAGES)
if (scroll) {
throw new Error(
`listEntries pagination exceeded MAX_PAGES (${MAX_PAGES})`
)
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/server/content/service.ts` around lines 61 - 65, The loop
in listEntries can exit due to hitting MAX_PAGES while scroll is still truthy,
leaving allEntries incomplete and then continuing to template selection; modify
listEntries so that if pages >= MAX_PAGES and scroll is still set you do not
proceed with downstream selection: detect this condition (pages >= MAX_PAGES &&
scroll) after the loop and either (a) abort/return a clear error or
truncated-result indicator to the caller (so template selection is skipped) or
(b) explicitly clear/complete the scroll and fetch remaining pages before
proceeding; reference allEntries, scroll, MAX_PAGES, listEntries and
serviceParams when making this change so template selection code only runs on a
complete result set.


return this.fillEntriesWithData(
allEntries,
serviceParams,
options.isPreview
)
Comment on lines +67 to +71

@coderabbitai coderabbitai Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Bound the CP enrichment fan-out before passing all pages downstream.

This now turns a single request into pages × entriesPerPage parallel getEntryData() calls through fillEntriesWithData(). On large catalogs that can blow the request budget or throttle CP hard. Please batch page enrichment or add a concurrency limit instead of sending the whole accumulated list at once.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/server/content/service.ts` around lines 67 - 71, The code
passes the entire accumulated allEntries list into fillEntriesWithData which
causes pages × entriesPerPage concurrent getEntryData() calls; instead, limit
concurrency by processing entries in bounded batches or adding a concurrency
limiter. Change the call-site or fillEntriesWithData signature to accept a
batchSize or concurrency option (e.g., batchSize/concurrency param) and then
iterate over allEntries in chunks (or use a p-limit style worker pool) so only N
getEntryData() calls run in parallel; reference the allEntries variable,
fillEntriesWithData(), getEntryData(), and serviceParams/options.isPreview when
implementing batching or a concurrency limiter. Ensure the final result order is
preserved by collecting batch results in sequence and returning the same
aggregated array.

}
return getCMSPage(options.cmsOptions)
}
Expand All @@ -60,6 +81,11 @@ export class ContentService {
const options = this.createContentOptions(plpParams)

if (isContentPlatformSource()) {
const directMatch = await this.getSingleContent<PLPContentType>(plpParams)
if (directMatch) {
return directMatch
}

const pages = (await this.getMultipleContent(plpParams)).data
if (!pages?.length) throw new MissingContentError(options.cmsOptions)
return findBestPLPTemplate(
Expand All @@ -79,6 +105,11 @@ export class ContentService {
const options = this.createContentOptions(pdpParams)

if (isContentPlatformSource()) {
const directMatch = await this.getSingleContent<PDPContentType>(pdpParams)
if (directMatch) {
return directMatch
}

const pages = (await this.getMultipleContent(pdpParams)).data
if (!pages.length) throw new MissingContentError(options.cmsOptions)
return findBestPDPTemplate(
Expand Down
10 changes: 5 additions & 5 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading