Skip to content

Commit f10e4d1

Browse files
yaaayaaaclaude
andauthored
fix: stop dropping analytics instances larger than ~100k rows (#87)
`downloadInstanceRows` appended parsed rows with `rows.push(...parseTsv(tsv))`. The spread passes every row as a separate argument, so once an instance holds roughly 100k+ rows the call exceeds the engine's argument-count limit and throws "RangeError: Maximum call stack size exceeded" (V8 reports the argument-count limit under that message; it is not stack recursion). The throw is caught by the `Promise.allSettled` handler in `fetchReportData`, which logs "instance download failed (data may be incomplete)" and skips the instance. Sync still reports success, so the report silently loses that instance's history. ONE_TIME_SNAPSHOT instances carry an app's whole history in one segment and routinely exceed the limit. On a real account this dropped every historical instance for the high-cardinality reports while the low-volume ones were unaffected: App Downloads 10 dates -> 636 dates (343,904 rows) Discovery and Engagement 11 dates -> 637 dates (398,161 rows) Installation and Deletion 13 dates -> 634 dates (210,623 rows) Sessions 636 dates -> 636 dates (unchanged, 85,039 rows) Replace both spreads with a `pushAll` helper that appends element by element. The same pattern was used when appending a single date's rows to the deduped array, so that call site is changed too. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 790d5f7 commit f10e4d1

4 files changed

Lines changed: 111 additions & 2 deletions

File tree

src/lib/asc/analytics-reports.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
PERF_METRICS_TTL,
99
findDateGaps,
1010
parseTsv,
11+
pushAll,
1112
type AscReportRequest,
1213
type AscReport,
1314
type AscReportInstance,
@@ -346,7 +347,9 @@ async function downloadInstanceRows(
346347
const rows: Array<Record<string, string>> = [];
347348
for (const seg of segResp.data) {
348349
const tsv = await downloadSegment(seg.attributes.url);
349-
rows.push(...parseTsv(tsv));
350+
// pushAll, not push(...): snapshot instances can hold 300k+ rows, which
351+
// exceeds the engine's argument-count limit when spread into push().
352+
pushAll(rows, parseTsv(tsv));
350353
}
351354
return rows;
352355
}
@@ -498,7 +501,9 @@ export async function fetchReportData(
498501
for (const [date, rows] of rowsByDate) {
499502
if (!seenDataDates.has(date)) {
500503
seenDataDates.add(date);
501-
deduped.push(...rows);
504+
// Same argument-count limit as above – a single date can hold enough
505+
// rows to exceed it once territory/source/version dimensions multiply.
506+
pushAll(deduped, rows);
502507
}
503508
}
504509
}

src/lib/asc/analytics-types.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,20 @@ export function parseTsv(raw: string): Array<Record<string, string>> {
126126
});
127127
}
128128

129+
/**
130+
* Append every element of `source` to `target`.
131+
*
132+
* Use this instead of `target.push(...source)`: the spread passes each element
133+
* as a separate argument, so it exceeds the engine's argument-count limit once
134+
* `source` holds roughly 100k+ elements. V8 reports that as
135+
* "RangeError: Maximum call stack size exceeded", which is misleading – it is
136+
* an argument-count limit, not stack recursion. Analytics snapshot instances
137+
* routinely exceed it (a single App Downloads instance can hold 300k+ rows).
138+
*/
139+
export function pushAll<T>(target: T[], source: readonly T[]): void {
140+
for (const item of source) target.push(item);
141+
}
142+
129143
// ---------- Helpers ----------
130144

