Skip to content

[Improve] Harden the Discord gateway delivery and leadership paths#410

Merged
daniel-lxs merged 4 commits into
developfrom
feat/discord-gateway-hardening
Jul 16, 2026
Merged

[Improve] Harden the Discord gateway delivery and leadership paths#410
daniel-lxs merged 4 commits into
developfrom
feat/discord-gateway-hardening

Conversation

@daniel-lxs

Copy link
Copy Markdown
Member

Reliability follow-ups to #268, from the gateway review that ran before it merged.

Changes

  • Dead-letter visibility — quarantined (undeliverable) events now surface: the gateway status tracks deadLetterDepth and Discord settings shows a failing Event delivery diagnostic row when anything is quarantined, instead of events accumulating in a stream nothing reads.
  • Bounded streams — the durable inbound stream and the dead-letter stream get approximate MAXLEN caps (DISCORD_GATEWAY_INBOUND_MAX_ENTRIES, default 50k; DISCORD_GATEWAY_DEAD_LETTER_MAX_ENTRIES, default 1k). A multi-hour delivery outage can no longer grow Redis without limit; capacity pressure is reported in status before shedding starts.
  • Lease-renewal tolerance — a single transient Redis error on the 10s renew no longer tears down a healthy Gateway connection whose 30s lease is still valid. renewDetailed on the Redis lock distinguishes definitive ownership loss (immediate teardown) from a transient error (tolerated once).
  • Backoff reset — the delivery-loop restart backoff starts over after a long healthy run instead of ratcheting toward the 30s maximum for the lifetime of the process.
  • Supervisor death pages — the embedded supervisor takes an onFatal hook (wired to Sentry in the BullMQ host) and stamps the shared status, so silently-stopped Discord ingestion is visible in error tracking and diagnostics rather than only a console line.

Validation

  • discord-gateway suite: 72/72 (includes new coverage: stream bounds + DLQ depth, backoff reset via fake-timer choreography, onFatal + status stamping)
  • web DiscordSetupStatus and bullmq suites green; check-types 26/26, oxlint, format, knip all pass
  • Honest gap: the lease-renewal streak logic in service.ts has no direct unit harness (the renew timer isn't injectable); it's covered by the renewDetailed contract and kept deliberately simple.

🤖 Generated with Claude Code

Follow-ups from the PR #268 reliability review:

- surface the dead-letter stream: quarantine depth is tracked in gateway
  status and shown as a failing diagnostic row in Discord settings, so
  undeliverable events are visible instead of accumulating silently
- bound the durable streams: the inbound stream and dead-letter stream
  get approximate MAXLEN caps (DISCORD_GATEWAY_INBOUND_MAX_ENTRIES,
  DISCORD_GATEWAY_DEAD_LETTER_MAX_ENTRIES), and capacity pressure is
  reported in status before shedding starts
- tolerate one transient Redis error on lease renewal: the 30s lease
  leaves room for two 10s renew intervals, so only a definitive
  ownership loss or a second consecutive failure tears down the Gateway
  connection (renewDetailed distinguishes lost from error)
- restart the delivery backoff after a long healthy run instead of
  ratcheting toward the lifetime maximum forever
- report supervisor death: the embedded supervisor takes an onFatal
  hook, wired to Sentry in the BullMQ host, and stamps the shared
  status so a silently-stopped Discord ingestion pages and shows in
  diagnostics

The lease-renewal tolerance is covered indirectly (renewDetailed
contract + simple streak logic); there is no service-level harness to
drive the renew timer directly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roomote-roomote

roomote-roomote Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

No new code issues found. See task

  • Bound retry metadata when stream entries are shedapps/discord-gateway/src/inbound-queue.ts:16: MAXLEN can evict undelivered entries without deleting their fields from DISCORD_INBOUND_ATTEMPTS_KEY; during a sustained outage, each newly exposed oldest entry is attempted and later trimmed, so the attempts hash still grows without bound. Remove stale attempt fields as entries are shed or store retry metadata in a structure that is bounded with the stream.
  • Keep capacity pressure visible while the backlog remains highapps/discord-gateway/src/service.ts:176-190 and apps/discord-gateway/src/delivery-loop.ts:99-103: the timer writes the pressure warning to lastError, but every successful delivery immediately publishes lastError: undefined. With partial throughput and a queue still above 90%, the warning is repeatedly erased even as MAXLEN is about to shed events; use a dedicated pressure field or only clear the warning after depth falls below the threshold.
  • Avoid deleting live retry counters during empty-stream pruningapps/discord-gateway/src/inbound-queue.ts:212-228: XRANGE and HKEYS/HDEL are separate operations. If XRANGE observes an empty stream, then an event is enqueued and delivery records its attempt before HKEYS, oldestId === null causes that live counter to be deleted, resetting retry accounting and allowing more than the configured maximum attempts. Snapshot the tracked IDs before reading the oldest entry or make the cleanup atomic, and cover that ordering in a test.

Reviewed fd390b3

daniel-lxs and others added 3 commits July 16, 2026 09:08
…delivery refactor

#409 replaced the deliveryAbort controller with a delivery object and an
abortDelivery() helper so the forwarder can restart on secret rotation;
the renew-error streak logic now abandons leadership through that helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sure warning

- pruneOrphanedAttempts deletes retry counters whose stream entries were
  shed by the MAXLEN cap (acknowledge/quarantine clean up their own);
  runs on the status refresh timer so the attempts hash stays bounded
  with the stream during a long outage
- the capacity pressure warning moves from lastError — which every
  successful delivery clears — to a dedicated capacityWarning status
  field whose only writer is the refresh timer, and Discord settings
  shows it as an Event backlog diagnostic row

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pruneOrphanedAttempts read the stream first: an event enqueued and
attempted between XRANGE (empty) and HKEYS put a live counter in the
deletion set, resetting its retry accounting past the configured
maximum. Snapshotting the tracked ids first means a counter recorded
after the snapshot can never be pruned; ordering is covered by tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@daniel-lxs
daniel-lxs merged commit 8372b49 into develop Jul 16, 2026
17 checks passed
@daniel-lxs
daniel-lxs deleted the feat/discord-gateway-hardening branch July 16, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant