Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5737ba590c | |||
| 4a4766f52d | |||
| 1dffe3b311 |
+39
-29
@@ -2330,44 +2330,54 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
)
|
)
|
||||||
|
|
||||||
const alt = msg.key.participantAlt || msg.key.remoteJidAlt
|
const alt = msg.key.participantAlt || msg.key.remoteJidAlt
|
||||||
// Handle LID/PN mappings with hybrid approach:
|
// Handle LID/PN mappings with optimized hot-path:
|
||||||
// - Store mapping operation runs in background (non-critical for decrypt)
|
// - storeLIDPNMappings is fire-and-forget (background) — does NOT block decrypt
|
||||||
// - Session migration MUST complete before decrypt() to avoid "No session record" errors
|
// - migrateSession is SYNC (await) — REQUIRED for decrypt to find session
|
||||||
// This addresses Codex/Copilot review concerns about race conditions with decrypt()
|
//
|
||||||
|
// 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) {
|
if (!!alt) {
|
||||||
const altServer = jidDecode(alt)?.server
|
const altServer = jidDecode(alt)?.server
|
||||||
const primaryJid = msg.key.participant || msg.key.remoteJid!
|
const primaryJid = msg.key.participant || msg.key.remoteJid!
|
||||||
|
|
||||||
if (altServer === 'lid') {
|
if (altServer === 'lid') {
|
||||||
// Check if mapping already exists to avoid unnecessary storage operations
|
// Fire-and-forget: storeLIDPNMappings has internal cache+dedup,
|
||||||
const existingMapping = await signalRepository.lidMapping.getPNForLID(alt)
|
// pre-check (getPNForLID) was redundant.
|
||||||
if (!existingMapping) {
|
signalRepository.lidMapping
|
||||||
// MUST await: normalizeMessageJids() runs after this and needs the mapping
|
.storeLIDPNMappings([{ lid: alt, pn: primaryJid }])
|
||||||
// in the LIDMappingStore to resolve LID→PN for events delivered to consumers
|
.catch(error => logger.warn({ error, alt, primaryJid }, 'background LID mapping store failed'))
|
||||||
await signalRepository.lidMapping
|
|
||||||
.storeLIDPNMappings([{ lid: alt, pn: primaryJid }])
|
|
||||||
.catch(error => logger.warn({ error, alt, primaryJid }, 'LID mapping storage failed'))
|
|
||||||
}
|
|
||||||
|
|
||||||
// CRITICAL: ALWAYS migrate session, even if mapping exists
|
// CRITICAL: ALWAYS migrate session SYNC, even if mapping exists.
|
||||||
// Other code paths (e.g., USync device lookup in messages-send.ts:310-319)
|
// Other code paths (e.g., USync device lookup in messages-send.ts) may create
|
||||||
// may create mappings via storeLIDPNMappings() without calling migrateSession()
|
// mappings via storeLIDPNMappings() without calling migrateSession(). This
|
||||||
// This leaves sessions under PN format while decrypt() expects LID format
|
// leaves sessions under PN format while decrypt() expects LID format.
|
||||||
// Skipping migration based on mapping existence causes "No session record" errors
|
// Skipping migration based on mapping existence causes "No session record" errors.
|
||||||
await signalRepository.migrateSession(primaryJid, alt)
|
await signalRepository.migrateSession(primaryJid, alt)
|
||||||
} else {
|
} else {
|
||||||
// Check if reverse mapping exists
|
// Fire-and-forget: same rationale as above.
|
||||||
const existingMapping = await signalRepository.lidMapping.getLIDForPN(alt)
|
signalRepository.lidMapping
|
||||||
if (!existingMapping) {
|
.storeLIDPNMappings([{ lid: primaryJid, pn: alt }])
|
||||||
// MUST await: normalizeMessageJids() runs after this and needs the mapping
|
.catch(error => logger.warn({ error, alt, primaryJid }, 'background LID mapping store failed'))
|
||||||
// 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'))
|
|
||||||
}
|
|
||||||
|
|
||||||
// CRITICAL: ALWAYS migrate session, even if mapping exists
|
// CRITICAL: ALWAYS migrate session SYNC.
|
||||||
// Same reasoning as above - mapping existence doesn't guarantee session migration
|
|
||||||
await signalRepository.migrateSession(alt, primaryJid)
|
await signalRepository.migrateSession(alt, primaryJid)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+12
-1
@@ -341,7 +341,18 @@ export const addTransactionCapability = (
|
|||||||
|
|
||||||
return result
|
return result
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logger.error({ error }, 'transaction failed, rolling back')
|
// 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')
|
||||||
|
}
|
||||||
|
|
||||||
throw error
|
throw error
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -58,7 +58,11 @@ export const BAD_MAC_ERROR_TEXT = 'Bad MAC'
|
|||||||
export const DECRYPTION_RETRY_CONFIG = {
|
export const DECRYPTION_RETRY_CONFIG = {
|
||||||
maxRetries: 3,
|
maxRetries: 3,
|
||||||
baseDelayMs: 100,
|
baseDelayMs: 100,
|
||||||
sessionRecordErrors: ['No session record', 'SessionError: No session record'],
|
// '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'],
|
||||||
corruptedSessionErrors: ['Bad MAC', 'MessageCounterError', MISSING_KEYS_ERROR_TEXT]
|
corruptedSessionErrors: ['Bad MAC', 'MessageCounterError', MISSING_KEYS_ERROR_TEXT]
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -421,9 +425,26 @@ export const decryptMessageNode = (
|
|||||||
const isCorrupted = isCorruptedSessionError(originalError)
|
const isCorrupted = isCorruptedSessionError(originalError)
|
||||||
const isSessionRecord = isSessionRecordError(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 = {
|
const errorContext = {
|
||||||
key: fullMessage.key,
|
key: fullMessage.key,
|
||||||
err: originalError,
|
err: isRecoverableCategory ? slimErr : originalError,
|
||||||
messageType: tag === 'plaintext' ? 'plaintext' : attrs.type,
|
messageType: tag === 'plaintext' ? 'plaintext' : attrs.type,
|
||||||
sender,
|
sender,
|
||||||
author,
|
author,
|
||||||
|
|||||||
@@ -82,6 +82,47 @@ const REAL_MSG_REQ_ME_STUB_TYPES = new Set([WAMessageStubType.GROUP_PARTICIPANT_
|
|||||||
* (TC_TOKEN_INDEX_KEY) via buildMergedTcTokenIndexWrite, so the 24h prune sweep in
|
* (TC_TOKEN_INDEX_KEY) via buildMergedTcTokenIndexWrite, so the 24h prune sweep in
|
||||||
* messages-recv picks them up across sessions.
|
* messages-recv picks them up across sessions.
|
||||||
*/
|
*/
|
||||||
|
/**
|
||||||
|
* Single-concurrency queue for `storeTcTokensFromHistorySync` calls.
|
||||||
|
*
|
||||||
|
* Why: the function does read-then-write merges (`keyStore.get('tctoken', ...)` →
|
||||||
|
* compute → `keyStore.set(...)`) which are NOT atomic at the store level. If two
|
||||||
|
* history-sync chunks invoke this concurrently (common during reconnect / QR
|
||||||
|
* scan), an older chunk that started first can `keyStore.set` AFTER a newer
|
||||||
|
* chunk, overwriting the newer entry — and worse, the merged `__index` write
|
||||||
|
* can drop JIDs the other chunk just added. Result: stale tcTokens / repeat 463
|
||||||
|
* sends until the next opportunistic refetch.
|
||||||
|
*
|
||||||
|
* Serialising via a chained Promise keeps the runs ordered while still freeing
|
||||||
|
* the calling `processMessage` to emit `messaging-history.set` immediately
|
||||||
|
* (the chain is fire-and-forget at the call site). Errors don't break the chain
|
||||||
|
* — each `catch` resets it to `Promise.resolve()` so a single failure can't
|
||||||
|
* stall future runs.
|
||||||
|
*
|
||||||
|
* The chain is module-scoped (one per Node process). Multiple Baileys instances
|
||||||
|
* sharing this module will serialise across instances too, but their writes
|
||||||
|
* target different keyStores so there's no correctness gain — only a tiny loss
|
||||||
|
* of inter-instance parallelism for tcToken syncs, which is acceptable given
|
||||||
|
* how rarely this runs vs. how rare cross-instance contention is.
|
||||||
|
*/
|
||||||
|
let historyTcTokenChain: Promise<void> = Promise.resolve()
|
||||||
|
|
||||||
|
function scheduleHistoryTcTokenSync(
|
||||||
|
chats: Chat[],
|
||||||
|
signalRepository: SignalRepositoryWithLIDStore,
|
||||||
|
keyStore: SignalKeyStoreWithTransaction,
|
||||||
|
logger?: ILogger
|
||||||
|
): void {
|
||||||
|
historyTcTokenChain = historyTcTokenChain
|
||||||
|
.catch(() => {
|
||||||
|
/* swallow prior error so chain stays alive */
|
||||||
|
})
|
||||||
|
.then(() => storeTcTokensFromHistorySync(chats, signalRepository, keyStore, logger))
|
||||||
|
.catch(err => {
|
||||||
|
logger?.warn({ err }, 'background tctoken history-sync persistence failed')
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
async function storeTcTokensFromHistorySync(
|
async function storeTcTokensFromHistorySync(
|
||||||
chats: Chat[],
|
chats: Chat[],
|
||||||
signalRepository: SignalRepositoryWithLIDStore,
|
signalRepository: SignalRepositoryWithLIDStore,
|
||||||
@@ -565,11 +606,28 @@ const processMessage = async (
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Persist tctokens carried by history-sync chats BEFORE emitting messaging-history.set
|
// Persist tctokens carried by history-sync chats in BACKGROUND, serialised.
|
||||||
// — 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.
|
// Originally awaited (PR #386) to avoid 463 on first multi-device send, but in
|
||||||
// Runs AFTER storeLIDPNMappings (see comment above) so LID resolution works.
|
// production this drained the event buffer per-chunk and added visible delivery
|
||||||
await storeTcTokensFromHistorySync(data.chats, signalRepository, keyStore, logger)
|
// latency (especially after restart / QR scan when many chunks arrived at once).
|
||||||
|
//
|
||||||
|
// `scheduleHistoryTcTokenSync` enqueues onto a single-concurrency promise chain
|
||||||
|
// (see definition above) — chunks persist sequentially in the order they were
|
||||||
|
// emitted, preserving timestamp monotonicity AND keeping the `__index` write
|
||||||
|
// safe from concurrent merge clobbers. The call returns immediately so the
|
||||||
|
// `messaging-history.set` emit is not blocked.
|
||||||
|
//
|
||||||
|
// TRADE-OFF: a listener that fires an outbound send IMMEDIATELY after the emit
|
||||||
|
// may race the still-pending 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, AND
|
||||||
|
// DO NOT call storeTcTokensFromHistorySync directly — it must go through the
|
||||||
|
// chain to preserve write ordering across overlapping chunks.
|
||||||
|
scheduleHistoryTcTokenSync(data.chats, signalRepository, keyStore, logger)
|
||||||
|
|
||||||
ev.emit('messaging-history.set', {
|
ev.emit('messaging-history.set', {
|
||||||
...data,
|
...data,
|
||||||
|
|||||||
+11
-2
@@ -28,7 +28,16 @@ console.info = function (...args: unknown[]) {
|
|||||||
|
|
||||||
// Track errors by type + JID to avoid duplicates (using Map for better performance)
|
// Track errors by type + JID to avoid duplicates (using Map for better performance)
|
||||||
const _errorTimestamps = new Map<string, number>()
|
const _errorTimestamps = new Map<string, number>()
|
||||||
const DEDUP_WINDOW_MS = 150
|
// 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
|
||||||
|
|
||||||
console.error = function (...args: unknown[]) {
|
console.error = function (...args: unknown[]) {
|
||||||
if (args.length > 0 && typeof args[0] === 'string') {
|
if (args.length > 0 && typeof args[0] === 'string') {
|
||||||
@@ -70,7 +79,7 @@ console.error = function (...args: unknown[]) {
|
|||||||
const lastTime = _errorTimestamps.get(dedupeKey)
|
const lastTime = _errorTimestamps.get(dedupeKey)
|
||||||
|
|
||||||
if (lastTime && now - lastTime < DEDUP_WINDOW_MS) {
|
if (lastTime && now - lastTime < DEDUP_WINDOW_MS) {
|
||||||
return // Skip duplicate within 150ms window
|
return // Skip duplicate within DEDUP_WINDOW_MS window
|
||||||
}
|
}
|
||||||
|
|
||||||
_errorTimestamps.set(dedupeKey, now)
|
_errorTimestamps.set(dedupeKey, now)
|
||||||
|
|||||||
Reference in New Issue
Block a user