fix(album): address code review feedback
Fixes all issues identified in PR #44 review: 1. **Album root message now relayed to server** (Critical) - Before: Only generated root message, never sent it - After: Calls relayMessage() on albumRootMsg before sending media items - Also emits own event if emitOwnEvents is enabled 2. **Fixed messageId collision** - Before: Spread ...options could pass same messageId to all items - After: Explicitly pass only safe options (timestamp, quoted, etc.) - Each media item now gets a fresh ID from generateWAMessage 3. **Fixed proto structure for album association** - Before: Used non-existent messageContextInfo.messageAddOnType - After: Uses correct messageContextInfo.messageAssociation with: - associationType: proto.MessageAssociation.AssociationType.MEDIA_ALBUM - parentMessageKey: albumKey 4. **Added runtime error for sendMessage misuse** - sendMessage() now throws clear error if called with { album: ... } - Forces users to use sendAlbumMessage() for proper behavior 5. **Fixed documentation** - delay: Changed "based on media size" to "based on media type" - retryAttempts: Clarified it's "total attempts" not just retries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
+47
-10
@@ -1353,7 +1353,7 @@ export const makeMessagesSocket = (config: SocketConfig) => {
|
|||||||
'Starting album message send'
|
'Starting album message send'
|
||||||
)
|
)
|
||||||
|
|
||||||
// Generate album root message first
|
// Generate album root message first (with counts of expected media)
|
||||||
const albumRootMsg = await generateWAMessage(jid, {
|
const albumRootMsg = await generateWAMessage(jid, {
|
||||||
album: { medias, delay: delayConfig, retryCount, continueOnFailure }
|
album: { medias, delay: delayConfig, retryCount, continueOnFailure }
|
||||||
}, {
|
}, {
|
||||||
@@ -1367,15 +1367,37 @@ export const makeMessagesSocket = (config: SocketConfig) => {
|
|||||||
}),
|
}),
|
||||||
upload: waUploadToServer,
|
upload: waUploadToServer,
|
||||||
mediaCache: config.mediaCache,
|
mediaCache: config.mediaCache,
|
||||||
...options
|
// Don't spread options here to avoid messageId collision
|
||||||
|
timestamp: options.timestamp,
|
||||||
|
quoted: options.quoted,
|
||||||
|
ephemeralExpiration: options.ephemeralExpiration,
|
||||||
|
mediaUploadTimeoutMs: options.mediaUploadTimeoutMs
|
||||||
})
|
})
|
||||||
|
|
||||||
const albumKey = albumRootMsg.key
|
const albumKey = albumRootMsg.key
|
||||||
|
|
||||||
|
// CRITICAL: Relay album root message to server first
|
||||||
|
// Without this, child media items reference a non-existent album key
|
||||||
|
await relayMessage(jid, albumRootMsg.message!, {
|
||||||
|
messageId: albumRootMsg.key.id!,
|
||||||
|
useCachedGroupMetadata: options.useCachedGroupMetadata
|
||||||
|
})
|
||||||
|
|
||||||
|
// Emit own event for album root if configured
|
||||||
|
if (config.emitOwnEvents) {
|
||||||
|
process.nextTick(async () => {
|
||||||
|
await messageMutex.mutex(() => upsertMessage(albumRootMsg, 'append'))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
logger.debug({ albumKeyId: albumKey.id }, 'Album root message relayed')
|
||||||
|
|
||||||
const results: AlbumMediaResult[] = []
|
const results: AlbumMediaResult[] = []
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Calculate adaptive delay based on media characteristics
|
* Calculate adaptive delay based on media characteristics
|
||||||
* Larger files and videos get more delay to prevent rate limiting
|
* Videos get more delay (2x), later items get slightly more delay,
|
||||||
|
* plus random jitter to prevent predictable patterns
|
||||||
*/
|
*/
|
||||||
const calculateAdaptiveDelay = (media: AlbumMediaItem, index: number): number => {
|
const calculateAdaptiveDelay = (media: AlbumMediaItem, index: number): number => {
|
||||||
const baseDelay = 500 // Base delay in ms
|
const baseDelay = 500 // Base delay in ms
|
||||||
@@ -1418,6 +1440,7 @@ export const makeMessagesSocket = (config: SocketConfig) => {
|
|||||||
attempts = attempt + 1
|
attempts = attempt + 1
|
||||||
try {
|
try {
|
||||||
// Generate message for this media item
|
// Generate message for this media item
|
||||||
|
// NOTE: Each item needs its own unique messageId, so we don't spread options.messageId
|
||||||
const mediaMsg = await generateWAMessage(jid, media as AnyMessageContent, {
|
const mediaMsg = await generateWAMessage(jid, media as AnyMessageContent, {
|
||||||
logger,
|
logger,
|
||||||
userJid,
|
userJid,
|
||||||
@@ -1429,18 +1452,23 @@ export const makeMessagesSocket = (config: SocketConfig) => {
|
|||||||
}),
|
}),
|
||||||
upload: waUploadToServer,
|
upload: waUploadToServer,
|
||||||
mediaCache: config.mediaCache,
|
mediaCache: config.mediaCache,
|
||||||
...options
|
// Don't spread ...options to avoid messageId collision
|
||||||
|
// Each item gets a fresh ID from generateWAMessage
|
||||||
|
timestamp: options.timestamp,
|
||||||
|
quoted: options.quoted,
|
||||||
|
ephemeralExpiration: options.ephemeralExpiration,
|
||||||
|
mediaUploadTimeoutMs: options.mediaUploadTimeoutMs
|
||||||
})
|
})
|
||||||
|
|
||||||
// Attach to parent album via messageContextInfo with parentKey
|
// Attach to parent album via messageAssociation (correct proto structure)
|
||||||
|
// Uses AssociationType.MEDIA_ALBUM and parentMessageKey as per WhatsApp protocol
|
||||||
if (!mediaMsg.message!.messageContextInfo) {
|
if (!mediaMsg.message!.messageContextInfo) {
|
||||||
mediaMsg.message!.messageContextInfo = {}
|
mediaMsg.message!.messageContextInfo = {}
|
||||||
}
|
}
|
||||||
mediaMsg.message!.messageContextInfo.messageAddOnType = proto.MessageAddOnType.MEDIA_ALBUM
|
mediaMsg.message!.messageContextInfo.messageAssociation = {
|
||||||
|
associationType: proto.MessageAssociation.AssociationType.MEDIA_ALBUM,
|
||||||
// Add reference to parent album message (as suggested by purpshell)
|
parentMessageKey: albumKey
|
||||||
// This allows proper grouping and retry of individual items
|
}
|
||||||
;(mediaMsg.message!.messageContextInfo as any).parentMessageKey = albumKey
|
|
||||||
|
|
||||||
// Relay the message
|
// Relay the message
|
||||||
await relayMessage(jid, mediaMsg.message!, {
|
await relayMessage(jid, mediaMsg.message!, {
|
||||||
@@ -1553,6 +1581,15 @@ export const makeMessagesSocket = (config: SocketConfig) => {
|
|||||||
},
|
},
|
||||||
|
|
||||||
sendMessage: async (jid: string, content: AnyMessageContent, options: MiscMessageGenerationOptions = {}) => {
|
sendMessage: async (jid: string, content: AnyMessageContent, options: MiscMessageGenerationOptions = {}) => {
|
||||||
|
// Check for album misuse - must use sendAlbumMessage instead
|
||||||
|
if (typeof content === 'object' && 'album' in content) {
|
||||||
|
throw new Boom(
|
||||||
|
'Cannot send album messages with sendMessage(). Use sendAlbumMessage() instead, ' +
|
||||||
|
'which properly sends the album root and individual media items.',
|
||||||
|
{ statusCode: 400 }
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
const userJid = authState.creds.me!.id
|
const userJid = authState.creds.me!.id
|
||||||
if (
|
if (
|
||||||
typeof content === 'object' &&
|
typeof content === 'object' &&
|
||||||
|
|||||||
@@ -319,7 +319,8 @@ export type AlbumMessageOptions = {
|
|||||||
medias: AlbumMediaItem[]
|
medias: AlbumMediaItem[]
|
||||||
/**
|
/**
|
||||||
* Delay strategy between media sends
|
* Delay strategy between media sends
|
||||||
* - 'adaptive': Calculates delay based on media size (recommended)
|
* - 'adaptive': Calculates delay based on media type (videos get 2x delay),
|
||||||
|
* position in album, and random jitter (recommended)
|
||||||
* - number: Fixed delay in milliseconds
|
* - number: Fixed delay in milliseconds
|
||||||
* @default 'adaptive'
|
* @default 'adaptive'
|
||||||
*/
|
*/
|
||||||
@@ -348,7 +349,7 @@ export type AlbumMediaResult = {
|
|||||||
message?: WAMessage
|
message?: WAMessage
|
||||||
/** Error details (if failed) */
|
/** Error details (if failed) */
|
||||||
error?: Error
|
error?: Error
|
||||||
/** Number of retry attempts made */
|
/** Total number of attempts made (1 = success on first try, >1 = retries occurred) */
|
||||||
retryAttempts: number
|
retryAttempts: number
|
||||||
/** Time taken to send this item in ms */
|
/** Time taken to send this item in ms */
|
||||||
latencyMs: number
|
latencyMs: number
|
||||||
@@ -429,7 +430,12 @@ export type AnyRegularMessageContent = (
|
|||||||
footer?: string
|
footer?: string
|
||||||
}
|
}
|
||||||
| {
|
| {
|
||||||
/** Album message - send multiple images/videos grouped together */
|
/**
|
||||||
|
* Album message - send multiple images/videos grouped together
|
||||||
|
* ⚠️ WARNING: Do NOT use with sendMessage() - use sendAlbumMessage() instead!
|
||||||
|
* sendMessage only relays the root message and won't send individual media items
|
||||||
|
* @internal Used internally by generateWAMessage
|
||||||
|
*/
|
||||||
album: AlbumMessageOptions
|
album: AlbumMessageOptions
|
||||||
}
|
}
|
||||||
| SharePhoneNumber
|
| SharePhoneNumber
|
||||||
|
|||||||
Reference in New Issue
Block a user