131145
export function emptyAnalyticsData(): AnalyticsData {
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { describe, it, expect } from "vitest";
2+
import { pushAll } from "@/lib/asc/analytics-types";
3+
4+
describe("pushAll", () => {
5+
it("appends every element, preserving order", () => {
6+
const target = [1, 2];
7+
pushAll(target, [3, 4, 5]);
8+
expect(target).toEqual([1, 2, 3, 4, 5]);
9+
});
10+
11+
it("is a no-op for an empty source", () => {
12+
const target = [1];
13+
pushAll(target, []);
14+
expect(target).toEqual([1]);
15+
});
16+
17+
it("handles sources far beyond the spread argument-count limit", () => {
18+
// `target.push(...source)` throws "RangeError: Maximum call stack size
19+
// exceeded" somewhere above ~100k elements because each element becomes a
20+
// separate argument. Analytics snapshot instances routinely exceed that.
21+
const source = Array.from({ length: 200_000 }, (_, i) => i);
22+
const target: number[] = [];
23+
24+
pushAll(target, source);
25+
26+
expect(target).toHaveLength(200_000);
27+
expect(target[0]).toBe(0);
28+
expect(target[199_999]).toBe(199_999);
29+
});
30+
});

tests/unit/asc/analytics.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4119,3 +4119,63 @@ describe("buildAnalyticsData – accumulation across refreshes", () => {
41194119
expect(consoleLogSpy).not.toHaveBeenCalledWith(expect.stringContaining("Backfill app-nogap"));
41204120
});
41214121
});
4122+
4123+
describe("large snapshot instances", () => {
4124+
it("ingests an instance whose row count exceeds the spread argument limit", async () => {
4125+
mockCacheGet.mockReturnValue(null);
4126+
// The console spies are shared across the file and keep earlier tests'
4127+
// calls, so clear before asserting on what this test logs.
4128+
consoleWarnSpy.mockClear();
4129+
4130+
// A ONE_TIME_SNAPSHOT instance carries the app's whole history in one
4131+
// segment. Real accounts reach 300k+ rows; anything above roughly 100k
4132+
// used to be dropped by `rows.push(...parseTsv(tsv))` with
4133+
// "RangeError: Maximum call stack size exceeded", and the failure was only
4134+
// logged as a warning – the report silently lost that instance's history.
4135+
//
4136+
// Every row shares one data date, so the second call site (appending one
4137+
// date's rows to the deduped array) is exercised too — though only on the
4138+
// fixed code, since on main the first spread throws before reaching it.
4139+
const ROW_COUNT = 150_000;
4140+
const rows = Array.from({ length: ROW_COUNT }, () => [
4141+
"2026-02-01",
4142+
"First-time download",
4143+
"1",
4144+
]);
4145+
const hugeTsv = tsvString(["Date", "Download Type", "Counts"], rows);
4146+
4147+
mockAscFetch.mockImplementation(async (url: string) => {
4148+
if (url.includes("/analyticsReportRequests") && !url.includes("/reports")) {
4149+
return reportRequestsResponse(["req-huge"]);
4150+
}
4151+
if (url.includes("/reports?filter")) {
4152+
return reportsResponse([
4153+
{ id: "rpt-huge", name: "App Downloads Standard", category: "COMMERCE" },
4154+
]);
4155+
}
4156+
if (url.includes("/instances?")) {
4157+
return instancesResponse([{ id: "inst-huge", processingDate: "2026-02-02" }]);
4158+
}
4159+
if (url.includes("/segments")) {
4160+
return segmentsResponse([{ id: "seg-huge", url: "https://s3.example.com/huge.tsv" }]);
4161+
}
4162+
return { data: [] };
4163+
});
4164+
mockFetch.mockResolvedValue(makeFetchResponse(hugeTsv));
4165+
4166+
const result = await buildAnalyticsData("app-huge-instance");
4167+
4168+
// Every row survives: no instance was skipped.
4169+
expect(result.dailyDownloads).toHaveLength(1);
4170+
expect(result.dailyDownloads[0]).toEqual({
4171+
date: "2026-02-01",
4172+
firstTime: ROW_COUNT,
4173+
redownload: 0,
4174+
update: 0,
4175+
});
4176+
expect(consoleWarnSpy).not.toHaveBeenCalledWith(
4177+
expect.stringContaining("instance download failed"),
4178+
expect.anything(),
4179+
);
4180+
});
4181+
});

0 commit comments

Comments
 (0)