-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(bot): bound outbox delivery retries, treat 50278 as permanent #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| import { | ||
| isExhausted, | ||
| isTerminalDmCode, | ||
| MAX_DELIVERY_ATTEMPTS, | ||
| nextAttemptAt, | ||
| retryDelayMs, | ||
| } from "./delivery-policy.js"; | ||
|
|
||
| describe("terminal DM codes", () => { | ||
| it("treats both 'cannot DM this user' codes as permanent", () => { | ||
| expect(isTerminalDmCode(50007)).toBe(true); | ||
| // The one that was missing and caused rows to be retried forever. | ||
| expect(isTerminalDmCode(50278)).toBe(true); | ||
| }); | ||
|
|
||
| it("accepts the string form discord.js sometimes hands back", () => { | ||
| expect(isTerminalDmCode("50278")).toBe(true); | ||
| }); | ||
|
|
||
| it("leaves genuinely transient failures retryable", () => { | ||
| expect(isTerminalDmCode(500)).toBe(false); | ||
| expect(isTerminalDmCode(undefined)).toBe(false); | ||
| expect(isTerminalDmCode(null)).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("backoff", () => { | ||
| it("grows with each failure", () => { | ||
| expect(retryDelayMs(0)).toBe(30_000); | ||
| expect(retryDelayMs(1)).toBe(60_000); | ||
| expect(retryDelayMs(2)).toBe(120_000); | ||
| }); | ||
|
|
||
| it("is capped so a stuck row cannot drift years into the future", () => { | ||
| expect(retryDelayMs(50)).toBe(30 * 60_000); | ||
| }); | ||
|
|
||
| it("never returns a negative or NaN delay for junk input", () => { | ||
| expect(retryDelayMs(-5)).toBe(30_000); | ||
| expect(retryDelayMs(1.7)).toBe(60_000); | ||
| }); | ||
|
|
||
| it("offsets from the given clock", () => { | ||
| const now = new Date("2026-08-18T00:00:00.000Z"); | ||
| expect(nextAttemptAt(0, now).toISOString()).toBe("2026-08-18T00:00:30.000Z"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("exhaustion", () => { | ||
| it("retires a row only after the last attempt", () => { | ||
| expect(isExhausted(MAX_DELIVERY_ATTEMPTS - 1)).toBe(false); | ||
| expect(isExhausted(MAX_DELIVERY_ATTEMPTS)).toBe(true); | ||
| }); | ||
|
|
||
| it("keeps the whole retry window under a few hours", () => { | ||
| let total = 0; | ||
| for (let i = 0; i < MAX_DELIVERY_ATTEMPTS; i++) total += retryDelayMs(i); | ||
| expect(total).toBeLessThan(3 * 60 * 60_000); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // When to stop trying to deliver an outbox row. | ||
| // | ||
| // Kept free of discord.js and Prisma so the policy itself is unit-testable: | ||
| // getting this wrong is not loud. A row that can never succeed and is never | ||
| // retired simply stays at the head of the queue, and since the poller takes a | ||
| // fixed batch ordered by age, enough of them starve every newer notification. | ||
|
|
||
| /** | ||
| * Discord error codes that mean "this DM will never arrive", as opposed to | ||
| * "not right now". Both are 403s and both look alike in a log: | ||
| * | ||
| * 50007 Cannot send messages to this user (DMs closed) | ||
| * 50278 Cannot send messages to this user due to having no mutual guilds | ||
| * | ||
| * 50278 was missing here until 2026-08-18, so applicants who left the guild | ||
| * left behind a row that was retried every 15 seconds indefinitely. | ||
| */ | ||
| export const TERMINAL_DM_CODES = [50007, 50278] as const; | ||
|
|
||
| export function isTerminalDmCode(code: unknown): boolean { | ||
| return (TERMINAL_DM_CODES as readonly number[]).includes(Number(code)); | ||
| } | ||
|
|
||
| /** | ||
| * Give up after this many failed attempts. The backoff below spreads them over | ||
| * roughly two hours, which outlasts a normal Discord incident without letting a | ||
| * permanently broken row live forever. | ||
| */ | ||
| export const MAX_DELIVERY_ATTEMPTS = 8; | ||
|
|
||
| const BASE_DELAY_MS = 30_000; | ||
| const MAX_DELAY_MS = 30 * 60_000; | ||
|
|
||
| /** Exponential backoff, capped. `attempts` is the number of failures so far. */ | ||
| export function retryDelayMs(attempts: number): number { | ||
| const n = Math.min(Math.max(0, Math.trunc(attempts)), 20); | ||
| return Math.min(MAX_DELAY_MS, BASE_DELAY_MS * 2 ** n); | ||
| } | ||
|
|
||
| /** When a row that just failed for the `attempts`-th time may be tried again. */ | ||
| export function nextAttemptAt(attempts: number, now: Date): Date { | ||
| return new Date(now.getTime() + retryDelayMs(attempts)); | ||
| } | ||
|
|
||
| /** True once a row has burned through its attempts and should be retired. */ | ||
| export function isExhausted(attempts: number): boolean { | ||
| return attempts >= MAX_DELIVERY_ATTEMPTS; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
packages/db/prisma/migrations/20260818003000_notification_retry_bounds/migration.sql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| -- Bound the outbox retry loop. | ||
| -- | ||
| -- Until now a notification that failed to deliver was simply left unread and | ||
| -- picked up again on the next 15s tick, forever. Discord returns 50278 ("no | ||
| -- mutual guilds") for applicants who left the server, and the bot only treated | ||
| -- 50007 as permanent, so those rows never retired. Because the poller takes a | ||
| -- fixed batch ordered by age, enough of them at the head of the queue would | ||
| -- have starved every newer notification. | ||
| ALTER TABLE "notifications" | ||
| ADD COLUMN "attempts" INTEGER NOT NULL DEFAULT 0, | ||
| ADD COLUMN "last_error" TEXT, | ||
| ADD COLUMN "next_attempt_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP; | ||
|
|
||
| -- The poller now scans: | ||
| -- WHERE read_at IS NULL AND next_attempt_at <= now() ORDER BY created_at ASC | ||
| -- so the pending partial index gains the new column. It supersedes | ||
| -- notifications_pending_idx from 20260620180000, which is dropped rather than | ||
| -- kept alongside: two partial indexes over the same rows would both have to be | ||
| -- maintained on every insert into a hot table. (Partial indexes aren't | ||
| -- expressible in the Prisma schema, hence the hand-written migration.) | ||
| CREATE INDEX IF NOT EXISTS "notifications_due_idx" | ||
| ON "notifications" ("next_attempt_at", "created_at") | ||
| WHERE "read_at" IS NULL; | ||
|
|
||
| DROP INDEX IF EXISTS "notifications_pending_idx"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.