Async import of the appStore packages #10
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| bookingToDelete.user.credentials | ||
| .filter((credential) => credential.type.endsWith("_calendar")) | ||
| .forEach(async (credential) => { | ||
| const calendar = getCalendar(credential); | ||
| const calendar = await getCalendar(credential); | ||
| for (const updBooking of updatedBookings) { | ||
| const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar")); | ||
| if (bookingRef) { |
There was a problem hiding this comment.
The new async callback in bookingToDelete.user.credentials.filter(...).forEach(async credential => …) never has its Promise observed, yet the code still calls Promise.all(prismaPromises.concat(apiDeletes)) immediately afterwards. Because the callback now awaits getCalendar before pushing into apiDeletes, the array is still empty when Promise.all runs, so recurring-booking deletions are no longer awaited and any rejection from getCalendar/deleteEvent becomes an unhandled promise rejection. This means we can return from cancellation while external events are still present (or fail silently). Iterate with for…of or collect the promises and await them before proceeding.
Useful? React with 👍 / 👎.
| 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, |
There was a problem hiding this comment.
bookingRefsFiltered.forEach(async bookingRef => { … await getCalendar … }) launches async work whose Promise is never awaited or collected. As soon as the forEach finishes (which is immediate), the handler proceeds to send emails and return, so calendar/video deletions might still be pending and any rejection from getCalendar/deleteEvent becomes an unhandled rejection. Before this change the callback was synchronous so deletions were at least kicked off before continuing. Please convert this loop to an awaited for…of/Promise.all so reschedules actually wait for the integrations to be cleaned up and errors are surfaced.
Useful? React with 👍 / 👎.
| 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); | ||
| } else if (bookingRef.type.endsWith("_video")) { | ||
| return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid); |
There was a problem hiding this comment.
In both wipemycalother (and likewise packages/app-store/vital/lib/reschedule.ts around the same lines) the cancellation logic is now bookingRefsFiltered.forEach(async bookingRef => … await getCalendar …). Because forEach doesn’t await its async callback, the surrounding try/catch never sees errors thrown inside and the function can return before the calendar/video deletions actually execute, leaving unhandled rejections when getCalendar or deleteEvent fails. To restore the original semantics, iterate with for…of (awaiting each deletion) or collect the promises and await Promise.all so that errors are caught and the reschedule waits for the clean-up to finish.
Useful? React with 👍 / 👎.
Test 2
Replicated from ai-code-review-evaluation/cal.com-cursor#2