Skip to content

NotificationDispatcher records a delivery attempt for an unregistered channel, though nothing was ever sent — the notification-side twin of recordUndeliverable #11860

Description

@os-sam

Measured while implementing the ack() status precondition on #11453 (PR #11858). Filed unassigned; deliberately left alone in that PR, for the reason in the last section.

The site

NotificationDispatcher has two branches that ack a claimed row without attempting a send at all — the channel the row names is not in the registry:

  • dispatcher.ts, processRow()ack(row.id, { success: false, error: "channel '<c>' not registered", dead: true })
  • dispatcher.ts, processDigestGroup() — the same, once per row in the group

ack increments attempts for every accepted call, so the row lands terminal dead with attempts >= 1. Nothing went on the wire. The counter says something was tried.

After #11453 that increment is conditional on the row being claimed, which is what closed the ack-as-cancel corruption route. It does not distinguish "claimed, sent, failed" from "claimed, never sent" — and this branch is the second one.

Why this is the shape recordUndeliverable already answers on the sibling

The HTTP outbox has this exact problem and solved it with a separate member: IHttpOutbox.recordUndeliverable writes a terminal row without incrementing attempts, precisely so attempts === 0 on a terminal row stays a truthful discriminator. assertHttpRedeliverable (http-outbox.ts:315+) then reads it to refuse redelivering a "parked, never sent" row — a security refusal, since such a row carries no HMAC signature (#7799, #8069).

The notification outbox has no such member and no such reader.

Why it was not fixed under #11453

Because nothing on this side consumes the value the way the HTTP side does. Measured on origin/main: the only reader of a notification row's attempts is classifyDeliveryAttempt(result, errorClass, row.attempts, …), which uses it to position the retry backoff — and a dead row is terminal, so its count is never read again.

So closing this today means declaring a discriminator (a new AckFailure field, or a recordUndeliverable twin) with no consumer, which is the capability expansion the #11453 scope ruling was explicitly narrowed to avoid. The honest sequence is the reverse: if a notification-side reader of "terminal but never sent" appears — an operator-facing redeliver, a delivery-health report, an alert that distinguishes misconfiguration from transport failure — it arrives needing this, and the discriminator lands with its consumer.

Recording it now so that reader's author finds the analysis instead of re-deriving it, and so nobody reads the notification attempts column as trustworthy for this distinction in the meantime.

Links

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions