From 56ca70bd756362b893017ecfb3d32585a780edd3 Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Wed, 29 Apr 2026 01:22:10 -0300 Subject: [PATCH] perf(decrypt): fail-fast on session-record errors to clear pipeline backpressure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRODUCTION BUG (still present after PR #396): inbound messages from the smartphone take ~60s to surface in the consumer (zpro). PR #396 fixed the PN/LID lock-vs-storage drift but did NOT clear the latency. Diff against the known-good Pedro snapshot pinpointed the residual cause: this retry wrapper. ROOT CAUSE: `DECRYPTION_RETRY_OPTIONS.shouldRetry` retries 3× with exponential backoff (200 ms → 400 ms → 800 ms ≈ 1.4 s per failed message) on `'No matching sessions found'` and friends. After WhatsApp's LID/DSM rollout own DSM messages flood in for sessions stored under the legacy `_1.0` format that no longer matches the LID-addressed envelope, so EVERY DSM hits this path. At ~30 DSM/min the accumulated backoff is ~42 s — enough to block real-contact messages from reaching the buffer flush. The Pedro snapshot (Feb 6 2026, 484 commits behind, confirmed fast in prod on the same auth state) has NO retry wrapper at all: try once → fail → send retry receipt → phone re-sends as `pkmsg` → fresh session → next message decrypts cleanly. Total recovery ~300 ms, no pipeline backpressure. WHY THE RETRIES WERE USELESS: 1. libsignal already scanned every stored session for the JID before throwing `No matching sessions found`. Re-running the same lookup 200 ms later gives the same answer — no new session record materialises in that window. 2. Bad MAC / counter errors mean the keys are simply wrong; retry doesn't regenerate keys. (This branch was already correctly returning false.) THIS PATCH: - `sessionRecordErrors` now also returns `false` from `shouldRetry` (matches the existing `corruptedSessionErrors` policy). - Unknown errors retry exactly once (was twice) — quick blip recovery only. - `maxAttempts` lowered to 2 to match. - Big block comment captures the rationale so the next person doesn't add retries back hoping it'll help. Recovery still happens — just upstream, via the retry-receipt → pkmsg flow, which is what WhatsApp protocol intends and what Pedro's working snapshot relies on. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Utils/decode-wa-message.ts | 50 ++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/src/Utils/decode-wa-message.ts b/src/Utils/decode-wa-message.ts index 5ebf5520..d61c8b3c 100644 --- a/src/Utils/decode-wa-message.ts +++ b/src/Utils/decode-wa-message.ts @@ -67,33 +67,59 @@ export const DECRYPTION_RETRY_CONFIG = { } /** - * Retry options for decryption operations - * Uses exponential backoff with jitter to handle transient failures + * Retry options for decryption operations. + * + * IMPORTANT — fail-fast policy for decryption: + * Both `sessionRecordErrors` ('No matching sessions found', etc.) and + * `corruptedSessionErrors` ('Bad MAC', 'MessageCounterError', missing keys) + * return `false` from `shouldRetry`. Rationale: + * + * 1. libsignal already scanned ALL stored sessions for the JID before + * throwing — retrying immediately gives the SAME result (no new session + * record materialises in the 200-800ms backoff window). + * 2. The real recovery flow is upstream: failed decrypt → retry receipt to + * WA → phone re-sends as `pkmsg` → libsignal builds a fresh session → + * next message decrypts cleanly. That handshake takes ~300ms total. + * 3. Wrapping decrypt in 3 attempts × exponential backoff (200ms→400ms→800ms + * ≈ 1.4 s per failed message) just blocks the inbound buffer pipeline. At + * the rate own DSM messages flood in after a LID/PN mismatch, this + * compounds to tens of seconds of accumulated delay before live messages + * from real contacts can even reach the consumer. + * + * Only truly *unknown* errors get a single retry — those might be transient + * (network blip, unexpected exception) and a quick 200ms retry is cheap. + * + * If we ever need transient-error retries again (e.g. the storage layer adds + * an async race that benefits from re-reading), set `sessionRecordErrors` to + * `attempt < 1` here, NOT `attempt < 3` — one extra read at most. */ export const DECRYPTION_RETRY_OPTIONS: RetryOptions = { - maxAttempts: 3, - baseDelay: 200, // 200ms base delay - maxDelay: 2000, // 2s max delay + maxAttempts: 2, + baseDelay: 200, // 200ms base delay (only used for unknown errors below) + maxDelay: 2000, backoffStrategy: 'exponential', backoffMultiplier: 2, - jitter: 0.2, // 20% jitter - collectMetrics: false, // No Prometheus metrics + jitter: 0.2, + collectMetrics: false, operationName: 'message_decryption', shouldRetry: (error: Error, attempt: number) => { const errorMsg = error?.message || '' - // Always retry on session record errors (session might be syncing) + // Session record errors: libsignal already exhausted all stored sessions. + // Retrying immediately gives the same result; the real recovery path + // is the upstream retry-receipt → pkmsg flow. Fail fast. if (DECRYPTION_RETRY_CONFIG.sessionRecordErrors.some(err => errorMsg.includes(err))) { - return attempt < 3 // Retry up to 3 times + return false } - // Don't retry on corrupted session errors (need cleanup first) + // Corrupted session errors: Bad MAC / counter errors. Same reasoning — + // the keys are wrong and won't right themselves on retry. if (DECRYPTION_RETRY_CONFIG.corruptedSessionErrors.some(err => errorMsg.includes(err))) { return false } - // Retry other transient errors - return attempt < 2 // Retry up to 2 times for unknown errors + // Unknown errors: one retry in case it was a transient blip. + return attempt < 1 } }