Skip to content

Commit 0b790bc

Browse files
committed
fix: improved resilience of webhook acknowledgement
1 parent 5eaf5fc commit 0b790bc

2 files changed

Lines changed: 34 additions & 11 deletions

File tree

api/src/webhook/webhook.controller.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,12 @@ export class WebhookController {
4545
throw new BadRequestException('missing event or delivery headers');
4646
}
4747

48-
await this.webhooks.handle(event, delivery, raw, req.body as unknown);
49-
48+
const shouldProcess = await this.webhooks.record(event, delivery, raw, req.body as unknown);
49+
50+
if (shouldProcess) {
51+
void this.webhooks.dispatch(event, delivery, req.body as unknown);
52+
}
53+
5054
return { ok: true };
5155
}
5256
}

api/src/webhook/webhook.service.ts

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,32 +28,51 @@ export class WebhookService {
2828
private readonly orchestrator: OrchestratorService,
2929
) {}
3030

31-
async handle(
31+
/**
32+
* Records the delivery for idempotency and returns true if it should be
33+
* dispatched. Call this synchronously before returning 202 to GitHub.
34+
*/
35+
async record(
3236
event: string,
3337
deliveryId: string,
3438
rawBody: string,
3539
payload: unknown,
36-
): Promise<void> {
40+
): Promise<boolean> {
3741
const action = (payload as { action?: string }).action;
42+
3843
this.metrics.recordWebhook(event, action);
3944

45+
4046
const existing = await this.prisma.webhookEvent.findUnique({ where: { deliveryId } });
47+
4148
if (existing?.processedAt) {
4249
this.logger.debug(`Duplicate delivery ${deliveryId} already processed; skipping`);
43-
return;
50+
return false;
4451
}
52+
4553
if (!existing) {
4654
await this.prisma.webhookEvent.create({
4755
data: { deliveryId, event, action, payload: rawBody },
4856
});
4957
}
58+
59+
return true;
60+
}
5061

51-
await this.route(event, payload);
52-
53-
await this.prisma.webhookEvent.update({
54-
where: { deliveryId },
55-
data: { processedAt: new Date() },
56-
});
62+
/**
63+
* Routes and marks the delivery processed. Intended to be called
64+
* fire-and-forget after record() returns true.
65+
*/
66+
async dispatch(event: string, deliveryId: string, payload: unknown): Promise<void> {
67+
try {
68+
await this.route(event, payload);
69+
await this.prisma.webhookEvent.update({
70+
where: { deliveryId },
71+
data: { processedAt: new Date() },
72+
});
73+
} catch (e) {
74+
this.logger.error(`Delivery ${deliveryId} routing failed: ${(e as Error).message}`);
75+
}
5776
}
5877

5978
private async route(event: string, payload: unknown): Promise<void> {

0 commit comments

Comments
 (0)