fix: address PR review — lint, dedup scope, and cleanup targeting
- Remove unused `autoCleanCorrupted` param from decryptMessageNode (lint fix) - Scope retry dedup by participant JID in group chats (not group JID) - Move dedup registration after early-return checks to avoid blocking subsequent messages when max retries reached - Use participant JID for session cleanup in groups (Signal sessions are per-participant, not per-group) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+16
-11
@@ -1220,7 +1220,10 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
// Per-JID deduplication: when multiple messages from the same contact
|
// Per-JID deduplication: when multiple messages from the same contact
|
||||||
// fail with Bad MAC simultaneously, only send ONE retry request.
|
// fail with Bad MAC simultaneously, only send ONE retry request.
|
||||||
// The peer will resend all failed messages when it receives the retry receipt.
|
// The peer will resend all failed messages when it receives the retry receipt.
|
||||||
const retryDedupeJid = jidNormalizedUser(node.attrs.from!)
|
// For group messages, scope by participant (each participant has its own Signal session).
|
||||||
|
const retryDedupeJid = msgKey.participant
|
||||||
|
? jidNormalizedUser(msgKey.participant)
|
||||||
|
: jidNormalizedUser(node.attrs.from!)
|
||||||
if (retryRequestActiveJids.has(retryDedupeJid)) {
|
if (retryRequestActiveJids.has(retryDedupeJid)) {
|
||||||
logger.debug(
|
logger.debug(
|
||||||
{ fromJid: retryDedupeJid, msgId },
|
{ fromJid: retryDedupeJid, msgId },
|
||||||
@@ -1229,9 +1232,6 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
retryRequestActiveJids.add(retryDedupeJid)
|
|
||||||
setTimeout(() => retryRequestActiveJids.delete(retryDedupeJid), 5_000)
|
|
||||||
|
|
||||||
if (messageRetryManager) {
|
if (messageRetryManager) {
|
||||||
// Check if we've exceeded max retries using the new system
|
// Check if we've exceeded max retries using the new system
|
||||||
if (messageRetryManager.hasExceededMaxRetries(msgId)) {
|
if (messageRetryManager.hasExceededMaxRetries(msgId)) {
|
||||||
@@ -1244,10 +1244,11 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
// on every Bad MAC in the hot path (decode-wa-message.ts).
|
// on every Bad MAC in the hot path (decode-wa-message.ts).
|
||||||
// The Signal Protocol recovers naturally via retry+pkmsg for most cases;
|
// The Signal Protocol recovers naturally via retry+pkmsg for most cases;
|
||||||
// this cleanup only runs as a last resort.
|
// this cleanup only runs as a last resort.
|
||||||
|
// For group messages, use participant JID (Signal sessions are per-participant, not per-group).
|
||||||
if (autoCleanCorrupted) {
|
if (autoCleanCorrupted) {
|
||||||
const fromJid = node.attrs.from!
|
const senderJid = msgKey.participant ? jidNormalizedUser(msgKey.participant) : jidNormalizedUser(node.attrs.from!)
|
||||||
try {
|
try {
|
||||||
const decryptionJid = await getDecryptionJid(fromJid, signalRepository)
|
const decryptionJid = await getDecryptionJid(senderJid, signalRepository)
|
||||||
const deletedCount = await cleanupCorruptedSession(decryptionJid, signalRepository, logger)
|
const deletedCount = await cleanupCorruptedSession(decryptionJid, signalRepository, logger)
|
||||||
if (deletedCount > 0) {
|
if (deletedCount > 0) {
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -1256,7 +1257,7 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
} catch (cleanupErr) {
|
} catch (cleanupErr) {
|
||||||
logger.warn({ msgId, fromJid, err: cleanupErr }, 'Failed to cleanup session after retry exhaustion')
|
logger.warn({ msgId, senderJid, err: cleanupErr }, 'Failed to cleanup session after retry exhaustion')
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1281,12 +1282,12 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
|
|
||||||
// Safety net cleanup (same as new system above)
|
// Safety net cleanup (same as new system above)
|
||||||
if (autoCleanCorrupted) {
|
if (autoCleanCorrupted) {
|
||||||
const fromJid = node.attrs.from!
|
const senderJid = msgKey.participant ? jidNormalizedUser(msgKey.participant) : jidNormalizedUser(node.attrs.from!)
|
||||||
try {
|
try {
|
||||||
const decryptionJid = await getDecryptionJid(fromJid, signalRepository)
|
const decryptionJid = await getDecryptionJid(senderJid, signalRepository)
|
||||||
await cleanupCorruptedSession(decryptionJid, signalRepository, logger)
|
await cleanupCorruptedSession(decryptionJid, signalRepository, logger)
|
||||||
} catch (cleanupErr) {
|
} catch (cleanupErr) {
|
||||||
logger.warn({ msgId, fromJid, err: cleanupErr }, 'Failed to cleanup session after retry exhaustion')
|
logger.warn({ msgId, senderJid, err: cleanupErr }, 'Failed to cleanup session after retry exhaustion')
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1298,6 +1299,11 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
recordMessageRetry('retry')
|
recordMessageRetry('retry')
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Register dedup AFTER early-return checks so that max-retries paths
|
||||||
|
// don't block subsequent messages from the same JID for 5 seconds.
|
||||||
|
retryRequestActiveJids.add(retryDedupeJid)
|
||||||
|
setTimeout(() => retryRequestActiveJids.delete(retryDedupeJid), 5_000)
|
||||||
|
|
||||||
const key = `${msgId}:${msgKey?.participant}`
|
const key = `${msgId}:${msgKey?.participant}`
|
||||||
const retryCount = (await msgRetryCache.get<number>(key)) || 1
|
const retryCount = (await msgRetryCache.get<number>(key)) || 1
|
||||||
|
|
||||||
@@ -2228,7 +2234,6 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
authState.creds.me!.lid || '',
|
authState.creds.me!.lid || '',
|
||||||
signalRepository,
|
signalRepository,
|
||||||
logger,
|
logger,
|
||||||
autoCleanCorrupted
|
|
||||||
)
|
)
|
||||||
|
|
||||||
const alt = msg.key.participantAlt || msg.key.remoteJidAlt
|
const alt = msg.key.participantAlt || msg.key.remoteJidAlt
|
||||||
|
|||||||
@@ -276,7 +276,6 @@ export const decryptMessageNode = (
|
|||||||
meLid: string,
|
meLid: string,
|
||||||
repository: SignalRepositoryWithLIDStore,
|
repository: SignalRepositoryWithLIDStore,
|
||||||
logger: ILogger,
|
logger: ILogger,
|
||||||
autoCleanCorrupted = true
|
|
||||||
) => {
|
) => {
|
||||||
const { fullMessage, author, sender } = decodeMessageNode(stanza, meId, meLid)
|
const { fullMessage, author, sender } = decodeMessageNode(stanza, meId, meLid)
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user