Skip to content

fix(core): deliver domain events at least once - #297

Merged
zaxovaiko merged 16 commits into
devfrom
fix/event-delivery
Oct 21, 2026
Merged

zaxovaiko merged 16 commits into
devfrom
fix/event-delivery

Conversation

@zaxovaiko

Copy link
Copy Markdown
Member

Summary

  • Events on money and compliance paths are now written to event_outbox in the same transaction as the change they describe, and the outbox relay always runs. 51 call sites in wallet, gaming, bonus, KYC, RG and player management moved from emit() to emitInTransaction(tx, ...).
  • RedisStreamsBroker ACKs an entry only after every local handler succeeded. A failed entry is redelivered, and if it still fails on its 5th delivery it moves to a dead-letter stream, oss:dlq:<topic>.
  • EventBus.on skips a handler that already handled an event, tracked in a new processed_events table, so a redelivery doesn't run the same side effect twice.
  • Streams are trimmed by age (7 days) instead of by count (10,000 entries).
  • The outbox relay claims rows with a lease, so two API replicas never publish the same row.
  • A payload that fails its domainEventSchemas check throws on emit instead of being published.

Why

Modules don't call each other for side effects. When the wallet credits a deposit, it publishes wallet.deposit.completed, and bonus, notifications and AML react to it. The event goes into a Redis stream (one per topic), and each service reads it through its own consumer group. A consumer group is Redis remembering, per service, which entries it already handled, so every service gets every event once and its replicas share the work.

This is how it worked before:

sequenceDiagram
    participant W as wallet
    participant DB as Postgres
    participant R as Redis stream
    participant B as bonus handler
    W->>DB: COMMIT deposit
    W-->>R: XADD, fire-and-forget
    Note over W,R: process dies or Redis blips here, event is gone
    R->>B: deliver
    B-->>B: throws (DB timeout)
    R->>R: XACK anyway, nobody retries
Loading

That flow could drop or repeat an event in six places:

  1. The event was published after the commit, without waiting for Redis. If the process died between the commit and the XADD, the deposit was saved and nobody heard about it. The outbox that closes this gap already existed, but it was off unless OUTBOX_ENABLED was set, and emitInTransaction had no callers.
  2. EventBus.on caught every handler error, so the broker never saw it and ACKed the entry. A bonus handler that timed out on the database got no second try, and the only trace was a log line.
  3. XADD ... MAXLEN ~ 10000 drops the oldest entries whether a consumer group read them or not. A service that fell 10,000 events behind on a busy topic lost the oldest ones.
  4. Delivery was already at-least-once in one case: a crash before XACK brings the entry back. Only two handlers deduplicated on eventId, so anything else could run twice, a bonus grant included.
  5. The relay's SELECT ... FOR UPDATE SKIP LOCKED committed before publishing, so the lock was gone by the time it published and every replica could pick up the same rows. One failing row also stopped the rest of the batch, and published rows were never deleted.
  6. A payload that failed validation was logged and published anyway, so consumers could get malformed data.

How it works now

sequenceDiagram
    participant W as wallet
    participant DB as Postgres
    participant Relay as OutboxRelay
    participant R as Redis stream
    participant B as bonus handler
    W->>DB: BEGIN, update balance, insert event_outbox row, COMMIT
    loop every 200ms
        Relay->>DB: claim pending rows (30s lease)
        Relay->>R: XADD
        Relay->>DB: set published_at
    end
    R->>B: deliver
    alt handler succeeds
        B->>DB: insert processed_events row
        R->>R: XACK
    else handler throws
        R->>R: entry stays pending, redelivered later
    end
Loading

emitInTransaction(tx, topic, payload) inserts the event into event_outbox inside the caller's transaction. If the transaction rolls back, the event never existed. The relay picks pending rows up every 200ms with UPDATE ... SET claimed_at = now() ... FOR UPDATE SKIP LOCKED RETURNING, so the claim and the lock are one statement. A claim is a 30s lease: if a replica dies mid-publish, another one takes the row once the lease runs out. A row that fails to publish releases its lease and the rest of the batch carries on. Published rows and processed_events older than 7 days are pruned once an hour.

On the reading side, every entry goes through this:

