feat(notifications): add Discord provider - #637
Conversation
TODO.md checked off Discord/Teams/Telegram/Google Chat as implemented, but only Slack/email/webhook existed in providers/ and NotificationChannel. Adds a real Discord provider as a first, self-contained slice; corrects the TODO.md checkboxes for the three still-unimplemented channels. - providers/discord.ts: webhook-based DiscordProvider mirroring SlackProvider, posting an embed (title/description/fields, with a priority color + footer for non-normal priority) - providers/payload-utils.ts: truncate/isUserFacingMetadata/ formatMetadataLabel extracted out of slack.ts so Discord doesn't duplicate them; slack.ts now imports from here - types.ts: NotificationChannel gains "discord"; DiscordPayload/ DiscordEmbed/DiscordEmbedField added - client.ts, providers/index.ts: DiscordProvider wired in and exported Deliberately not touched: the alarms DB schema, RPC destination validation (packages/rpc/src/routers/alarms.ts), and the dashboard alarm-sheet UI. Wiring a new channel into the alarms feature end-to-end is a separate, larger surface — a natural follow-up once this provider itself lands. Refs databuddy-analytics#635
|
@FindMalek is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR adds a webhook-based Discord notification provider and integrates it with NotificationClient.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Client as NotificationClient
participant Discord as DiscordProvider
participant Guard as SSRF-safe fetch
participant API as Discord Webhook API
Caller->>Client: sendToChannel("discord", payload)
Client->>Discord: send(payload)
Discord->>Discord: build bounded embed
Discord->>Guard: POST webhook with retry/timeout
Guard->>API: validated request
API-->>Guard: HTTP response
Guard-->>Discord: response
Discord-->>Client: NotificationResult
Client-->>Caller: NotificationResult
Reviews (3): Last reviewed commit: "fix(notifications): enforce Discord's ag..." | Re-trigger Greptile |
| return { | ||
| title: truncate(payload.title, MAX_TITLE_LENGTH), | ||
| description: truncate(payload.message, MAX_DESCRIPTION_LENGTH), | ||
| ...(fields.length > 0 && { fields }), | ||
| ...(elevatedPriority && { | ||
| color: PRIORITY_COLORS[elevatedPriority], | ||
| footer: { text: `Priority: ${elevatedPriority.toUpperCase()}` }, |
There was a problem hiding this comment.
Aggregate embed limit is unenforced
When a notification contains a near-limit description and multiple long metadata fields, these independently valid components exceed Discord's 6,000-character aggregate embed limit, causing Discord to reject the webhook and the alert to remain undelivered after retries.
| export type { DiscordProviderConfig } from "./discord"; | ||
| export { DiscordProvider } from "./discord"; |
There was a problem hiding this comment.
Discord extends the provider barrel
These exports expose the new Discord provider through an index-file barrel despite the repository's direct-import requirement, extending the prohibited indirection and making provider ownership less explicit.
Context Used: Basic guidelines for the project so vibe coders do... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Greptile flagged that per-component caps alone aren't enough: Discord rejects an embed whose title + description + every field's name/value + footer text sum past 6000 characters, even when each piece individually respects its own limit (25 fields at the max 1024 chars each alone sums to 25,600). Track a running total while building fields and stop adding more once the next one would push the embed over the aggregate limit.
|
Fixed the real one, pushing back gently on the other: Aggregate 6000-char limit — fixed in 07b65f0. Good catch, this was a genuine gap: the per-component caps (256/4096/25 fields/1024 per value) don't prevent the sum from exceeding Discord's actual 6000-char aggregate embed limit — 25 fields at max value length alone would be 25,600 chars. Now tracking a running total while building fields and stopping before the next field would push the embed over 6000. Added a test that constructs a payload designed to hit the aggregate cap before the field-count cap and asserts the total stays ≤ 6000. Barrel export — leaving as-is. I don't think there's actually a direct-import rule here: |
|
@greptile review |
|
@izadoesdev this is ready for review whenever you have a chance — CI is green aside from the Vercel preview checks, which need a team member to authorize the deploy (outside my permissions as an external contributor). |
Refs #635
Summary
packages/notifications/TODO.mdchecked off Discord (along with Teams, Telegram, Google Chat) as implemented, but onlyslack/email/webhookexisted inproviders/and in theNotificationChanneltype. This adds a real Discord provider as a first, self-contained slice, and corrects the TODO.md checkboxes for the three channels that are still genuinely unimplemented.Approach
providers/discord.ts: webhook-basedDiscordProvidermirroringSlackProvider's shape (retry/timeout viaBaseProvider, safe fetch via the SSRF guard). Posts a Discord embed (title/description/fields, capped to Discord's actual limits — 256/4096/25 fields/1024 per field value), with a color + footer shown only for non-normal priority, mirroring Slack'selevatedPrioritycontext block.providers/payload-utils.ts: pulledtruncate/isUserFacingMetadata/formatMetadataLabelout ofslack.tsinto a shared module sodiscord.tsdoesn't duplicate them (identical behavior,slack.tsnow just imports from here — no logic changes there).types.ts:NotificationChannelgains"discord"; addedDiscordPayload/DiscordEmbed/DiscordEmbedField, mirroring the existing Slack types.client.ts/providers/index.ts:DiscordProviderwired intoNotificationClientand exported from the barrel, same pattern as the other three providers.TODO.md: Discord stays checked (now true); unchecked Teams/Telegram/Google Chat since those still don't exist in code.Deliberately out of scope
Not touched: the alarms DB schema, RPC destination validation (
packages/rpc/src/routers/alarms.ts—SLACK_WEBHOOK_PATTERN-style validation, discriminated union), or the dashboardalarm-sheetUI. Wiring a new channel into the Alarms feature end-to-end (schema + validation + UI) is a bigger, separate surface than the provider primitive itself — happy to open that as a follow-up once this lands, rather than bundling a large multi-layer change into one PR.Verification
Full monorepo
turbo run check-typesandturbo run testalso ran clean via the repo's pre-commit/pre-push hooks.Disclosure
I used Claude Code to help write the provider (following
SlackProvider's existing shape) and its tests. I read every changed line, ran the test suite and type-check myself, and manually checked Discord's actual embed field limits (256/4096/25 fields/1024 per value) against the API docs before encoding them.Summary by cubic
Add a webhook-based Discord provider to
packages/notificationsand wire it intoNotificationClient. Previously only Slack/email/webhook were supported; nowNotificationChannelincludes "discord" and the client can send rich embeds to Discord, enforcing per-field caps and Discord’s 6000‑character aggregate embed limit to avoid API errors.DiscordProviderwith SSRF-guarded fetch via@databuddy/shared/ssrf-guard, retries/timeouts fromBaseProvider, and embeds with truncation, 25-field cap, and color/footer for high/urgent priority.providers/payload-utils.ts;SlackProvidernow imports them with no behavior change.DiscordPayload/DiscordEmbedand exports the provider from the barrel;NotificationClientaccepts adiscordconfig.TODO.mdto keep Teams/Telegram/Google Chat unchecked. Refs Linear docs(notifications): TODO.md claims Discord/Teams/Telegram/Google Chat providers exist — they don't #635.Rollout
discord: { webhookUrl, username?, avatarUrl?, retries?, retryDelay?, timeout? }toNotificationClient.Written for commit 07b65f0. Summary will update on new commits.