Skip to content

Commit 87a2cd4

Browse files
committed
fix(poll): removes ETag caching and fixes event pagination
ETag removal: GitHub's server-side cache returns stale 304s for 10+ minutes, preventing background change detection. Removes If-None-Match headers and relies on _lastEventId numeric comparison against full 200 responses instead (60 req/hr, 1.2% of rate limit budget). _lastEventId fix: uses max ID via reduce instead of assuming allEvents[0] is newest, preventing redundant targeted refreshes when events arrive out of order. Pagination: fetches up to 3 pages when page 1 is full, with early-exit optimization that skips deeper pages when the current page contains events at or below the _lastEventId threshold (events are newest-first). Adds 7 pagination tests covering full-page trigger, partial-page stop, 3-page cap, early-exit, straddling threshold, error handling, and empty-page termination.
1 parent 7fdc8a8 commit 87a2cd4

3 files changed

Lines changed: 232 additions & 71 deletions

File tree

src/app/services/events.ts

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,13 @@ export const ACTIONABLE_EVENT_TYPES = [
3232
"PushEvent",
3333
] as const;
3434

35-
// ── Module-level ETag state ───────────────────────────────────────────────────
35+
// ── Module-level state ───────────────────────────────────────────────────────
3636

37-
let _eventsETag: string | null = null;
3837
let _lastEventId: string | null = null;
3938

4039
// ── Auth cleanup ──────────────────────────────────────────────────────────────
4140

4241
export function resetEventsState(): void {
43-
_eventsETag = null;
4442
_lastEventId = null;
4543
}
4644

@@ -60,58 +58,76 @@ export async function fetchUserEvents(
6058
return { events: [], changed: false };
6159
}
6260

63-
const headers: Record<string, string> = {};
64-
if (_eventsETag) {
65-
headers["If-None-Match"] = _eventsETag;
66-
}
67-
6861
try {
62+
// GitHub docs suggest per_page is capped at 30 for Events API, but empirical
63+
// testing (2026-05-03) confirmed per_page: 100 returns 100 events successfully.
6964
const response = await octokit.request("GET /users/{username}/events", {
7065
username,
7166
per_page: 100,
72-
headers,
7367
});
7468

75-
// Store ETag for next conditional request
76-
const etag = (response.headers as Record<string, string>)["etag"];
77-
if (etag) {
78-
_eventsETag = etag;
69+
let allEvents = (response.data as GitHubEvent[]);
70+
71+
// Paginate if page is full — older events on subsequent pages would be
72+
// permanently missed since _lastEventId advances past them.
73+
let page = 2;
74+
let lastPageEvents = allEvents;
75+
while (lastPageEvents.length === 100 && page <= 3) {
76+
if (_lastEventId !== null) {
77+
const threshold = parseInt(_lastEventId, 10);
78+
if (lastPageEvents.some((e) => parseInt(e.id, 10) <= threshold)) break;
79+
}
80+
try {
81+
const next = await octokit.request("GET /users/{username}/events", {
82+
username,
83+
per_page: 100,
84+
page,
85+
});
86+
const nextEvents = (next.data as GitHubEvent[]);
87+
if (nextEvents.length === 0) break;
88+
allEvents = [...allEvents, ...nextEvents];
89+
lastPageEvents = nextEvents;
90+
page++;
91+
} catch (err) {
92+
console.warn(`[events] pagination error on page ${page}:`, err instanceof Error ? err.message : String(err));
93+
break;
94+
}
7995
}
8096

81-
const allEvents = (response.data as GitHubEvent[]);
97+
const maxId = allEvents.reduce(
98+
(max, e) => Math.max(max, parseInt(e.id, 10)),
99+
0,
100+
);
82101

83-
// First call: no ID filter — seed _lastEventId and return all events
102+
// First call: seed _lastEventId and return all events
84103
if (_lastEventId === null) {
85-
if (allEvents.length > 0) {
86-
_lastEventId = allEvents[0].id; // events are newest-first
104+
if (maxId > 0) {
105+
_lastEventId = String(maxId);
87106
}
88107
return { events: allEvents, changed: allEvents.length > 0 };
89108
}
90109

91110
// Subsequent calls: filter to only events newer than _lastEventId
92-
// Use numeric comparison — event IDs are numeric strings; lexicographic
93-
// comparison would break for IDs of different lengths (e.g. "9" > "10").
94111
const lastIdNum = parseInt(_lastEventId, 10);
95112
const newEvents = allEvents.filter(
96113
(e) => parseInt(e.id, 10) > lastIdNum,
97114
);
98115

99-
if (newEvents.length > 0) {
100-
_lastEventId = allEvents[0].id; // newest event is always first
116+
if (maxId > lastIdNum) {
117+
_lastEventId = String(maxId);
101118
}
102119

103120
return { events: newEvents, changed: newEvents.length > 0 };
104121
} catch (err) {
105-
// Octokit throws RequestError on 304 — same pattern as hasNotificationChanges()
106-
if (
107-
typeof err === "object" &&
108-
err !== null &&
109-
(err as { status?: number }).status === 304
110-
) {
111-
return { events: [], changed: false };
122+
const status =
123+
err && typeof err === "object" && "status" in err
124+
? (err as { status: number }).status
125+
: null;
126+
if (status === 304) {
127+
console.warn("[events] unexpected 304 from proxy/CDN; events suppressed this cycle");
128+
} else {
129+
console.warn("[events] fetchUserEvents error:", err instanceof Error ? err.message : String(err));
112130
}
113-
// Silent fallback for all other errors — full refresh handles reconciliation
114-
console.warn("[events] fetchUserEvents error:", err instanceof Error ? err.message : String(err));
115131
return { events: [], changed: false };
116132
}
117133
}

src/app/services/poll.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,9 @@ export function createEventsPollCoordinator(
866866
schedule(myGeneration, EVENTS_POLL_INTERVAL_MS);
867867
}
868868

869-
// First cycle fires immediately (delay=0) to establish ETag baseline
869+
// First cycle fires immediately (delay=0) to seed _lastEventId baseline.
870+
// May trigger a targeted refresh if tracked repos have recent events — the
871+
// isFullRefreshing() guard prevents doubling up with the initial full poll.
870872
const gen = chainGeneration;
871873
timeoutId = setTimeout(() => void cycle(gen), 0);
872874

0 commit comments

Comments
 (0)