ruicesar/InfiniteAPI
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

perf(inbound-latency): restore async LID mapping + fire-and-forget tc… (#390)

Browse Source 
* perf(inbound-latency): restore async LID mapping + fire-and-forget tctoken history sync

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
This commit is contained in:
Renato Alcara
2026-04-26 10:30:09 -03:00
committed by GitHub
parent 4fe708445a
commit 1dffe3b311
2 changed files with 102 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Socket/messages-recv.ts
+39 -29
View File
@@ -2330,44 +2330,54 @@ 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:
// it persists a mapping that downstream consumers can re-derive from key.*Alt,
// while migrateSession actually moves the Signal session record that decrypt()
// will load microseconds later — those two have very different criticality.
//
// DO NOT make migrateSession async — decrypt() depends on the session being at
// the correct identifier (LID vs PN) when it runs. Other code paths (USync
// device lookup in messages-send.ts) create LID/PN mappings without migrating
// the session, so we cannot skip migration even when the mapping already exists.
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)
}
}