Review incident-notification fanout

Review exercise · TypeScript

Nightmare60 min

Review brief

An at-least-once incident stream triggers immediate notifications to an on-call roster and one escalation. Provider calls can partially fail or outlive the handler, acknowledgements race delivery, and each provider enforces a small concurrency quota.

Behavioral contract

Events are delivered at least once. Persist success per eventId, recipientId, and channel only after provider acknowledgement; a retry skips only those completed deliveries and marks the event complete only when all intended deliveries are complete or explicitly cancelled by incident acknowledgement.
Each provider call uses the stable idempotency key eventId:recipientId:channel. One recipient's failure must not prevent attempts for the other recipients in that event.
Run at most five sends concurrently per provider and join every started send before the handler settles. Reject after the batch with the failed recipient deliveries so the stream retries them.
Check incident acknowledgement immediately before every provider send and again before scheduling escalation. Once acknowledged, cancel the stable escalation id and do not start additional sends.
Schedule exactly one escalation with id incidentId:primary, due at openedAt plus 15 minutes. Redelivery or delayed processing must not move or duplicate it.

Review instructions

1Trace a crash after one provider ack, a partial provider failure, redelivery, and acknowledgement during fanout.
2Anchor each finding to the narrowest useful changed line and explain the incident-response impact.
3Recommend a direction rather than writing replacement code.
4Prioritize no more than six findings.

Learning objectives

• Model fanout progress at recipient granularity
• Coordinate acknowledgement races and stable escalation
• Bound, isolate, and join provider work under partial failure

src/incident-fanout-types.tsTypeScript

@@ -0,0 +1,55 @@

1+export type NotificationChannel = "sms" | "push";

3+export interface IncidentRecipient {

4+ id: string;

5+ address: string;

6+ channel: NotificationChannel;

7+}

9+export interface IncidentOpened {

10+ eventId: string;

11+ incidentId: string;

12+ openedAt: Date;

13+ summary: string;

14+ recipients: readonly IncidentRecipient[];

15+}

16+

17+export interface NotificationProvider {

18+ send(input: {

19+ address: string;

20+ message: string;

21+ idempotencyKey: string;

22+ }): Promise<void>;

23+}

24+

25+export interface IncidentProviders {

26+ forChannel(channel: NotificationChannel): NotificationProvider;

27+}

28+

29+export interface FanoutProgressStore {

30+ isEventComplete(eventId: string): Promise<boolean>;

31+ markEventComplete(eventId: string): Promise<void>;

32+ isDeliveryComplete(

33+ eventId: string,

34+ recipientId: string,

35+ channel: NotificationChannel,

36+ ): Promise<boolean>;

37+ markDeliveryComplete(

38+ eventId: string,

39+ recipientId: string,

40+ channel: NotificationChannel,

41+ ): Promise<void>;

42+}

43+

44+export interface IncidentAcknowledgements {

45+ isAcknowledged(incidentId: string): Promise<boolean>;

46+}

47+

48+export interface EscalationScheduler {

49+ schedule(input: {

50+ id: string;

51+ incidentId: string;

52+ dueAt: Date;

53+ }): Promise<void>;

54+ cancel(id: string): Promise<void>;

55+}

Reference solution

The findings below are ordered by priority.

Findings

1. The event is marked complete before any delivery starts

Location: src/incident-fanout.ts:20-27

markEventComplete runs before the acknowledgement check and provider calls. A crash, provider error, or process shutdown afterward causes redelivery to return at line 21, permanently losing every unfinished notification and escalation.

Use recipient-level completion as the durable progress record and mark the event complete only after all intended deliveries finish or acknowledgement cancels the remainder.

2. All recipients share one provider idempotency key

Location: src/incident-fanout.ts:48-53

Every call uses only event.eventId. A provider that deduplicates globally by key can accept the first SMS or push and suppress every other recipient as a duplicate.

Use the stable composite eventId:recipientId:channel for each delivery and reuse it on retry.

3. Redelivery cannot resume at recipient granularity

Location: src/incident-fanout.ts:20-34 and src/incident-fanout.ts:48-59

Although the store exposes isDeliveryComplete, the handler never reads it. Without the premature event marker, a retry would resend already acknowledged recipients; with that marker, it sends nobody. Neither behavior resumes only the failed subset.

Filter each intended delivery by its durable per-recipient completion record before starting provider work.

4. Fanout ignores provider concurrency limits and does not isolate failure

Location: src/incident-fanout.ts:31-34

Promise.all launches the entire roster at once, even when dozens share one provider, exceeding the maximum of five. It also rejects on the first failure while other sends remain in flight, so the stream can redeliver before all progress writes settle.

Use a per-provider limiter, collect every started outcome, join the batch, then reject with the precise failed deliveries.

5. Acknowledgement is checked only once before the whole fanout

Location: src/incident-fanout.ts:27-38

An incident acknowledged after the initial check still receives every not-yet-started notification and an escalation. The stable cancellation operation is never called on the acknowledged path either.

Recheck immediately before each send and before scheduling, stop new work after acknowledgement, and cancel incidentId:primary idempotently.

6. Escalation timing and identity change on every attempt

Location: src/incident-fanout.ts:35-39

The due time is based on processing time rather than openedAt, so delayed handling postpones escalation. The id contains the current clock and event id, so redelivery schedules another independent escalation instead of converging on the incident's one primary schedule.

Use stable id incidentId:primary and due time openedAt + 15 minutes.

Reasonable non-findings

Storing provider completion only after provider acknowledgement is correct.
Using the incident summary as the provider message is sufficient under this exercise's content contract.
It is acceptable for the handler to reject after all started work joins so the stream retries failed deliveries.
Acknowledgement need not cancel provider calls that were already accepted; it must stop calls not yet started.