flowchart TD
    A[Entry read from stream] --> P{Valid JSON and payload?}
    P -- no --> J
    P -- yes --> B{processed_events has this event for this handler?}
    B -- yes --> S[Skip the handler]
    B -- no --> C[Run the handler]
    C -- ok --> D[Insert into processed_events]
    C -- throws --> E[Leave the entry pending]
    D --> F{Every handler done?}
    S --> F
    F -- yes --> G[XACK]
    F -- no --> E
    E --> H{5th delivery?}
    H -- no --> I[XAUTOCLAIM picks it up after 60s idle]
    I --> A
    H -- yes --> J[Copy to oss:dlq:topic, then XACK]
Loading

The dedup key is (event_id, consumer). Each plugin handler gets a consumer id <service>/<plugin>/<topic>, with #n added for a second handler on the same topic in one plugin. One consumer group can run several handlers for a topic, so when one of them fails the whole entry comes back, and processed_events is what stops its siblings from running again. The DLQ entry keeps the group, stream id, delivery count and last error, so it's possible to tell what broke and replay it.

Retention is MINID ~ now - 7 days on every XADD. A slow consumer keeps everything from the last week, however many entries that is.

ADR-0044 records the decisions. ADR-0017 and ADR-0030 got update notes where this supersedes them.

Testing

  • redis-streams-broker.int.test.ts covers a failing handler leaving the entry pending until a redelivery succeeds, dead-lettering after 5 deliveries, bad JSON going straight to the DLQ, the age-based trim, and recent entries surviving past the old 10,000 cap. The two trim tests fail against the old MAXLEN trimming.
  • event-bus.int.test.ts covers a duplicate delivery running a handler once, only the failed handler re-running on redelivery, an invalid payload throwing on emit and emitInTransaction, an invalid inbound payload landing in the DLQ, and a rolled-back emitInTransaction publishing nothing.
  • relay.int.test.ts covers two concurrent relays publishing each row once, a failing row not blocking the batch, the lease, and pruning.
  • The wallet, gaming, bonus, compliance and player service tests now assert that their events go through emitInTransaction.
  • pnpm verify passes.

Alternatives considered

  • Swap Redis Streams for RabbitMQ or Kafka. They come with dead-lettering and disk-backed retention, but every deployment would need another piece of infra, and neither fixes the gap between commit and publish or duplicate handling. The broker seam stays open for anyone who wants that.
  • One consumer group per handler instead of per service, so a failing handler only retries itself. That multiplies the Redis read loops by the number of handlers; processed_events gets the same effect with one table.
  • Run the handler and the processed_events insert in one transaction, for exactly-once. Handlers don't get a tx today and some call external services, so it stays check, run, record.

Risks

  • Dedup is check, run, record. If a reclaim races a slow handler, two replicas can run the same handler for the same event. Money handlers still need their own database guard, like a unique key or a conditional update.
  • Events that moved to the outbox now reach subscribers up to about 200ms after the commit. A flow or test that reads a side effect straight away needs to wait for it; game-geo-blocking needed a vi.waitFor.
  • A handler that keeps failing retries for about 5 minutes before its entry lands in oss:dlq:<topic>. The DLQ is never trimmed and has no tooling yet, so for now it's drained by hand with XRANGE.
  • A consumer group more than 7 days behind still loses entries. Worth alerting on group lag.
  • The #n consumer ids come from registration order, so reordering two handlers on one topic in a plugin re-runs in-flight events once.
  • bus.on called outside a plugin has no consumer id and isn't deduped.
  • emit() now throws on an invalid payload. On a post-commit emit that happens after the commit, so the caller sees an error for a change that did save.
  • OUTBOX_ENABLED is gone and the relay always runs. Migration 0001_event_delivery on the outbox history adds processed_events and event_outbox.claimed_at, so downstream needs db:migrate after the bump.
  • Gamification payouts still use emit(), because their transaction is spread across 4 services. Not included in this PR.
  • The changeset follows in a separate PR, per .changeset/README.md.

…laim outbox rows atomically

The Redis Streams broker acks an entry only once every local handler succeeded, reclaims idle pending entries with XAUTOCLAIM and dead-letters to oss:dlq:<topic> after 5 deliveries. Streams trim by MINID (7 days) instead of MAXLEN. The outbox is always bound; the relay claims rows with a leased UPDATE ... SKIP LOCKED, releases a failing row without blocking the batch and prunes old rows. processed_events dedups each plugin handler by (event_id, consumer). Known event payloads are validated on emit and on receive.
Wallet, swap, reconciliation, gaming, bonus lifecycle, KYC, RG, player status and admin geo-rule events are written with emitInTransaction so they exist if and only if the state change commits. Bonus expiry and forfeit are emitted once, by the grant lifecycle service.
@klaudia-blazyczek-blurify

Copy link
Copy Markdown
Collaborator

