fix(bot): stop retrying a webhook delivery forever when it throws

apps/bot/src/webhook-retry.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from "vitest";
+
+import { backoffMs, failedAttempt, MAX_ATTEMPTS } from "./webhook-retry.js";
+
+const NOW = Date.UTC(2026, 10, 5, 12, 0, 0);
+
+describe("failedAttempt", () => {
+  it("counts the attempt and schedules a retry", () => {
+    const next = failedAttempt(0, "HTTP 500", NOW);
+    expect(next.status).toBe("pending");
+    expect(next.attempts).toBe(1);
+    expect(next.lastError).toBe("HTTP 500");
+    expect(next.nextAttemptAt.getTime()).toBe(NOW + backoffMs(1));
+    expect(next.nextAttemptAt.getTime()).toBeGreaterThan(NOW);
+  });
+
+  it("gives up after MAX_ATTEMPTS", () => {
+    expect(failedAttempt(MAX_ATTEMPTS - 2, "x", NOW).status).toBe("pending");
+    expect(failedAttempt(MAX_ATTEMPTS - 1, "x", NOW).status).toBe("failed");
+  });
+
+  it("ends a row that fails on every tick instead of retrying it forever", () => {
+    let attempts = 0;
+    let status = "pending";
+    let ticks = 0;
+    while (status === "pending" && ticks < 100) {
+      const next = failedAttempt(attempts, "Cannot read properties of undefined", NOW);
+      attempts = next.attempts;
+      status = next.status;
+      ticks++;
+    }
+    expect(status).toBe("failed");
+    expect(ticks).toBe(MAX_ATTEMPTS);
+  });
+
+  it("keeps lastError short enough for the column", () => {
+    expect(failedAttempt(0, "e".repeat(2000), NOW).lastError).toHaveLength(500);
+    expect(failedAttempt(0, null, NOW).lastError).toBeNull();
+  });
+});

apps/bot/src/webhook-retry.ts

@@ -0,0 +1,35 @@
+/** Give up (mark failed) after this many attempts. */
+export const MAX_ATTEMPTS = 6;
+
+/** Exponential-ish backoff (minutes) keyed by the attempt just made. */
+export function backoffMs(attempts: number): number {
+  const minutes = [1, 5, 15, 60, 180, 360];
+  return (minutes[Math.min(attempts, minutes.length - 1)] ?? 360) * 60_000;
+}
+
+/** The columns a failed attempt writes back to its `WebhookDelivery` row. */
+export type FailedAttempt = {
+  status: "pending" | "failed";
+  attempts: number;
+  lastError: string | null;
+  nextAttemptAt: Date;
+};
+
+/**
+ * State after one failed attempt: count it, back off, and give up once
+ * MAX_ATTEMPTS is reached. Every failure goes through here, including an
+ * exception thrown while building the body, so no row can be retried forever.
+ */
+export function failedAttempt(
+  previousAttempts: number,
+  error: string | null,
+  now: number = Date.now(),
+): FailedAttempt {
+  const attempts = previousAttempts + 1;
+  return {
+    status: attempts >= MAX_ATTEMPTS ? "failed" : "pending",
+    attempts,
+    lastError: error === null ? null : error.slice(0, 500),
+    nextAttemptAt: new Date(now + backoffMs(attempts)),
+  };
+}

apps/bot/src/webhooks.ts

@@ -15,11 +15,10 @@ import { config } from "./config.js";
 import { guildStrings } from "./guild-i18n.js";
 import { buildLogEmbed } from "./log-embed.js";
 import { dashboardSubmissionUrl } from "./urls.js";
+import { failedAttempt } from "./webhook-retry.js";
 
 /** How many pending deliveries to drain per tick. */
 const BATCH = 25;
-/** Give up (mark failed) after this many attempts. */
-const MAX_ATTEMPTS = 6;
 /** Per-request timeout. */
 const TIMEOUT_MS = 10_000;
 
@@ -45,12 +44,6 @@ const DISCORD_WEBHOOK_GONE = new Set([401, 404]);
 /** What happened to one delivery, as far as the poll loop needs to know. */
 type DeliveryResult = { rateLimitedUntil?: number; disabled?: boolean };
 
-/** Exponential-ish backoff (minutes) keyed by the attempt just made. */
-function backoffMs(attempts: number): number {
-  const minutes = [1, 5, 15, 60, 180, 360];
-  return (minutes[Math.min(attempts, minutes.length - 1)] ?? 360) * 60_000;
-}
-
 /** The thin payload stored in the outbox at enqueue time. */
 type ThinPayload = {
   event?: WebhookEvent;
@@ -66,12 +59,12 @@ type ThinPayload = {
  * Turn the thin outbox payload into the rich {@link buildSubmissionWebhookPayload}
  * body by loading the submission + form once at delivery time. This enriches
  * every event regardless of which path enqueued it (submit, web review, bulk, bot
- * accept, automations) and always reflects the current state. Falls back to the
- * stored thin payload when the submission has since been deleted.
+ * accept, automations) and always reflects the current state. Returns null when
+ * the payload names no submission or the submission has since been deleted.
  */
-async function hydratePayload(stored: unknown): Promise<unknown> {
+async function hydratePayload(stored: unknown): Promise<SubmissionWebhookPayload | null> {
   const thin = (stored ?? {}) as ThinPayload;
-  if (!thin.event || !thin.submissionId) return stored ?? {};
+  if (!thin.event || !thin.submissionId) return null;
 
   const sub = await prisma.submission.findUnique({
     where: { id: thin.submissionId },
@@ -86,7 +79,7 @@ async function hydratePayload(stored: unknown): Promise<unknown> {
       user: { select: { discordId: true, username: true } },
     },
   });
-  if (!sub) return stored ?? {};
+  if (!sub) return null;
 
   return buildSubmissionWebhookPayload({
     event: thin.event,
@@ -174,9 +167,20 @@ async function deliverOne(row: DeliveryRow): Promise<DeliveryResult> {
     body = JSON.stringify(auditBody);
   } else {
     const hydrated = await hydratePayload(row.payload);
-    body = isDiscord
-      ? JSON.stringify(buildDiscordWebhookBody(hydrated as SubmissionWebhookPayload))
-      : JSON.stringify(hydrated);
+    if (hydrated) {
+      body = JSON.stringify(isDiscord ? buildDiscordWebhookBody(hydrated) : hydrated);
+    } else if (isDiscord) {
+      // The Discord embed is built from the submission itself. Without it there
+      // is nothing to show, and retrying will not bring it back.
+      await prisma.webhookDelivery.update({
+        where: { id: row.id },
+        data: { status: "failed", lastError: "Submission no longer exists." },
+      });
+      return {};
+    } else {
+      // Generic hooks still get the stored thin payload, as before.
+      body = JSON.stringify(row.payload ?? {});
+    }
   }
 
   const headers: Record<string, string> = {
@@ -248,19 +252,33 @@ async function deliverOne(row: DeliveryRow): Promise<DeliveryResult> {
     return { disabled: true };
   }
 
-  const giveUp = attempts >= MAX_ATTEMPTS;
   await prisma.webhookDelivery.update({
     where: { id: row.id },
-    data: {
-      status: giveUp ? "failed" : "pending",
-      attempts,
-      lastError: error,
-      nextAttemptAt: new Date(Date.now() + backoffMs(attempts)),
-    },
+    data: failedAttempt(row.attempts, error),
   });
   return {};
 }
 
+/**
+ * Count an attempt that threw before it could record its own outcome (a body
+ * that cannot be built, a database hiccup). Without this the row stays due and
+ * is retried on every tick, forever.
+ */
+async function recordCrash(row: DeliveryRow, err: unknown): Promise<void> {
+  const message = err instanceof Error ? err.message : String(err);
+  try {
+    await prisma.webhookDelivery.update({
+      where: { id: row.id },
+      data: failedAttempt(row.attempts, `Delivery crashed: ${message}`),
+    });
+  } catch (updateErr) {
+    console.error(
+      `[bot] could not record the failed delivery ${row.id}:`,
+      (updateErr as Error).message,
+    );
+  }
+}
+
 /** Drain the webhook outbox: deliver every due `pending` row (best-effort). */
 export async function deliverPendingWebhooks(): Promise<void> {
   if (running) return;
@@ -292,6 +310,7 @@ export async function deliverPendingWebhooks(): Promise<void> {
         if (result.disabled) blocked.set(row.webhookId, Number.POSITIVE_INFINITY);
       } catch (err) {
         console.error(`[bot] webhook delivery ${row.id} failed:`, (err as Error).message);
+        await recordCrash(row, err);
       }
     }
   } catch (err) {