fix: address PR review — stale comments, mutex key normalization, test sync
- Update stale comments in event-buffer.ts (15s→3s) and socket.ts (5s→2s) to reflect actual timeout values - Normalize mutex keys with jidNormalizedUser() for receiptMutex and notificationMutex to handle device suffix variants (e.g. user:5@s.whatsapp.net) - Update offline-buffer-timeout.test.ts to match new 2s timeout value (was hardcoded at 5s, causing test/source divergence) - Rejected suggestion to use attrs.id as fallback (false positive — attrs.id is a message ID, not a chat identifier; 'unknown' is the correct catch-all) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1282,7 +1282,7 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
await Promise.all([
|
await Promise.all([
|
||||||
receiptMutex.mutex(remoteJid || 'unknown', async () => {
|
receiptMutex.mutex(jidNormalizedUser(remoteJid) || 'unknown', async () => {
|
||||||
const status = getStatusFromReceiptType(attrs.type)
|
const status = getStatusFromReceiptType(attrs.type)
|
||||||
if (
|
if (
|
||||||
typeof status !== 'undefined' &&
|
typeof status !== 'undefined' &&
|
||||||
@@ -1356,7 +1356,7 @@ export const makeMessagesRecvSocket = (config: SocketConfig) => {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
await Promise.all([
|
await Promise.all([
|
||||||
notificationMutex.mutex(remoteJid || 'unknown', async () => {
|
notificationMutex.mutex(jidNormalizedUser(remoteJid) || 'unknown', async () => {
|
||||||
const msg = await processNotification(node)
|
const msg = await processNotification(node)
|
||||||
if (msg) {
|
if (msg) {
|
||||||
const fromMe = areJidsSameUser(node.attrs.participant || remoteJid, authState.creds.me!.id)
|
const fromMe = areJidsSameUser(node.attrs.participant || remoteJid, authState.creds.me!.id)
|
||||||
|
|||||||
@@ -1667,7 +1667,7 @@ export const makeSocket = (config: SocketConfig) => {
|
|||||||
// already-authenticated session (no QR re-scan needed). In this case we skip
|
// already-authenticated session (no QR re-scan needed). In this case we skip
|
||||||
// the offline buffer entirely so live messages are not held hostage waiting for the
|
// the offline buffer entirely so live messages are not held hostage waiting for the
|
||||||
// server to finish flushing the pending-message backlog (CB:ib,,offline).
|
// server to finish flushing the pending-message backlog (CB:ib,,offline).
|
||||||
// For normal restarts (no stale routingInfo) the standard 5 s safety cap applies.
|
// For normal restarts (no stale routingInfo) the standard 2 s safety cap applies.
|
||||||
const OFFLINE_BUFFER_TIMEOUT_MS = 2_000
|
const OFFLINE_BUFFER_TIMEOUT_MS = 2_000
|
||||||
let offlineBufferTimeout: NodeJS.Timeout | undefined
|
let offlineBufferTimeout: NodeJS.Timeout | undefined
|
||||||
|
|
||||||
|
|||||||
@@ -54,7 +54,7 @@ export interface BufferConfig {
|
|||||||
*/
|
*/
|
||||||
export function loadBufferConfig(): BufferConfig {
|
export function loadBufferConfig(): BufferConfig {
|
||||||
return {
|
return {
|
||||||
// perf(inbound-latency): reduced from 15s → 5s so the safety auto-flush fires sooner.
|
// perf(inbound-latency): reduced from 15s → 3s so the safety auto-flush fires sooner.
|
||||||
// processNodeWithBuffer always calls ev.flush() explicitly (no-op for this timer),
|
// processNodeWithBuffer always calls ev.flush() explicitly (no-op for this timer),
|
||||||
// but socket.ts's offline-phase buffer and any stalled buffer benefit from the lower cap.
|
// but socket.ts's offline-phase buffer and any stalled buffer benefit from the lower cap.
|
||||||
bufferTimeoutMs: parseInt(process.env.BAILEYS_BUFFER_TIMEOUT_MS || '3000', 10),
|
bufferTimeoutMs: parseInt(process.env.BAILEYS_BUFFER_TIMEOUT_MS || '3000', 10),
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ import { jest } from '@jest/globals'
|
|||||||
* bad-ack-handling.test.ts.
|
* bad-ack-handling.test.ts.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
const OFFLINE_BUFFER_TIMEOUT_MS = 5_000
|
const OFFLINE_BUFFER_TIMEOUT_MS = 2_000
|
||||||
|
|
||||||
/** Mirrors the state variables declared at the top of makeSocket */
|
/** Mirrors the state variables declared at the top of makeSocket */
|
||||||
interface OfflineBufferState {
|
interface OfflineBufferState {
|
||||||
@@ -101,10 +101,10 @@ describe('offline-buffer safety timer (socket.ts)', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
// 1. Timeout path — CB:ib,,offline never arrives within 5 s
|
// 1. Timeout path — CB:ib,,offline never arrives within 2 s
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
it('fires after 5 s and flushes when CB:ib,,offline is delayed', () => {
|
it('fires after 2 s and flushes when CB:ib,,offline is delayed', () => {
|
||||||
startBuffer(state, mockFlush, mockWarn)
|
startBuffer(state, mockFlush, mockWarn)
|
||||||
|
|
||||||
expect(mockFlush).not.toHaveBeenCalled()
|
expect(mockFlush).not.toHaveBeenCalled()
|
||||||
@@ -144,17 +144,17 @@ describe('offline-buffer safety timer (socket.ts)', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
// 2. Happy path — CB:ib,,offline arrives before the 5 s timer fires
|
// 2. Happy path — CB:ib,,offline arrives before the 2 s timer fires
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
it('CB:ib,,offline cancels the timer and flushes exactly once', () => {
|
it('CB:ib,,offline cancels the timer and flushes exactly once', () => {
|
||||||
startBuffer(state, mockFlush, mockWarn)
|
startBuffer(state, mockFlush, mockWarn)
|
||||||
|
|
||||||
// Server responds before the 5 s timeout
|
// Server responds before the 2 s timeout
|
||||||
jest.advanceTimersByTime(1_000)
|
jest.advanceTimersByTime(1_000)
|
||||||
onOffline(state, mockFlush)
|
onOffline(state, mockFlush)
|
||||||
|
|
||||||
// Timer should be cancelled — advancing past 5 s must not cause a second flush
|
// Timer should be cancelled — advancing past 2 s must not cause a second flush
|
||||||
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS)
|
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS)
|
||||||
|
|
||||||
expect(mockFlush).toHaveBeenCalledTimes(1)
|
expect(mockFlush).toHaveBeenCalledTimes(1)
|
||||||
@@ -192,7 +192,7 @@ describe('offline-buffer safety timer (socket.ts)', () => {
|
|||||||
|
|
||||||
onClose(state)
|
onClose(state)
|
||||||
|
|
||||||
// Timer must be gone — advancing past 5 s must not trigger any flush
|
// Timer must be gone — advancing past 2 s must not trigger any flush
|
||||||
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS)
|
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS)
|
||||||
|
|
||||||
expect(mockFlush).not.toHaveBeenCalled()
|
expect(mockFlush).not.toHaveBeenCalled()
|
||||||
@@ -236,7 +236,7 @@ describe('offline-buffer safety timer (socket.ts)', () => {
|
|||||||
// 4. Boundary / timing edge cases
|
// 4. Boundary / timing edge cases
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
it('does not flush before exactly 5 s have elapsed', () => {
|
it('does not flush before exactly 2 s have elapsed', () => {
|
||||||
startBuffer(state, mockFlush, mockWarn)
|
startBuffer(state, mockFlush, mockWarn)
|
||||||
|
|
||||||
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS - 1)
|
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS - 1)
|
||||||
@@ -244,7 +244,7 @@ describe('offline-buffer safety timer (socket.ts)', () => {
|
|||||||
expect(mockFlush).not.toHaveBeenCalled()
|
expect(mockFlush).not.toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
it('flushes at exactly the 5 s boundary', () => {
|
it('flushes at exactly the 2 s boundary', () => {
|
||||||
startBuffer(state, mockFlush, mockWarn)
|
startBuffer(state, mockFlush, mockWarn)
|
||||||
|
|
||||||
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS)
|
jest.advanceTimersByTime(OFFLINE_BUFFER_TIMEOUT_MS)
|
||||||
|
|||||||
Reference in New Issue
Block a user