Skip to content

fix: stop the post-cutover prod ERROR-log spam (ledger scan + sign-up race)#4262

Merged
TaprootFreak merged 4 commits into
developfrom
fix/prd-error-log-spam
Jul 18, 2026
Merged

fix: stop the post-cutover prod ERROR-log spam (ledger scan + sign-up race)#4262
TaprootFreak merged 4 commits into
developfrom
fix/prd-error-log-spam

Conversation

@TaprootFreak

Copy link
Copy Markdown
Collaborator

Problem

Since the ledger cutover (17.07), prod emits ~467 ERROR lines / 60 min. A read-only investigation (Loki + prod DB) attributed them to a few clusters — most are expected/by-design state logged at ERROR every cron cycle, not real failures. #4261 already fixed the largest one (bank_tx BUY_CRYPTO_RETURN head-of-line block) on develop; this PR removes the remaining self-inflicted noise and fixes one real concurrency bug.

What this fixes

1. Ledger content-change scan gate-block (~120/60min)63e5511
The §4.7 G-a gate-block is a designed self-healing retry signal, but runContentChangeScan logged it (and re-scanned the same head-of-line row) at ERROR every cycle. A typed LedgerGateBlockedError is now logged at verbose; every other throw stays ERROR (never swallowed).

2. crypto_input content-change scan spam (~120/60min)9891d03

  • crypto_input N has no asset: an asset-less crypto_input is a FAILED pay-in (never settled) the status-agnostic scan re-selects every cycle → now skipped when non-settled (a settled asset-less row still fails loud).
  • … neither buyFiat/buyCrypto nor isPayment — skip seq0: a legitimate skip logged at error → verbose.

3. Concurrent sign-up address race (~15/60min + a real 500)f9d4d5f
A check-then-act race let the loser's createUser hit 23505 on user.address UNIQUE, surfaced as a raw 500. The guard is consolidated into doSignUp (covers authenticate/signUp/alby/lnurl), keyed on error.code === '23505' → reload + sign the winner in; any other error still propagates.

Deliberately NOT changed (kept visible)

  • External transient errors (Olky socket hang up, blockchain balance updates, CoinGecko blips, IBAN) — measured 5/11/14/0 per 60 min, intermittent, each already caught/handled. Legitimate outage signals; silencing would hide real failures → leave-as-is.
  • No valid price found for EURS → CHF (~77/60min, incl. buy-fiat AML) — a real functional issue, not log noise: price_rule 46 (CoinGecko stasis-eurs) goes stale (validity 300 s) when the thin CoinGecko market intermittently fails, blocking a EURS sell. This is a pricing-source decision (route EURS via EUR, widen validity, or handle the stuck sell), not a log fix — flagged separately, not silenced here.

Testing (Node 20)

type-check clean · eslint clean · prettier applied · the 5 affected specs pass (135/135), incl. new tests: gate-block→verbose vs genuine→error, the non-settled asset-less scan skip, the no-product verbose skip, and the sign-up race (winner signed in) + non-23505 rethrow.

PR completeness

No migration, env/infra, DTO/service, or frontend changes — log-level / skip-guard + one auth concurrency guard only.

…ot error

