Skip to content

fix(appfigures): stop gracefully at the reviews page-depth cap - #72489

Merged
Gilbert09 merged 2 commits into
masterfrom
posthog-code/fix-appfigures-review-page-limit
Jul 21, 2026
Merged

fix(appfigures): stop gracefully at the reviews page-depth cap#72489
Gilbert09 merged 2 commits into
masterfrom
posthog-code/fix-appfigures-review-page-limit

Conversation

@Gilbert09

Copy link
Copy Markdown
Member

Problem

Error tracking surfaced a fatal HTTPError from the Appfigures reviews sync (issue):

400 Client Error: page * count must be less than or equal to 10000. Please
request fewer products or a smaller date range. for url: .../reviews?count=500&page=21&...

Appfigures' /reviews endpoint caps offset pagination at page * count <= 10000. Once a sync's backlog is deep enough to cross that boundary (e.g. a large historical backfill), our pagination loop keeps incrementing page past the cap and the resulting 400 is treated as a fatal error, failing the whole sync.

Changes

This is an upstream API limit, not something we can request around, so I applied the same pattern already used for GitHub's pagination-cap 403 (see #68292): detect the benign cap response and stop paginating gracefully, keeping the rows already gathered, instead of letting it raise via raise_for_status.

  • _is_page_limit_response detects the stable "must be less than or equal to 10000" message (surfaced via the HTTP reason phrase, with a body-text fallback).
  • _fetch raises a new AppfiguresPageLimitError control-flow signal when it sees one, before the generic raise_for_status fallback.
  • _iter_paged catches it and breaks the loop, keeping whatever pages were already yielded.

Reviews sync incrementally off the date watermark, so on the next scheduled run the source resumes just past the last row it successfully synced, effectively walking a very large backlog forward in capped chunks across multiple syncs rather than failing outright.

How did you test this code?

Added tests in test_appfigures.py:

  • TestIsPageLimitResponse — parameterized detector cases, including that a different 400 (or the same message on a different status) is not treated as the cap, so real errors stay fatal.
  • test_page_limit_stops_and_keeps_rows_gathered_so_far — end-to-end through get_rows/_iter_paged: page 1 succeeds, page 2 hits the cap, and the rows gathered from page 1 are yielded without raising.

Ran the full test_appfigures.py suite (36 tests, all passing) plus ruff check/ruff format and a repo-wide mypy check (no new errors).

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

N/A — internal pagination/error-handling fix, no user-facing behavior or documented workflow changed.

🤖 Agent context

Autonomy: Fully autonomous

Authored by Claude Code (Sonnet 5) from an error-tracking alert for the Appfigures reviews sync. I confirmed the stack trace originates in products/warehouse_sources/backend/temporal/data_imports/sources/appfigures/appfigures.py, read the pagination logic in _iter_paged, and classified this as a fixable bug (our pagination doesn't respect the API's documented depth cap) rather than a user/credentials error, since nothing about the customer's config caused it.

I found PR #68292 (GitHub source's pagination-cap 403 fix) as a close precedent and mirrored its shape — a detector function, a control-flow exception, and a graceful loop break — rather than inventing a new mechanism. I invoked the /writing-tests skill before adding tests.

Appfigures caps offset pagination on /reviews at page*count <= 10000. Once
a sync's backlog crosses that depth it returns HTTP 400 ("page * count must
be less than or equal to 10000"), which we let bubble up as a fatal
HTTPError and killed the whole sync.

Mirrors the existing pattern in the GitHub source for its pagination-cap
403: detect the benign cap response and stop paginating, keeping the rows
already gathered, instead of raising. Because reviews sync incrementally
off the `date` watermark, the next scheduled sync picks up where this one
stopped.

Generated-By: PostHog Code
Task-Id: 27f1c31a-731e-446a-8085-85305f54b168
@github-actions

Copy link
Copy Markdown
Contributor

Hey @Gilbert09! 👋

It looks like your git author email on this PR isn't your @posthog.com address (owerstom@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 21, 2026 06:59
Generated-By: PostHog Code
Task-Id: 84af0109-67b4-481d-82b5-13e6bafa6bcc
@Gilbert09
Gilbert09 merged commit 0d4bd46 into master Jul 21, 2026
312 of 314 checks passed
@Gilbert09
Gilbert09 deleted the posthog-code/fix-appfigures-review-page-limit branch July 21, 2026 16:42
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 21, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-21 17:31 UTC Run
prod-us ✅ Deployed 2026-07-21 17:52 UTC Run
prod-eu ✅ Deployed 2026-07-21 17:50 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants