Skip to content

Commit 5ca5c61

Browse files
garethxclaude
andcommitted
feat: make catch-up a guarantee rather than a hope
Three separate things stood between "a replay was requested" and "every missed request was recovered". Each was measured against a live project rather than reasoned about, and each was wrong. The filter matched only half the cases. The two disconnect regimes look different, and only one was covered: tunnel never existed events_count 0, ignored_count 1 tunnel connected, then killed events_count 0, ignored_count 0 The second is the hard crash. It matches neither `ignored_count >= 1` nor `cli_events_count: 0` — measured directly, that field is not populated when no CLI event was ever created, so filtering on it excludes exactly the case catch-up is for. The query now keys on `events_count: 0` alone, which is the true signature of a request that produced no delivery. Live, the replay went from matching ~0 requests to 3 of 3. The window did not survive a crash, fixed in the previous commit. The outcome was assumed. The replay call returns as soon as the batch is accepted, so `estimated_count` is a plan and not a result. The plugin now polls the batch until Hookdeck reports it finished, and logs `N of M request(s) replayed`; a shortfall warns and names it, and a batch that does not finish inside the wait is reported as unknown rather than as success. The wait is bounded, because this runs when a tunnel reconnects and must not hold recovery open indefinitely. One approach was tried and abandoned, and the reason is worth keeping: a post-replay pass that re-read the window for requests still showing no event. It can never pass. A replay re-ingests each request as a NEW request with new events, so the original stays at events_count 0 for ever — the check reported recovered requests as stranded, and no amount of waiting would have changed that. The batch's own counts are the only evidence available. The honest limit is retention: 3 days on free, 7 on Team, 30 on Growth. Beyond it the request is gone at the source and nothing reaches it. 669 tests, and the live suite is now 14/14. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent cca0461 commit 5ca5c61

6 files changed

Lines changed: 350 additions & 8 deletions

File tree

