fix: batch merge notifications and add storage failure warning
Fixes Copilot audit issues #2 and #4 (code we added): Issue #2 - Multiple Event Emissions (MEDIUM severity): - Changed: Collect all merge notifications in array - Emit: Single batched event instead of multiple separate events - Impact: Better performance, fewer DB transactions, no UI flickering Issue #4 - Storage Validation (MEDIUM severity): - Added: Warning log when LID-PN mappings fail to store - Tracks: errors count vs notifications sent for debugging - Improves: Observability of partial storage failures Technical changes: - Declared mergeNotifications array before loop - Compute mergedAt timestamp once (not per iteration) - Push notifications to array instead of emitting in loop - Emit single chats.update with all notifications - Log warning with detailed counts if result.errors > 0 Benefits: ✅ 100x fewer events for 100 mappings (1 vs 100) ✅ Better consumer performance (ZPRO) ✅ Improved observability of storage failures ✅ Zero breaking changes (backward compatible) Note: Did NOT fix Issue #3 (Prototype Pollution) as it's in pre-existing code (event-buffer.ts), not code we added. https://claude.ai/code/session_01SoNUGBEWbJwWWws3F2fuzh
This commit is contained in:
+28
-10
@@ -1309,6 +1309,10 @@ export const makeChatsSocket = (config: SocketConfig) => {
|
|||||||
|
|
||||||
// Automatic chat merge: notify consumers about LID→PN mapping
|
// Automatic chat merge: notify consumers about LID→PN mapping
|
||||||
// This allows ZPRO and other consumers to merge/rename chats accordingly
|
// This allows ZPRO and other consumers to merge/rename chats accordingly
|
||||||
|
// Collect all merge notifications to emit in a single batch
|
||||||
|
const mergeNotifications: ChatUpdate[] = []
|
||||||
|
const mergedAt = Date.now()
|
||||||
|
|
||||||
for (const mapping of mappings) {
|
for (const mapping of mappings) {
|
||||||
const lidUser = jidNormalizedUser(mapping.lid)
|
const lidUser = jidNormalizedUser(mapping.lid)
|
||||||
const pnUser = jidNormalizedUser(mapping.pn)
|
const pnUser = jidNormalizedUser(mapping.pn)
|
||||||
@@ -1316,20 +1320,34 @@ export const makeChatsSocket = (config: SocketConfig) => {
|
|||||||
if (lidUser && pnUser && lidUser !== pnUser) {
|
if (lidUser && pnUser && lidUser !== pnUser) {
|
||||||
logger.debug(
|
logger.debug(
|
||||||
{ lid: lidUser, pn: pnUser },
|
{ lid: lidUser, pn: pnUser },
|
||||||
'emitting chat update for LID→PN merge notification'
|
'collected chat update for LID→PN merge notification'
|
||||||
)
|
)
|
||||||
|
|
||||||
// Emit chat update indicating this chat should be merged/renamed from LID to PN
|
mergeNotifications.push({
|
||||||
ev.emit('chats.update', [
|
id: pnUser,
|
||||||
{
|
merged: true,
|
||||||
id: pnUser,
|
previousId: lidUser,
|
||||||
merged: true,
|
mergedAt
|
||||||
previousId: lidUser,
|
})
|
||||||
mergedAt: Date.now()
|
|
||||||
}
|
|
||||||
])
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Emit single batch of merge notifications for better performance
|
||||||
|
if (mergeNotifications.length > 0) {
|
||||||
|
logger.debug(
|
||||||
|
{ count: mergeNotifications.length },
|
||||||
|
'emitting batch of chat merge notifications'
|
||||||
|
)
|
||||||
|
ev.emit('chats.update', mergeNotifications)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Log warning if some mappings failed to store
|
||||||
|
if (result.errors > 0) {
|
||||||
|
logger.warn(
|
||||||
|
{ errors: result.errors, total: mappings.length, notified: mergeNotifications.length },
|
||||||
|
'some LID-PN mappings failed to store, but merge notifications were sent'
|
||||||
|
)
|
||||||
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
logger.warn({ count: mappings.length, error }, 'Failed to store LID-PN mappings')
|
logger.warn({ count: mappings.length, error }, 'Failed to store LID-PN mappings')
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user