Mailchimp performance update #19
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } else { | ||
| logger.error(`Failed to delete MailChimp subscriber from list ${list.id}: ${err.message}`) | ||
| throw err | ||
| const deletionResults = await Promise.allSettled( |
There was a problem hiding this comment.
[correctness]
Using Promise.allSettled is a good choice for handling multiple promises where you want to continue processing even if some fail. However, be cautious about the potential for unhandled promise rejections if any of the axios.post calls throw an error that is not a 404. Consider logging all errors within the catch block to ensure no silent failures.
| }) | ||
| ) | ||
|
|
||
| const failedDeletion = deletionResults.find((result) => result.status === 'rejected') |
There was a problem hiding this comment.
[maintainability]
The current implementation only throws the first error encountered in failedDeletion. If multiple deletions fail, you might miss other errors. Consider logging all errors or aggregating them to provide a complete picture of what went wrong.
| logger.error(`MailChimp deletion failed for ${member.email}: ${err.message}`) | ||
| } | ||
| // Kick off MailChimp deletion without blocking the API response. | ||
| ;(async () => { |
There was a problem hiding this comment.
[readability]
The use of an immediately invoked function expression (IIFE) to perform an asynchronous operation is unconventional and may lead to confusion. Consider using a named async function or a promise-based approach to improve readability and maintainability.
| // Kick off MailChimp deletion without blocking the API response. | ||
| ;(async () => { | ||
| try { | ||
| await mailchimp.deleteSubscriber(member.email) |
There was a problem hiding this comment.
[❗❗ correctness]
The member.email variable is used within the asynchronous function, which may lead to unexpected behavior if member is modified elsewhere before the function executes. Consider capturing member.email in a local variable before the IIFE to ensure the correct value is used.
No description provided.