From a631c8c3c361841f2c3c53f9c26382bfcbea9293 Mon Sep 17 00:00:00 2001 From: Renato Alcara Date: Sun, 26 Apr 2026 09:59:54 -0300 Subject: [PATCH] perf(inbound-latency): restore async LID mapping + fire-and-forget tctoken history sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRODUCTION ISSUE: Inbound messages from smartphone to ZPRO frontend were arriving with seconds of delay. Outbound (ZPRO → smartphone) was instant. Started after PR #386 (tctoken lifecycle) deploy. ROOT CAUSE: Three compounding factors: 1. The historical fix d73cd28d39 (2026-02-03, "fix inbound latency by making LID mapping async") was partially reverted the same day by c3fc792351 ("hybrid approach") due to a valid race-condition concern with decrypt(). The reversion was over-protective: storeLIDPNMappings does NOT need to be sync — only migrateSession does. The hybrid kept all 3 awaits sync. 2. PR #386 added `await storeTcTokensFromHistorySync(...)` BEFORE the `messaging-history.set` emit. Per chunk this drains the event buffer with 2-4 store ops, which compounds when many chunks arrive at once (restart, QR scan, multi-device login). 3. Each pre-check `await getPNForLID(alt)` / `getLIDForPN(alt)` before storeLIDPNMappings was redundant — the store has its own LRU cache + dedup. Combined under production load (multi-instance store contention, post-PR #386 extra ops per send) the per-message hot-path penalty became user-visible delay. THIS FIX: #1+#3: messages-recv.ts ~line 2332 — `storeLIDPNMappings` becomes fire-and-forget, pre-check `getPNForLID/getLIDForPN` removed. `migrateSession` stays SYNC (REQUIRED for decrypt — see Codex/Copilot review on PR #72 / commit c3fc792351). normalizeMessageJids has a fast-path that uses key.*Alt directly without hitting the store, so the just-arrived message normalizes correctly even before the background store completes. #2: process-message.ts ~line 451 — `storeTcTokensFromHistorySync` becomes fire-and-forget. Trade-off: a listener firing an outbound send IMMEDIATELY after the emit may race the background persistence and hit error 463 on that specific send. Existing 463 handler in messages-recv.ts triggers getPrivacyTokens() refetch that auto-recovers in seconds. Net UX is much better than per-chunk stalls. INVARIANTS PRESERVED: - migrateSession remains SYNC — decrypt() depends on it (race condition guard) - normalizeMessageJids remains SYNC — events need correct JIDs before emit - messageMutex remains SYNC — per-chat ordering preserved - All 824 tests still pass DOCUMENTATION: Full rationale, before/after code, test scenarios, rollback procedure and guidance for future maintainers in: Downloads/InfiniteAPI-Inbound-Latency-Fix-Documentation.md DO NOT REVERT WITHOUT READING THE DOC. Customizations untouched: zero diff in src/Utils/messages.ts (carousel/buttons/ lists), src/Socket/groups.ts, WAProto/*. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Socket/messages-recv.ts | 63 +++++++++++++++++++----------------- src/Utils/process-message.ts | 21 +++++++++--- 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/Socket/messages-recv.ts b/src/Socket/messages-recv.ts index 04665b2d..52a742dd 100644 --- a/src/Socket/messages-recv.ts +++ b/src/Socket/messages-recv.ts @@ -2330,44 +2330,49 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => { ) const alt = msg.key.participantAlt || msg.key.remoteJidAlt - // Handle LID/PN mappings with hybrid approach: - // - Store mapping operation runs in background (non-critical for decrypt) - // - Session migration MUST complete before decrypt() to avoid "No session record" errors - // This addresses Codex/Copilot review concerns about race conditions with decrypt() + // Handle LID/PN mappings with optimized hot-path: + // - storeLIDPNMappings is fire-and-forget (background) — does NOT block decrypt + // - migrateSession is SYNC (await) — REQUIRED for decrypt to find session + // + // SAFETY: normalizeMessageJids has a fast-path that uses key.*Alt directly without + // hitting the store, so the just-arrived message normalizes correctly even before + // the background store completes. Subsequent messages in the same chat hit the + // store after the background write is done (ms later). + // + // Pre-check (getPNForLID/getLIDForPN) was removed — storeLIDPNMappings has internal + // LRU cache + dedup, the pre-check was a redundant store round-trip per inbound + // message that added latency under load. + // + // HISTORICAL: this restores the intent of d73cd28d39 (2026-02-03) which was + // partially reverted by c3fc792351 the same day due to a race-condition concern + // with migrateSession (kept sync here). storeLIDPNMappings was over-protected. + // + // DO NOT make migrateSession async — decrypt() depends on it. + // See Downloads/InfiniteAPI-Inbound-Latency-Fix-Documentation.md for full context. if (!!alt) { const altServer = jidDecode(alt)?.server const primaryJid = msg.key.participant || msg.key.remoteJid! if (altServer === 'lid') { - // Check if mapping already exists to avoid unnecessary storage operations - const existingMapping = await signalRepository.lidMapping.getPNForLID(alt) - if (!existingMapping) { - // MUST await: normalizeMessageJids() runs after this and needs the mapping - // in the LIDMappingStore to resolve LID→PN for events delivered to consumers - await signalRepository.lidMapping - .storeLIDPNMappings([{ lid: alt, pn: primaryJid }]) - .catch(error => logger.warn({ error, alt, primaryJid }, 'LID mapping storage failed')) - } + // Fire-and-forget: storeLIDPNMappings has internal cache+dedup, + // pre-check (getPNForLID) was redundant. + signalRepository.lidMapping + .storeLIDPNMappings([{ lid: alt, pn: primaryJid }]) + .catch(error => logger.warn({ error, alt, primaryJid }, 'background LID mapping store failed')) - // CRITICAL: ALWAYS migrate session, even if mapping exists - // Other code paths (e.g., USync device lookup in messages-send.ts:310-319) - // may create mappings via storeLIDPNMappings() without calling migrateSession() - // This leaves sessions under PN format while decrypt() expects LID format - // Skipping migration based on mapping existence causes "No session record" errors + // CRITICAL: ALWAYS migrate session SYNC, even if mapping exists. + // Other code paths (e.g., USync device lookup in messages-send.ts) may create + // mappings via storeLIDPNMappings() without calling migrateSession(). This + // leaves sessions under PN format while decrypt() expects LID format. + // Skipping migration based on mapping existence causes "No session record" errors. await signalRepository.migrateSession(primaryJid, alt) } else { - // Check if reverse mapping exists - const existingMapping = await signalRepository.lidMapping.getLIDForPN(alt) - if (!existingMapping) { - // MUST await: normalizeMessageJids() runs after this and needs the mapping - // in the LIDMappingStore to resolve LID→PN for events delivered to consumers - await signalRepository.lidMapping - .storeLIDPNMappings([{ lid: primaryJid, pn: alt }]) - .catch(error => logger.warn({ error, alt, primaryJid }, 'LID mapping storage failed')) - } + // Fire-and-forget: same rationale as above. + signalRepository.lidMapping + .storeLIDPNMappings([{ lid: primaryJid, pn: alt }]) + .catch(error => logger.warn({ error, alt, primaryJid }, 'background LID mapping store failed')) - // CRITICAL: ALWAYS migrate session, even if mapping exists - // Same reasoning as above - mapping existence doesn't guarantee session migration + // CRITICAL: ALWAYS migrate session SYNC. await signalRepository.migrateSession(alt, primaryJid) } } diff --git a/src/Utils/process-message.ts b/src/Utils/process-message.ts index df77a801..e76b64e8 100644 --- a/src/Utils/process-message.ts +++ b/src/Utils/process-message.ts @@ -565,11 +565,22 @@ const processMessage = async ( } } - // Persist tctokens carried by history-sync chats BEFORE emitting messaging-history.set - // — listeners may immediately fire outbound sends that need the tctoken, and the store - // has to be populated first to avoid an error 463 on the first multi-device send. - // Runs AFTER storeLIDPNMappings (see comment above) so LID resolution works. - await storeTcTokensFromHistorySync(data.chats, signalRepository, keyStore, logger) + // Persist tctokens carried by history-sync chats in BACKGROUND. + // Originally awaited (PR #386) to avoid 463 on first multi-device send, but in + // production this drains the event buffer per-chunk and adds visible delivery + // latency (especially after restart / QR scan when many chunks arrive at once). + // + // TRADE-OFF: a listener that fires an outbound send IMMEDIATELY after the emit + // may race the background persistence and get a 463 on that specific send. The + // existing 463 handler in messages-recv.ts triggers a getPrivacyTokens() refetch + // that auto-recovers within seconds. Net result is much better UX than per-chunk + // stalls. + // + // DO NOT add `await` back here without re-evaluating production latency. + // See Downloads/InfiniteAPI-Inbound-Latency-Fix-Documentation.md for full context. + storeTcTokensFromHistorySync(data.chats, signalRepository, keyStore, logger).catch(err => + logger?.warn({ err }, 'background tctoken history-sync persistence failed') + ) ev.emit('messaging-history.set', { ...data,