5f5b08bfda
Root cause: recordGrowEvent returned the existing row on dedupe hits, and
routeGrowEventToUserActor was called unconditionally — so every duplicate
ingest re-enqueued the same event to the actor queue, saturating it.
Fix at the persistence boundary:
- recordGrowEventWithResult returns {event, inserted} using atomic
onConflictDoNothing().returning() (row present = fresh insert, no row =
dedupe hit). No clock heuristics, no xmax probe.
- Conflict lookup tries dedupeKey first, then id PK fallback, covering both
constraint types.
- shouldRouteGrowEvent(event, inserted) gates routing to only newly-inserted,
user-resolved, pending events. Dedupe hits never re-enqueue.
- recordGrowEvent kept as thin .event wrapper — zero changes to ~10 callers.
Fix the actor run loop (mirrors workflow-run-actor.ts):
- tryStep with catch:['timeout','exhausted'] replaces step+rethrow. Per-event
failure is marked terminal in a separate record-failure step, loop advances.
- isProcessableStatus guard allows pending/processing/processed — only skips
failed/unresolved. This ensures mission reducers (actor-only) still run on
events that got synchronous projections.
- Status guard + claim moved inside tryStep's run for atomic retry unit.
- onError handler added to workflow().
Ingress callers migrated (3 paths):
- redis-consumer recordAndRoute: shouldRouteGrowEvent gate + always mark
processed after projections.
- routes/events.ts ingest: shouldRouteGrowEvent gate + always mark processed.
- v1/events/events-routes.ts /track: inserted flag for dedupe skip.
Tests: 3 new (event-dedupe-routing, user-event-actor-advance,
event-ingress-route-integration with injectable enqueue proving 5x duplicate
ingest enqueues exactly once). All existing touched-scope tests green.
Co-authored-by: independent review (2 rounds)
98 lines
5.2 KiB
TypeScript
98 lines
5.2 KiB
TypeScript
import assert from "node:assert/strict";
|
|
import { isProcessableStatus } from "../src/actors/events/user-event-actor.js";
|
|
|
|
/**
|
|
* Regression: one failed event must not block the next event in the queue.
|
|
*
|
|
* Before the fix, the actor's run loop used loopCtx.step + manual try/catch
|
|
* that rethrew on failure. The rethrow stalled the ctx.loop, preventing
|
|
* subsequent queued messages from being processed.
|
|
*
|
|
* After the fix, the loop uses tryStep (failures caught, event marked terminal)
|
|
* and a DB status guard (isProcessableStatus) that only skips truly inert rows.
|
|
*
|
|
* Dual-path rationale: the synchronous ingress paths (routes/events.ts,
|
|
* redis-consumer.ts, v1/events/events-routes.ts) run projections and mark the
|
|
* event "processed". The actor runs mission reducers (stage patches, artifacts,
|
|
* actions) that ONLY exist in the actor. So the actor must still process
|
|
* "processed" events to run mission reducers — projections are idempotent so
|
|
* re-running them is safe, and the in-memory processedEventIds guard prevents
|
|
* double-processing within one actor lifetime. The routing gate
|
|
* (shouldRouteGrowEvent) prevents saturation from duplicate ingests; this guard
|
|
* is defense-in-depth that only blocks "failed" and "unresolved".
|
|
*/
|
|
|
|
// ── 1. Pending → processable (first attempt) ─────────────────────────────────
|
|
assert.equal(isProcessableStatus("pending"), true, "pending events must be processable");
|
|
|
|
// ── 2. Processing → processable (crash recovery: retry idempotent work) ──────
|
|
// A prior attempt called markGrowEventProcessing then crashed before completing.
|
|
// The actor restarts, replays the queue message, and must retry.
|
|
assert.equal(isProcessableStatus("processing"), true, "processing events must be retryable after a crash");
|
|
|
|
// ── 3. Processed → processable (actor must run mission reducers) ─────────────
|
|
// The sync ingress path marks "processed" after projections. The actor still
|
|
// needs to run mission reducers — the only place they exist. Projections
|
|
// re-running is safe (idempotent). processedEventIds prevents in-memory dupes.
|
|
assert.equal(
|
|
isProcessableStatus("processed"),
|
|
true,
|
|
"processed events must be processable — actor must run mission reducers that only exist in the actor",
|
|
);
|
|
|
|
// ── 4. Failed → NOT processable (terminal, do not retry) ─────────────────────
|
|
// A per-event failure was already marked terminal by the record-failure step.
|
|
assert.equal(isProcessableStatus("failed"), false, "failed events must be skipped");
|
|
|
|
// ── 5. Unresolved → NOT processable (no userId, nothing to do) ───────────────
|
|
assert.equal(isProcessableStatus("unresolved"), false, "unresolved events must be skipped");
|
|
|
|
// ── 6. Null/undefined → NOT processable (defensive) ──────────────────────────
|
|
assert.equal(isProcessableStatus(null), false, "null status must be skipped");
|
|
assert.equal(isProcessableStatus(undefined), false, "undefined status must be skipped");
|
|
|
|
// ── 7. Unknown status → NOT processable (defensive) ──────────────────────────
|
|
assert.equal(isProcessableStatus("unknown"), false, "unknown status must be skipped");
|
|
|
|
// ── 8. Queue advancement simulation ──────────────────────────────────────────
|
|
// Queue: [pending, processed, pending]. Actor processes ALL three — the
|
|
// "processed" one still gets mission reducers. The key point: none of them
|
|
// block the next. This proves one event does not stall the queue.
|
|
{
|
|
const statuses = ["pending", "processed", "pending"] as const;
|
|
const processable = statuses.map((s) => isProcessableStatus(s));
|
|
assert.deepEqual(
|
|
processable,
|
|
[true, true, true],
|
|
"actor must process all three — processed event still runs mission reducers, none block the next",
|
|
);
|
|
}
|
|
|
|
// ── 9. Failed event does not block the next ──────────────────────────────────
|
|
// Queue: [failed, pending]. Actor skips the failed one and processes the next.
|
|
// This is the core regression: one failed event must not block subsequent events.
|
|
{
|
|
const statuses = ["failed", "pending"] as const;
|
|
const processable = statuses.map((s) => isProcessableStatus(s));
|
|
assert.deepEqual(
|
|
processable,
|
|
[false, true],
|
|
"actor must skip failed event and process the next pending event — failed does not block",
|
|
);
|
|
}
|
|
|
|
// ── 10. Crash-recovery queue simulation ──────────────────────────────────────
|
|
// Queue: [processing (crashed), pending]. Actor retries #1 and processes #2.
|
|
{
|
|
const statuses = ["processing", "pending"] as const;
|
|
const processable = statuses.map((s) => isProcessableStatus(s));
|
|
assert.deepEqual(
|
|
processable,
|
|
[true, true],
|
|
"actor must retry a crashed 'processing' event AND process the next pending event",
|
|
);
|
|
}
|
|
|
|
console.log("user-event-actor-advance tests passed");
|
|
process.exit(0);
|