The §4.7 G-a gate-block is a DESIGNED self-healing retry signal (a late-settling row's opening is not yet booked), but runContentChangeScan logged every thrown error uniformly at ERROR and re-scanned the same head-of-line row every cron cycle — spamming ~120 ERROR/60min on prod after the cutover. Throw a typed LedgerGateBlockedError for the gate-block and log only that at verbose; every other throw stays a genuine scan error at error (never swallowed). No behavioural change to the retry/cursor semantics.
…-spamming on unbookable rows

Two by-design states were logged at ERROR every cron cycle: (1) an asset-less crypto_input (a FAILED pay-in, never settled) hit walletAsset() and threw 'has no asset' on every content-change scan — now skipped (buildSeq0Input returns undefined) when non-settled, while a settled asset-less row still fails loud; (2) a crypto_input with no buyFiat/buyCrypto product and not a payment logged its legitimate seq0 skip at error — now verbose. No functional change to what gets booked.
…stead of a 500

A check-then-act race on a brand-new address let two concurrent sign-ups both pass the existence check and both reach createUser → the loser hit SQLSTATE 23505 on user.address UNIQUE, surfaced as a raw 500 and logged raw duplicate-key noise. Consolidate the guard into doSignUp (scoped to the createUser call), keyed on error.code==='23505', so every wallet-signature caller (authenticate, signUp, alby, lnurl) reloads and signs the winner in; any other error still propagates. Removes the now-redundant catch on authenticate.
…name gate-block exception

1) doSignUp: reload by address before signing the winner in, so a 23505 on the address UNIQUE routes to signIn while a 23505 on any OTHER user unique (e.g. ref) rethrows the original error instead of a misleading NotFound. Adds a non-address-23505 rethrow test and asserts the winner receives a real accessToken.

2) Rename LedgerGateBlockedError -> LedgerGateBlockedException to match the repo's *Exception convention (the file was already .exception.ts).
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Review + validation summary

Two independent reviews (correctness, and hidden-failure/scope/tests) ran on the diff — no blockers. Verified:

  • Gate-block → verbose is safe: the break/cursor-hold is byte-for-byte identical (only the log level changed), every non-LedgerGateBlockedException throw still logs at ERROR, and a genuinely-wedged row still surfaces via the frozen cursor + the daily reconciliation (§7). The two known permanent-wedge causes (preCutoverSettled, isUnpricedAtCutover F2 alarm) are untouched and stay loud.
  • crypto_input skip-guard only fires on a non-settled (FAILED) asset-less pay-in; a settled asset-less row still fails loud. Forward path (status-filtered) is unaffected.
  • Sign-up 23505 guard covers all wallet-signature callers idempotently; the winner's already-verified signature makes signing them in safe.

One minor review finding was addressed in bf7b38b: the 23505 guard now reloads by address before signing the winner in, so a 23505 on any other user unique (e.g. ref) rethrows the original error instead of a misleading NotFound (added a dedicated non-address-23505 rethrow test; the winner test now asserts a real accessToken). Also renamed LedgerGateBlockedErrorLedgerGateBlockedException to match the repo convention.

Validation: type-check, eslint, prettier clean; the 5 affected specs pass (136/136); CI green.

