chore: parallelize follow-up email sending with Promise.all #1239
Conversation
Previously, follow-up emails were sent sequentially inside a for...of loop, leading to slower execution. This change uses Promise.all to send emails in parallel, significantly improving the speed of the process.
|
@0xajinkya is attempting to deploy a commit to the OpenStatus Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for you PR My concern if we parallelize every send email we will hit resend rate limit |
Hey @thibaultleouay, I am looking into this. Will keep you updated. |
Improve the follow-up email system by: - Optimizing database queries to select only necessary email fields - Implementing batched email sending (80 emails per batch) - Adding proper rate limit detection and handling - Improving error logging and recovery
|
Hey @thibaultleouay, Can you look here? Following are the key changes:
These changes should make the follow-up email process more reliable and prevent issues when sending to large numbers of users. The batching approach also provides better visibility into the sending process and enables easier recovery when errors occur. |
apps/workflows/src/cron/emails.ts
Outdated
| // Chunk emails into batches of 100 | ||
| const batchSize = 80; | ||
| for (let i = 0; i < validEmails.length; i += batchSize) { | ||
| const batch = validEmails.slice(i, i + batchSize) as string[]; |
There was a problem hiding this comment.
not a huge fan of as string[], cant we auto infer it instead?
There was a problem hiding this comment.
Latest change takes care of this. Please have a look.
packages/emails/src/client.tsx
Outdated
| throw result.error; | ||
| } | ||
|
|
||
| console.error(`Batch send error:`, result.error); |
There was a problem hiding this comment.
I would also log the error above move this after line 59
There was a problem hiding this comment.
The thing is, if the batch send fails specifically due to rate limit, it is intuitional that subsequent batches will fail as well. So, that's why I am not logging that particular error. And, instead handling it in the loop itself (Logging it and breaking from the loop). Any other error, are being simply consolled.
packages/emails/src/client.tsx
Outdated
| } | ||
|
|
||
| console.log(`Sent follow up email to ${req.to}`); | ||
| } catch (err: any) { |
There was a problem hiding this comment.
not sure about the try/catch
There was a problem hiding this comment.
This is explained in the latest reply to above conversation. Have a look.
The changes improve email delivery reliability by: - Enhancing email validation with stronger type guards - Adding proper break statement for rate limit errors to prevent wasteful API calls - Simplifying error flow control with early returns and consistent error handling - Restructuring try/catch blocks for better exception management - Improving TypeScript type safety with more specific error handling These modifications ensure the system gracefully handles rate limiting while providing clearer logs for debugging. Email validation now properly checks for empty strings, preventing attempts to send to invalid addresses.
|
Any updates @thibaultleouay? |
|
@0xajinkya Just pushed some update in your pr let me know if you have any question |
Initially, we were not checking type of email and calling trim() on it, this improvision check whether email is really a string and then calls trim() on it.
|
Hey @thibaultleouay, All good, added a minor fix. Also, I am wondering why are we doing this:
This seems repetitive. |
|
@0xajinkya We are filtering null first, then removing empty email |
apps/workflows/src/cron/emails.ts
Outdated
| .map((u) => u.email) | ||
| .filter((email) => email !== null) | ||
| // I don't know why but I can't have both filter at the same time | ||
| .filter((email) => typeof email === "string" && email.trim() !== ""); |
There was a problem hiding this comment.
I dont understand the typeof email here, because the type is already infered ?
There was a problem hiding this comment.
Yes, it was totally not needed. It was error from my end. Changing it.
Type of change
Description
Previously, follow-up emails were sent sequentially inside a for...of loop, leading to slower execution. This change uses Promise.all to send emails in parallel, significantly improving the speed of the process.
This PR improves the performance of the sendFollowUpEmails function by replacing the sequential email sending (using a for...of loop) with parallel sending using Promise.all.
This change speeds up the process, especially when there are many users to email, without affecting the functionality.
A picture tells a thousand words (if any)
Before this PR
After this PR
Related Issue (optional)