/codex all

@blurify-codex blurify-codex Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker: a transient audit write failure can leave a committed money or RG change without an audit record. Blocker: first-time Redis groups can skip retained events, and a swallowed bonus enqueue failure is recorded as handled.

Reviewed security, compliance, conventions, performance, reliability, rollout, spec, the whole change.

Handlers that fired promises and returned early when their service was not yet
constructed were acked as processed even when the work failed. They now return
or await the work and throw when a required dependency is missing, so the broker
retries and dead-letters instead of dropping the event.
Withdrawal approve/reject and KYC events looked up the player id on a second pool
connection while holding row locks. The identity reader now accepts the caller's
tx, and the cooling-off sweep resolves player ids before opening its transaction.
The relay keeps claiming while batches come back full and marks a batch published
in one update; the lease compares against the DB clock. Reclaim walks the whole
pending list per interval instead of one page. processed_events outlives the
stream retention by a day so a late redelivery still dedupes.
…ciliation runs

gaming.round.started is written after the provider launches the game, as before
the outbox change, so a failed launch announces nothing. Reconciliation records
its audit entry and threshold alert after the run is marked finished, so a failed
alert write no longer rewrites a completed run as failed.
@klaudia-blazyczek-blurify

Copy link
Copy Markdown
Collaborator

/codex all

@blurify-codex blurify-codex Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker: startRound can debit a player and launch a game without recording the round-start event. Blocker: a failed event write after a successful game launch can turn a retry into a second bet.

Reviewed security, compliance, conventions, performance, reliability, rollout, spec, changes since ae3b1f2.

@zaxovaiko

Copy link
Copy Markdown
Member Author

Not mergeable yet.

Addressed by later commits: audit and bonus enqueue now await (5b6d4c2), player id lookup on the caller's tx (60f8887), relay drain loop and full reclaim walk (99480c2), gaming.round.started only after launch (466dc43).

Still open, no fix or reply yet:

  • gaming.service.ts:695-704 (threads by codex and @klaudia-blazyczek-blurify on 466dc43): the debit commits, then launchGame, then emitInTransaction(this.drizzle.db, ...) outside the debit tx. If that insert fails the route errors after a committed debit and a live provider session, and a retry mints a new roundId and debits again. On a money path: make the post-launch emit non-failing for the caller (catch and log, or write it in a second tx keyed on round.id and return the launch), and add a test where the outbox insert throws after launch and the player is debited once.
  • create-app.ts:413: new RedisStreamsBroker(redis, { serviceName }) never passes startId, so a new group still starts at $ and a newly split service skips the retained backlog (codex thread on ae3b1f2). Either default new groups to 0 (safe now that handlers dedupe) or document why $ is intended and reply on the thread.
  • relay.ts:12 (threads by @klaudia-blazyczek-blurify on ae3b1f2 and codex on 466dc43): processed_events retention of 7d + 1d still loses to MINID ~ plus a pending entry that outlives it, so a late reclaim re-runs succeeded handlers. Either drop pending entries older than EVENT_RETENTION_MS on reclaim (ACK + DLQ) or key retention off the oldest pending id. Add a test for a reclaim past retention.
  • relay.ts:96-110 (thread by @klaudia-blazyczek-blurify on 466dc43): published_at is set only after the whole batch, so a batch slower than the 30s lease is republished by another replica. Mark rows as they publish, or cap the batch publish time below CLAIM_LEASE_MS.
  • reconciliation.service.ts:505 (codex on 466dc43): a completed run whose audit or alert write fails has no audit row and no retry. Reply with the intended behaviour, or retry the audit/alert write for a completed run.

Reply on each thread once fixed.

# Conflicts:
#	packages/core/src/engagement/notifications/plugin.ts
#	packages/core/src/pam/player-management/service/player.service.ts
#	packages/core/src/server/db/index.ts
#	packages/core/src/server/db/outbox/drizzle/migrations/meta/0001_snapshot.json
#	packages/core/src/server/db/outbox/drizzle/migrations/meta/_journal.json
#	packages/core/src/server/db/outbox/schema.ts
#	packages/core/src/server/plugin-host/index.ts
#	packages/core/src/server/runtime/create-app.ts
#	packages/core/src/wallet/service/wallet.service.ts
@zaxovaiko
zaxovaiko merged commit 43c1c75 into dev Oct 21, 2026
7 checks passed
@zaxovaiko
zaxovaiko deleted the fix/event-delivery branch October 21, 2026 09:31
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.

3 participants