[Bot] Fail loudly instead of silently black-holing every event on a misconfigured notification channel #369
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Impact: CRITICAL
Found in the August 2026 session lifecycle review (#319).
What the user experiences
A GM types a Discord channel ID into a plain text field with no validation and no instructions. If it's wrong — non-numeric, deleted, or the bot lacks permission to post there — every single notification the product would ever send to that campaign (reminders, vote posts, summaries, everything) silently vanishes, forever, with no error in Discord and no error in the web app. This is the single most likely first-run misconfiguration for a new hosted customer, and today it is completely undiagnosable from either side of the product.
Evidence
bot/questboard_bot/cogs/notifications.py:291-314—_resolve_channelreturnsNonefor a non-integer, deleted, forbidden, or non-text channel with no caller-visible signal.bot/questboard_bot/services/redis_bus.py(_process_entry, lines ~85-113, docstring at :93-95: "The entry is XACKed even when the handler could not post ... acking avoids a poison entry blocking the group forever") — the stream entry is acknowledged as delivered regardless of whether_resolve_channelactually succeeded, so there is no durable record that delivery failed either.webapp/frontend/src/pages/CampaignDetail.jsx:1830-1836— "Bot notification channel ID (optional)" is a bare free-text field with no instructions and no validation, inside a collapsed Edit Campaign form.Why it matters for a hosted product
A typo in one text field produces total, permanent, silent loss of every downstream notification with no signal on either side. This is the textbook "the bot just stopped working" ticket, and today there is no way for a self-hoster or a support agent to diagnose it without reading logs.
Proposed fix
redis_bus._process_entrymust not XACK when the handler could not post (or must XACK but record the failure separately so it isn't silently discarded).notifications.py's channel-resolution failure should be surfaced back to the web app — e.g. via a lightweight status callback or a periodic health check — soCampaignDetailcan show "Discord notifications are failing — check the channel ID" (the audit's P16). The existing "Send test message" button (CampaignDetail.jsx:1839-1846) is a genuinely good pattern already in the codebase for this kind of feedback — extend that pattern rather than inventing a new one, and make it available for each ID field independently rather than only once both webhook and channel ID are filled (:1837).Acceptance criteria
CampaignDetail.jsxshows a persistent, visible warning when the configured notification channel is failing to deliver._resolve_channelreturnsNoneand asserts the entry is not silently marked delivered.Picking this up as v4.3.0 phase 5, lane A (#514), followed by #371 on the same branch. Decisions: the failure is recorded durably on the campaign (
discord_delivery_statusJSONB, one migration) through a new additive bot→backend endpoint, cleared by the next successful post; the stream entry is still acked, but only after the failure is recorded, so nothing is both unrecorded and acked;CampaignDetailshows a persistent banner while delivery is failing; the test-message affordance can test the channel ID on its own and the input validates the ID shape.Done in PR #523 (auto-merging on green); ships with v4.3.0.
A channel-resolution failure is now recorded rather than only logged.
_resolve_channelreturns why it failed (invalid_id/not_found/forbidden/not_text/unknown) andon_bot_notifyposts that to a new additivePOST /api/bot/campaigns/{id}/delivery-status, which writes a nullable JSONBcampaigns.discord_delivery_status({state, reason, channel_id, event_type, at, message}; migrationf0ab1c2d3e4fone9fa0b1c2d3e, one head;atis stamped by the backend so a bot on another machine cannot produce a future timestamp). NULL is deliberately not "ok" and nothing is backfilled: an untried channel has demonstrated nothing.A fifth reason the issue did not list,
cannot_send, comes from adiscord.Forbiddenon the post rather than the resolve. A channel the bot can see but not speak in was indistinguishable from success, and it is the commonest misconfiguration after a wrong ID.The entry is still acked (the poison-entry argument stands), but the record is written before the ack, and if the record also fails that goes to the log at ERROR with campaign, reason and event type, so nothing is ever both unrecorded and acked without a trace. Both paths are regression-tested.
CampaignResponseexposes the status to every member (a channel ID, a reason code and a sentence; no secrets, and a player wondering why the reminders stopped is owed the same answer).CampaignDetailrenders it as a banner at the top of the page, outside the collapsed Edit form, since putting it behind the same disclosure as the broken field would inherit the invisibility. It clears itself on the first successful post, including the first after a bot restart.The "Send test message" button had three further problems and is replaced: it was admin-only on a page any GM can open (a 403 for most self-hosters), it reported success on having published to Redis rather than on anything arriving (the same lie as the black hole), and it tested channel and webhook as one thing. There are now two buttons, one per field, hitting a GM-authorised
POST /api/campaigns/{id}/discord-testthat is synchronous end to end: the channel test calls a newPOST /test-channelon the bot, which resolves and posts before answering, and the verdict is written to the delivery status, so a passing test clears the banner; the webhook test is guarded by the existing Discord-webhook URL check. The channel field rejects anything that is not 17–20 digits, with a line saying where to find the ID. An older bot returns 404 on/test-channel, which the backend reports as "upgrade the bot image" rather than blaming the channel ID.No
BOT_CONTRACT_VERSIONbump: the endpoint is purely additive and an older bot never calls it. Bot 432 tests pass, backend 2405 / 13 skipped, frontend 539, ruff and version-sync clean, migration round-trips up/down/up on Postgres 16.