Compare commits

..

2 Commits

Author SHA1 Message Date
Renato Alcara 9e3af35b54 fix(history-sync): serialise tcToken persistence + drop external doc reference
Addresses 4 PR #390 review findings (Codex P1 + Copilot ×3):

#1 Codex P1 (real, MAJOR) — `storeTcTokensFromHistorySync` fire-and-forget
   created a write race when multiple history-sync chunks arrived in parallel
   (common during reconnect/QR scan). The function does a non-atomic
   read-then-merge-then-write: chunk B could read `existing` before chunk A
   wrote, then commit later and overwrite chunk A's newer timestamp. Even
   worse, the merged `__index` write could drop JIDs added by the other
   chunk, leaving stale tcTokens and persistent 463 send errors.

   FIX: introduced `scheduleHistoryTcTokenSync()` — a module-scoped
   single-concurrency promise chain that sequentialises all calls in order.
   The chain swallows prior errors so a single failure can't stall future
   runs. Call site stays fire-and-forget so the `messaging-history.set`
   emit is still instant (which is the whole point of PR #389).

#2 Copilot (same root as #1) — already covered by the chain.

#3+#4 Copilot — code comments referenced
   `Downloads/InfiniteAPI-Inbound-Latency-Fix-Documentation.md` which is
   intentionally out-of-tree (user keeps it locally). Replaced the broken
   refs with self-contained explanations covering the same rationale —
   future maintainers no longer need an external file to understand the
   trade-offs.

Skipped: CodeRabbit nitpick to extract a `fireAndForgetMappingStore` helper
(stylistic, current explicit form makes the lid/pn argument order obvious to
reviewers — keeping it).

Tests: 35/35 suites, 824/824 still passing. Customizations untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 10:25:27 -03:00
Renato Alcara a631c8c3c3 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

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) <noreply@anthropic.com>
2026-04-26 09:59:54 -03:00
3 changed files with 5 additions and 46 deletions
+1 -12
View File
@@ -341,18 +341,7 @@ export const addTransactionCapability = (
return result
} catch (error) {
// SessionError is part of the normal Bad MAC recovery flow
// (retry receipt → sender resends as pkmsg → new session within ~1.3s).
// Logging it as ERROR creates 2 noise lines per recoverable Bad MAC cycle.
// Downgrade to debug for SessionError; keep ERROR for everything else.
// The error is still re-thrown — recovery behavior is unchanged.
const errName = (error as { name?: string })?.name
if (errName === 'SessionError') {
logger.debug({ error }, 'transaction failed (SessionError — recoverable via retry receipt)')
} else {
logger.error({ error }, 'transaction failed, rolling back')
}
logger.error({ error }, 'transaction failed, rolling back')
throw error
}
})
+2 -23
View File
@@ -58,11 +58,7 @@ export const BAD_MAC_ERROR_TEXT = 'Bad MAC'
export const DECRYPTION_RETRY_CONFIG = {
maxRetries: 3,
baseDelayMs: 100,
// 'No matching sessions found' is the libsignal error when decryptWithSessions exhausts
// all stored sessions for a JID. Same recovery flow (retry receipt → pkmsg → new session)
// — categorise it as session-record so the caller logs DEBUG on retry, ERROR only when
// retries are exhausted (instead of dumping the full stack as an unknown error).
sessionRecordErrors: ['No session record', 'SessionError: No session record', 'No matching sessions found'],
sessionRecordErrors: ['No session record', 'SessionError: No session record'],
corruptedSessionErrors: ['Bad MAC', 'MessageCounterError', MISSING_KEYS_ERROR_TEXT]
}
@@ -425,26 +421,9 @@ export const decryptMessageNode = (
const isCorrupted = isCorruptedSessionError(originalError)
const isSessionRecord = isSessionRecordError(originalError)
// Slim error projection — keep name/message/type for diagnosis,
// drop `stack` which adds 4-5 lines of node_modules paths per log
// for known-recoverable libsignal errors.
//
// CRITICAL: only slim for KNOWN-RECOVERABLE categories (corrupted /
// session-record). The unknown-error branch keeps the full Error so
// protobuf/parsing/runtime bugs still emit a stack trace where it
// matters most. Catches Copilot/Codex P2 review on PR #391.
const slimErr = originalError
? {
name: (originalError as { name?: string }).name,
message: (originalError as { message?: string }).message,
type: (originalError as { type?: string }).type
}
: undefined
const isRecoverableCategory = isCorrupted || isSessionRecord
const errorContext = {
key: fullMessage.key,
err: isRecoverableCategory ? slimErr : originalError,
err: originalError,
messageType: tag === 'plaintext' ? 'plaintext' : attrs.type,
sender,
author,
+2 -11
View File
@@ -28,16 +28,7 @@ console.info = function (...args: unknown[]) {
// Track errors by type + JID to avoid duplicates (using Map for better performance)
const _errorTimestamps = new Map<string, number>()
// Dedup window for repeated decrypt-error console lines (Bad MAC / Counter / etc).
// Was 150ms, but retry attempts of the SAME message are typically ~300-1000ms apart,
// so the second attempt fell outside the window and double-printed.
//
// TRADE-OFF: dedup key is `errorType + JID` (no message-id). With 5s, a burst of
// errors for the SAME JID — even of slightly different categories or different
// messages — collapses to one log line every 5s. This is intentional for a noisy
// production stream; if you need per-message visibility, set BAILEYS_LOG_LEVEL=debug
// to bypass this console-side dedup and see the structured pino logs in full.
const DEDUP_WINDOW_MS = 5000
const DEDUP_WINDOW_MS = 150
console.error = function (...args: unknown[]) {
if (args.length > 0 && typeof args[0] === 'string') {
@@ -79,7 +70,7 @@ console.error = function (...args: unknown[]) {
const lastTime = _errorTimestamps.get(dedupeKey)
if (lastTime && now - lastTime < DEDUP_WINDOW_MS) {
return // Skip duplicate within DEDUP_WINDOW_MS window
return // Skip duplicate within 150ms window
}
_errorTimestamps.set(dedupeKey, now)