Deliberately not silenced: the external transient errors (intermittent, legitimate outage signals) and — importantly — No valid price found for EURS → CHF, which is a real functional issue (a stale CoinGecko stasis-eurs price blocking a EURS sell), not log noise. That one needs a pricing decision and is out of scope here.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 17, 2026 22:11
@TaprootFreak
TaprootFreak merged commit 97c2580 into develop Jul 18, 2026
7 checks passed
@TaprootFreak
TaprootFreak deleted the fix/prd-error-log-spam branch July 18, 2026 07:33
Danswar added a commit that referenced this pull request Jul 24, 2026
…ose instead of error (#4293)

* fix(accounting): log designed-transient BankTxConsumer states at verbose instead of error

Two designed-transient states in the ledger BankTxConsumer spammed the error
logs on every scan cycle:

- a fresh BUY_CRYPTO / BUY_CRYPTO_RETURN bank_tx scanned before the async
  pricing job has set amountInChf on the linked buy_crypto row — now thrown
  as LedgerGateBlockedException, logged verbose by the forward loop and the
  content-change scan (which already handles it since #4262)
- a bank_tx whose type is still null (not yet classified) hitting the
  default switch branch — now logged verbose; a NON-null unhandled type
  stays at error (genuine enum gap, fail-loud)

Retry and watermark semantics are unchanged: the gate-block still breaks the
forward loop head-of-line (pricing does not bump bank_tx.updated, so that
retry is the only booking path), and the null-type skip still advances the
watermark (classification bumps updated, the content-change scan re-books).

Extends the quiet-retry pattern established in #4262 for the sibling
consumers.

* fix(accounting): gate-block only provably transient unpriced states, keep fail-loud defaults

Narrow the previous commit to the states that provably self-heal:

- BUY_CRYPTO: gate-block only while the linked buy_crypto has no amlCheck
  result yet (doAmlCheck prices amlCheck==null rows). A linked row WITH an
  amlCheck result and no amountInChf (FAIL via chargebackFillUp or
  resetAmlCheck) is never re-priced — that stays a fail-loud Error.
- BUY_CRYPTO_RETURN: gate-block only the unlinked link race (the
  chargebackFillUp cron links the return bank_tx). A LINKED chargeback with
  no amountInChf is permanently unpriced — fail-loud Error.
- Revert the type-null default-branch downgrade: reconcileBooking no-ops on
  never-booked rows, so a skipped null-type row is never booked later — the
  gap must stay visible at error level for all unhandled types.

The forward-loop catch keeps logging LedgerGateBlockedException at verbose;
retry and watermark semantics are unchanged throughout. A two-run test pins
the head-of-line-retry recovery the gate-block relies on.

* fix(accounting): correct gate-block comments and use CheckStatus enum in spec

- forward-loop catch comment now names both gate-block feeders (AML pricing
  pending, chargeback link pending) instead of only the unpriced linked row
- drop the resetAmlCheck attribution from the fail-loud comments: it nulls
  amlCheck and amountInChf together, so it produces the gate-block state,
  never the fail-loud state
- spec uses CheckStatus.FAIL instead of a string literal

* fix(accounting): correct remaining gate-block comments

- the exception doc now names both forward-path feeders precisely
  (BUY_CRYPTO linked but not yet AML-priced; BUY_CRYPTO_RETURN not yet
  linked to its chargeback) instead of 'linked row'
- the type-null test comment no longer attributes reconcileBooking to the
  forward loop: the watermark passed the row and the content-change scan's
  reconcileBooking no-ops on never-booked rows

* fix(accounting): tighten gate-block comment claims and wrap to 120 cols

- 'is never re-priced' overgeneralized: a manual FAIL→PENDING re-trigger
  (allowed while the buy_crypto is not complete) re-prices via doAmlCheck's
  PENDING selector — the comments now say 'not re-priced by the cron'
- wrap the forward-catch and exception doc comments to the 120-column
  convention

* fix(accounting): align chargeback gate comment with the corrected re-pricing claim

The RETURN-side guard comment still carried the categorical 'FAIL rows are
never re-priced' phrase that the previous commit corrected on the BUY_CRYPTO
side. It now names the actual freezing mechanism: the linking cron completes
the row, freezing amlCheck — so a linked chargeback with no CHF is not
re-priced by the cron. The mirrored spec comment is aligned the same way.

* fix(accounting): narrow the gate-block to doAmlCheck-eligible rows, revert the RETURN-leg downgrade

BUY_CRYPTO: amlCheck == null alone does not prove the pricing race -
doAmlCheck only prices rows that also have amlReason null, input
amount/asset set, no user chargeback date and isComplete false. An
ineligible unpriced row would wedge the forward watermark at verbose.
The gate now uses BuyCrypto.isAmlPricingPending, a mirror of the
doAmlCheck selection, with reciprocal keep-in-sync comments.

BUY_CRYPTO_RETURN: buyCryptoChargeback == null does not prove the
fillUp link race - a bank_tx can be classified BUY_CRYPTO_RETURN
without any chargeback ever linking, and chargebackFillUp only links
FAIL/incomplete orders with an executed chargebackOutput. No cheap
provable invariant exists on the consumer side, so the downgrade is
reverted: the leg fails loud again for both sub-cases.
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.

1 participant