docs/durability.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,27 @@ So while the transport is running the plugin records a liveness marker every `ca
3232

3333
Over-shooting that window by up to one heartbeat is harmless: the catch-up query matches only requests that produced no event at all, so anything that did deliver is excluded by construction.
3434

35+
## What catch-up guarantees
36+
37+
Within Hookdeck's retention, catch-up recovers every request that arrived during an outage and produced no delivery. Three things make that a guarantee rather than a hope, and each was wrong at some point:
38+
39+
**The filter matches every way a request can be stranded.** Measured against a live project, the two disconnect regimes look different:
40+
41+
| | `events_count` | `ignored_count` |
42+
|---|---|---|
43+
| The tunnel never existed | 0 | 1 |
44+
| The tunnel connected, then the process was killed | 0 | **0** |
45+
46+
The second is the hard-crash case. It matches neither `ignored_count >= 1` nor `cli_events_count: 0` — that field is not populated when no CLI event was ever created — so a filter using either excludes precisely the case catch-up exists for. The query keys on `events_count: 0` alone.
47+
48+
**The outage window survives a crash.** See [above](#a-crash-that-never-ran-its-shutdown).
49+
50+
**The result is confirmed, not assumed.** The replay call returns as soon as the batch is accepted, so its `estimated_count` is a plan. The plugin polls the batch until Hookdeck reports it finished and then logs `N of M request(s) replayed`. A shortfall is a warning naming it; a batch that does not finish within the wait is reported as unknown rather than as success.
51+
52+
What it cannot do is recover anything Hookdeck has already aged out — 3 days on free, 7 on Team, 30 on Growth. Beyond that the request is gone at the source, and no filter or replay reaches it.
53+
54+
> Do not try to verify recovery by re-reading the original requests. A replay re-ingests each one as a **new** request with new events, so the original stays at `events_count: 0` for ever. Checking it will always report a stranded request that was in fact recovered.
55+
3556
## Malformed bodies never reach the plugin
3657

3758
Verified end to end: Hookdeck rejects an unparseable JSON body **at the edge**, answering the sender `400` with `rejection_cause: UNPARSABLE_JSON` and creating no event. The plugin's own `malformed_json` handling is therefore defence in depth rather than a path real traffic takes — it covers a body that survives the edge and fails here, such as one that is valid JSON but not valid UTF-8.

scripts/e2e-live.mjs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,37 @@ record(
209209
);
210210

211211
log = await startGateway("restart");
212-
await sleep(20000);
212+
await sleep(25000);
213213
const catchUpLog = log.join("");
214214
record(
215215
"catch-up replay is issued on reconnect",
216216
/catch-up replay queued/.test(catchUpLog),
217217
(catchUpLog.match(/catch-up replay queued[^\n]*/) ?? ["not in log"])[0].slice(0, 80),
218218
);
219+
record(
220+
"the replay matched the stranded request rather than nothing",
221+
/catch-up replay queued \(~[1-9]/.test(catchUpLog),
222+
(catchUpLog.match(/catch-up replay queued[^\n]*/) ?? ["not in log"])[0].slice(0, 60),
223+
);
224+
record(
225+
"Hookdeck confirms the batch finished, with counts",
226+
/catch-up replay finished: \d+ of \d+/.test(catchUpLog),
227+
(catchUpLog.match(/catch-up replay finished:[^\n]*/) ?? ["not reported"])[0].slice(0, 70),
228+
);
229+
record(
230+
"every request it planned to replay was replayed",
231+
!/was not recovered/.test(catchUpLog),
232+
(catchUpLog.match(/catch-up replay finished:[^\n]*/) ?? [""])[0].slice(0, 50),
233+
);
234+
// The event should now exist for our connection.
235+
const recovered = (await eventsForConnection()).filter(
236+
(e) => e.created_at > new Date(t4 - 3000).toISOString(),
237+
);
238+
record(
239+
"the event missed during the outage now exists",
240+
recovered.length > 0,
241+
`${recovered.length} event(s) created after the outage began`,
242+
);
219243

220244
// ======================================================== 5. crash recovery
221245
const t5 = Date.now();

src/catchup.ts

Lines changed: 111 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,94 @@ export interface CatchUpQueryParams {
3434
sourceId?: string;
3535
}
3636

37+
/** What Hookdeck reported about a replay batch once it settled. */
38+
export interface BatchOutcome {
39+
/** True once Hookdeck reported the batch finished. */
40+
complete: boolean;
41+
replayed?: number;
42+
planned?: number;
43+
/** Set when the outcome could not be established. */
44+
unknown?: string;
45+
}
46+
47+
/**
48+
* Waits for a bulk replay to finish, within a bound.
49+
*
50+
* Bounded because this runs when a tunnel reconnects: a batch that never
51+
* completes must not hold the transport's recovery path open indefinitely. A
52+
* timeout is reported as unknown rather than as success.
53+
*/
54+
async function waitForBatch(
55+
options: CatchUpOptions,
56+
batchId: string,
57+
): Promise<BatchOutcome> {
58+
const deadline =
59+
(options.now ?? Date.now)() + (options.verifyTimeoutMs ?? 30_000);
60+
const sleep =
61+
options.sleep ?? ((ms: number) => new Promise((r) => setTimeout(r, ms)));
62+
63+
while ((options.now ?? Date.now)() < deadline) {
64+
const batch = await options.client.getBulkReplay(batchId);
65+
if (!batch.ok) {
66+
options.logger.warn(
67+
`catch-up replay was requested but its progress could not be read: ${batch.message}`,
68+
);
69+
return { complete: false, unknown: batch.message };
70+
}
71+
72+
if (batch.data.completed_at != null || batch.data.in_progress === false) {
73+
const replayed = batch.data.completed_count;
74+
const planned = batch.data.estimated_count;
75+
const short =
76+
replayed !== undefined && planned !== undefined && replayed < planned;
77+
78+
const line = `catch-up replay finished: ${replayed ?? "?"} of ${planned ?? "?"} request(s) replayed`;
79+
if (short) {
80+
options.logger.warn(
81+
`${line}. The shortfall arrived during the outage and was not recovered — ` +
82+
`replay it explicitly, or check it is still within Hookdeck's retention.`,
83+
);
84+
} else {
85+
options.logger.info(line);
86+
}
87+
88+
return {
89+
complete: true,
90+
...(replayed !== undefined ? { replayed } : {}),
91+
...(planned !== undefined ? { planned } : {}),
92+
};
93+
}
94+
await sleep(1000);
95+
}
96+
97+
// Not an error, but not a success either: saying which is the whole point.
98+
options.logger.warn(
99+
`catch-up replay ${batchId} had not finished within the wait, so whether every ` +
100+
`missed request was recovered is unknown. Check the batch in the Hookdeck dashboard.`,
101+
);
102+
return { complete: false, unknown: "timed out waiting for the batch" };
103+
}
104+
37105
export function buildCatchUpQuery(
38106
params: CatchUpQueryParams,
39107
): Record<string, unknown> {
40108
return {
41-
// Requests that produced no CLI event and at least one ignored event: the
42-
// signature of "arrived while nothing was listening".
43-
cli_events_count: 0,
44-
ignored_count: { gte: 1 },
109+
/**
110+
* Requests that produced NO EVENT AT ALL — the only filter that catches
111+
* every way a request can be stranded.
112+
*
113+
* Measured against a live project, because the two disconnect regimes look
114+
* different and only one of them was covered before:
115+
*
116+
* tunnel never existed events_count 0, ignored_count 1
117+
* tunnel connected then died events_count 0, ignored_count 0
118+
*
119+
* The second is the hard-crash case, and it matches neither
120+
* `ignored_count >= 1` nor `cli_events_count: 0` — that field is not
121+
* populated when no CLI event was ever created, so filtering on it
122+
* excludes precisely the case catch-up exists for.
123+
*/
124+
events_count: 0,
45125
ingested_at: {
46126
gte: new Date(params.sinceMs).toISOString(),
47127
...(params.untilMs !== undefined
@@ -61,12 +141,24 @@ export interface CatchUpOptions {
61141
sourceId?: string;
62142
/** Below this, an outage is not worth a bulk operation. */
63143
minGapMs?: number;
144+
/** Set false to skip waiting for the batch and the post-replay check. */
145+
verify?: boolean;
146+
/** How long to wait for the batch before reporting the outcome unknown. */
147+
verifyTimeoutMs?: number;
148+
/** Injectable for tests, so waiting costs no wall-clock. */
149+
sleep?(ms: number): Promise<void>;
64150
now?(): number;
65151
}
66152

67153
export type CatchUpResult =
68154
| { ran: false; reason: "gap_too_small" | "no_connection" }
69-
| { ran: true; batchId?: string; estimated?: number }
155+
| {
156+
ran: true;
157+
batchId?: string;
158+
estimated?: number;
159+
/** What Hookdeck reported once the batch finished. */
160+
recovered?: BatchOutcome;
161+
}
70162
| { ran: false; reason: "failed"; message: string };
71163

72164
export const DEFAULT_MIN_GAP_MS = 30_000;
@@ -109,11 +201,25 @@ export async function runCatchUp(
109201
return { ran: false, reason: "failed", message: result.message };
110202
}
111203

204+
// Waited for, because the replay call returns as soon as the batch is
205+
// accepted. The batch's own counts are the evidence of recovery: a replay
206+
// re-ingests each request as a NEW request with new events, leaving the
207+
// original at events_count 0 forever — so re-reading the window can never
208+
// show recovery, however long you wait for it.
209+
const batch =
210+
result.data.id !== undefined && options.verify !== false
211+
? await waitForBatch(options, result.data.id)
212+
: undefined;
213+
214+
// Checked, not assumed. A bulk replay reports what it queued rather than
215+
// what it recovered, so without this the only honest claim would be "a
216+
// replay was requested".
112217
return {
113218
ran: true,
114219
...(result.data.id !== undefined ? { batchId: result.data.id } : {}),
115220
...(result.data.estimated_count !== undefined
116221
? { estimated: result.data.estimated_count }
117222
: {}),
223+
...(batch !== undefined ? { recovered: batch } : {}),
118224
};
119225
}

src/hookdeck/client.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,31 @@ export interface HookdeckClient {
230230
id: string;
231231
verified?: boolean | null;
232232
rejection_cause?: string | null;
233+
/** Zero means the request produced no delivery to anything. */
234+
events_count?: number | null;
235+
ingested_at?: string;
233236
}[]
234237
>
235238
>;
236239

240+
/**
241+
* Progress of a bulk replay.
242+
*
243+
* The replay call returns as soon as the batch is accepted, so its
244+
* `estimated_count` is a plan rather than a result. Only `completed_at`
245+
* says the work is done, which is the difference between "a replay was
246+
* requested" and "the events exist".
247+
*/
248+
getBulkReplay(id: string): Promise<
249+
ApiResult<{
250+
id: string;
251+
completed_at?: string | null;
252+
in_progress?: boolean;
253+
estimated_count?: number;
254+
completed_count?: number;
255+
}>
256+
>;
257+
237258
bulkReplayRequests(params: {
238259
query: Record<string, unknown>;
239260
target: { webhook_ids?: string[]; source_id?: string };
@@ -492,6 +513,10 @@ export function createHookdeckClient(
492513
return result.ok ? { ok: true, data: result.data.models ?? [] } : result;
493514
},
494515

516+
async getBulkReplay(id) {
517+
return request("GET", `/bulk/requests/replay/${encodeURIComponent(id)}`);
518+
},
519+
495520
async bulkReplayRequests(params) {
496521
// `target` goes INSIDE `query`, and is required there. Sending it at the
497522
// top level is answered `422 query.target is required` — the whole call

0 commit comments

Comments
 (0)