Async import of the appStore packages#10
Conversation
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. |
Nitpicks 🔍
|
| bookingRefsFiltered.forEach(async (bookingRef) => { | ||
| if (bookingRef.uid) { | ||
| if (bookingRef.type.endsWith("_calendar")) { | ||
| const calendar = getCalendar(credentialsMap.get(bookingRef.type)); | ||
| const calendar = await getCalendar(credentialsMap.get(bookingRef.type)); | ||
| return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent); |
There was a problem hiding this comment.
Suggestion: The async callback passed to forEach is never awaited, so calendar/video deletions may still be in progress (or throw unhandled promise rejections) after the function continues, and the surrounding try/catch will not catch any errors from the awaited operations inside the callback. [logic error]
Severity Level: Critical 🚨
- ❌ Vital webhook reschedules but may leave calendar events active.
- ❌ WipeMyCalOther wipe may finish before external deletions complete.
- ⚠️ Errors from deleteEvent/deleteMeeting bypass Reschedule try/catch logging.| bookingRefsFiltered.forEach(async (bookingRef) => { | |
| if (bookingRef.uid) { | |
| if (bookingRef.type.endsWith("_calendar")) { | |
| const calendar = getCalendar(credentialsMap.get(bookingRef.type)); | |
| const calendar = await getCalendar(credentialsMap.get(bookingRef.type)); | |
| return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent); | |
| for (const bookingRef of bookingRefsFiltered) { | |
| if (!bookingRef.uid) continue; | |
| if (bookingRef.type.endsWith("_calendar")) { | |
| const calendar = await getCalendar(credentialsMap.get(bookingRef.type)); | |
| if (calendar) { | |
| await calendar.deleteEvent(bookingRef.uid, builder.calendarEvent); | |
| } | |
| } else if (bookingRef.type.endsWith("_video")) { | |
| await deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid); | |
| } | |
| } |
Steps of Reproduction ✅
1. Note that `Reschedule` in `packages/app-store/vital/lib/reschedule.ts:19-153` is called
from two API handlers via a queue: `packages/app-store/vital/api/webhook.ts:128-136` and
`packages/app-store/wipemycalother/api/wipe.ts:48-56`, where each booking is pushed as
`q.push(() => { return Reschedule(booking.uid, ""); })` and then `await q.start()` is
called.
2. For any booking with external integrations (i.e., with `references` that have `type`
matching a credential in `userOwner.credentials`), `Reschedule` builds
`bookingRefsFiltered` at `reschedule.ts:117-123`, so that `bookingRefsFiltered` contains
calendar (`*_calendar`) or video (`*_video`) references that should be cancelled.
3. When `Reschedule` reaches the try block at `reschedule.ts:124-135`, it executes
`bookingRefsFiltered.forEach(async (bookingRef) => { ... })`. Because
`Array.prototype.forEach` does not await asynchronous callbacks, the async function body
(which calls `await getCalendar(...)` and then `calendar?.deleteEvent(...)` or
`deleteMeeting(...)`) runs in fire-and-forget promises; `Reschedule` immediately continues
past line 135 to the email-sending block at `reschedule.ts:141-149` and then returns
`true` at line 151 without waiting for any deletions to finish.
4. As a result, on any real call path (e.g., the Vital webhook handler
`packages/app-store/vital/api/webhook.ts:33-149` when it triggers the queue at lines
128-136, or the WipeMyCalOther endpoint `/api/integrations/wipemycalother/wipe` documented
at `wipemycalother/api/wipe.ts:15-17`), the HTTP handler awaits `q.start()` which resolves
once `Reschedule` returns, not when the deletion promises settle. Calendar/video events
may still exist or fail to delete after the API has responded, and any rejection thrown
inside the async callback (e.g., from `getCalendar` in
`packages/app-store/_utils/getCalendar.ts:9-22` or from
`calendar.deleteEvent`/`deleteMeeting`) is not caught by the surrounding `try/catch` in
`Reschedule`, since it only guards synchronous code, leading to unhandled promise
rejections for these deletions.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/app-store/vital/lib/reschedule.ts
**Line:** 125:129
**Comment:**
*Logic Error: The async callback passed to `forEach` is never awaited, so calendar/video deletions may still be in progress (or throw unhandled promise rejections) after the function continues, and the surrounding try/catch will not catch any errors from the awaited operations inside the callback.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| ); | ||
| try { | ||
| bookingRefsFiltered.forEach((bookingRef) => { | ||
| bookingRefsFiltered.forEach(async (bookingRef) => { |
There was a problem hiding this comment.
Suggestion: As in the other reschedule handler, the async callback passed to forEach is never awaited, so the calendar/video deletions can complete after the function continues and any errors from the awaited operations inside the callback will not be caught by the surrounding try/catch. [logic error]
Severity Level: Critical 🚨
- ❌ /api/integrations/wipemycalother/wipe may leave events active.
- ❌ Vital webhook rescheduling may not cancel external events.
- ⚠️ Errors from deleteEvent/deleteMeeting bypass intended logging.
- ⚠️ Reschedule resolves before calendar/video deletions complete.| @@ -458,7 +458,7 @@ async function handler(req: CustomRequest) { | |||
| bookingToDelete.user.credentials | |||
There was a problem hiding this comment.
Suggestion: In the recurring calendar cancellation path, the use of forEach(async ...) combined with awaiting inside the callback means calendar deletions are kicked off in the background and apiDeletes is populated only after each deletion finishes, so the later Promise.all(prismaPromises.concat(apiDeletes)) can run before any of these deletions are added, causing the handler to return before recurring calendar events are actually deleted. [race condition]
Severity Level: Critical 🚨
- ❌ /api/cancel leaves recurring calendar events partially undeleted.
- ⚠️ Calendar integrations show outdated events after cancellation.| (ref) => !!credentialsMap.get(ref.type) | ||
| ); | ||
| bookingRefsFiltered.forEach((bookingRef) => { | ||
| bookingRefsFiltered.forEach(async (bookingRef) => { |
There was a problem hiding this comment.
Suggestion: The async callback passed to forEach starts calendar/video deletion operations without awaiting their completion or handling rejections, so these deletions may still be in-flight (or throw unhandled promise rejections) when the function continues to send emails and webhooks, leading to inconsistent state and brittle error handling; wrap the async work in Promise.all and await it instead of using forEach with an async callback. [logic error]
Severity Level: Major ⚠️
- ❌ Reschedule API returns success before calendar/video deletions complete.
- ❌ External calendars may still show cancelled bookings as active.
- ⚠️ Possible unhandled promise rejections from calendar/video providers.|
CodeAnt AI finished running the review. |
User description
Test 2nn
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#2](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/2)*CodeAnt-AI Description
Load app-store integrations asynchronously to ensure integrations are available before use
What Changed
Impact
✅ Fewer calendar integration failures during cancellations and reschedules✅ Fewer failed video meeting operations (create/update/delete and recording fetches)✅ Payments start/refunds occur only after the payment provider has loaded💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.