jsconsumer: stop the schedule check failing consumers that adopted nothing

main

Commit

nodo3w ago

The schedule validator ran for every consumer, and Schedule read a negative MaxDeliver as an error rather than as this module's own spelling of unlimited. So MaxDeliver -1 — accepted eleven lines earlier by the same validate, returned verbatim by EffectiveMaxDeliver, and pinned by an existing test as "unlimited stays unlimited" — became a permanent hard failure at Start and Run, for a durable carrying no Retry and no ladder. A validator this package added took down consumers that had adopted none of it.

Fixed at both ends, because either alone would leave the other latent: Schedule now treats non-positive MaxDeliver as unlimited (only < -1 is nonsense) and skips the duration checks, which have no terminal branch to measure; and Config only runs the check when a Retry or a BackOff is actually configured. A consumer that opted into neither gains no new way to fail.

The check also moved after the Retry-vs-consumer MaxDeliver cross-check, so a genuine mismatch reports as itself instead of as generic incoherence.

Also from the same review:

  • Start now re-runs the schedule against the stream's real max_age. A ladder that outlives retention never reaches its dead-letter branch — the stream discards the message first, which is silent data loss wearing a retry policy. Config.validate cannot see it, and the README was advertising a check that only ever ran in fleet CI. Unreadable stream info stays non-fatal: the declarative-provisioning race is Run's to ride out.
  • shouldQuarantine still described condition 3 as coming "from its own delivery count against the ladder (message-local, exact)" — the inference removed two commits ago, and "exact" backwards for a clock that resets on restart and is deliberately late.
  • The observe-only log fired only on the ladder branch, so a trip coinciding with exhaustion set BreakerTripped but logged nothing, leaving log-based counting short of the metric. It now reports wherever the trip happens.
  • sweepFailuresLocked rescanned the whole map per victim while holding the mutex every Settle waits on. Ordered once instead.
  • Documented that a trip spends its quarantine claim when the condition is met rather than when the capture succeeds, and why: the alternative lets a broken DLQ re-trip against message after message.

The Retry doc's first-adopter wiring is on the boundary where the breaker can only fire on the delivery exhaustion would have handled anyway. Left as-is — it is the real shipping config — but now says so, and says what it means for the observe soak: trips that all coincide with exhaustion look like evidence and are not. The consumer worth soaking is one with a long ladder.

Refs ENT-1601, ENT-1535.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com Claude-Session: https://claude.ai/code/session_01AJNdqGnT92NC2TRnpf8MNr Entire-Checkpoint: d12ddc2c0816

Checkpoints

Fix NATS Consumer Retry Ladder Implementation

Claude CodeFable 5
View session
Checkpoint 1