Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/app-store/_utils/getCalendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import appStore from "..";

const log = logger.getChildLogger({ prefix: ["CalendarManager"] });

export const getCalendar = (credential: CredentialPayload | null): Calendar | null => {
export const getCalendar = async (credential: CredentialPayload | null): Promise<Calendar | null> => {
if (!credential || !credential.key) return null;
let { type: calendarType } = credential;
if (calendarType?.endsWith("_other_calendar")) {
calendarType = calendarType.split("_other_calendar")[0];
}
const calendarApp = appStore[calendarType.split("_").join("") as keyof typeof appStore];
const calendarApp = await appStore[calendarType.split("_").join("") as keyof typeof appStore];
if (!(calendarApp && "lib" in calendarApp && "CalendarService" in calendarApp.lib)) {
log.warn(`calendar of type ${calendarType} is not implemented`);
return null;
Expand Down
88 changes: 29 additions & 59 deletions packages/app-store/index.ts
Original file line number Diff line number Diff line change
@@ -1,63 +1,33 @@
// import * as example from "./_example";
import * as applecalendar from "./applecalendar";
import * as caldavcalendar from "./caldavcalendar";
import * as closecom from "./closecom";
import * as dailyvideo from "./dailyvideo";
import * as exchange2013calendar from "./exchange2013calendar";
import * as exchange2016calendar from "./exchange2016calendar";
import * as exchangecalendar from "./exchangecalendar";
import * as facetime from "./facetime";
import * as giphy from "./giphy";
import * as googlecalendar from "./googlecalendar";
import * as googlevideo from "./googlevideo";
import * as hubspot from "./hubspot";
import * as huddle01video from "./huddle01video";
import * as jitsivideo from "./jitsivideo";
import * as larkcalendar from "./larkcalendar";
import * as office365calendar from "./office365calendar";
import * as office365video from "./office365video";
import * as plausible from "./plausible";
import * as salesforce from "./salesforce";
import * as sendgrid from "./sendgrid";
import * as stripepayment from "./stripepayment";
import * as sylapsvideo from "./sylapsvideo";
import * as tandemvideo from "./tandemvideo";
import * as vital from "./vital";
import * as wipemycalother from "./wipemycalother";
import * as zapier from "./zapier";
import * as zohocrm from "./zohocrm";
import * as zoomvideo from "./zoomvideo";

const appStore = {
// example,
applecalendar,
caldavcalendar,
closecom,
dailyvideo,
googlecalendar,
googlevideo,
hubspot,
huddle01video,
jitsivideo,
sylapsvideo,
larkcalendar,
office365calendar,
office365video,
plausible,
salesforce,
zohocrm,
sendgrid,
stripepayment,
tandemvideo,
vital,
zoomvideo,
wipemycalother,
giphy,
zapier,
exchange2013calendar,
exchange2016calendar,
exchangecalendar,
facetime,
// example: import("./example"),
applecalendar: import("./applecalendar"),
caldavcalendar: import("./caldavcalendar"),
closecom: import("./closecom"),
dailyvideo: import("./dailyvideo"),
googlecalendar: import("./googlecalendar"),
googlevideo: import("./googlevideo"),
hubspot: import("./hubspot"),
huddle01video: import("./huddle01video"),
jitsivideo: import("./jitsivideo"),
larkcalendar: import("./larkcalendar"),
office365calendar: import("./office365calendar"),
office365video: import("./office365video"),
plausible: import("./plausible"),
salesforce: import("./salesforce"),
zohocrm: import("./zohocrm"),
sendgrid: import("./sendgrid"),
stripepayment: import("./stripepayment"),
tandemvideo: import("./tandemvideo"),
vital: import("./vital"),
zoomvideo: import("./zoomvideo"),
wipemycalother: import("./wipemycalother"),
giphy: import("./giphy"),
zapier: import("./zapier"),
exchange2013calendar: import("./exchange2013calendar"),
exchange2016calendar: import("./exchange2016calendar"),
exchangecalendar: import("./exchangecalendar"),
facetime: import("./facetime"),
sylapsvideo: import("./sylapsvideo"),
};

export default appStore;
4 changes: 2 additions & 2 deletions packages/app-store/vital/lib/reschedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,10 @@ const Reschedule = async (bookingUid: string, cancellationReason: string) => {
(ref) => !!credentialsMap.get(ref.type)
);
try {
bookingRefsFiltered.forEach((bookingRef) => {
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);
Comment on lines +125 to 129

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Suggested change
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.
👍 | 👎

} else if (bookingRef.type.endsWith("_video")) {
return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
Expand Down
4 changes: 2 additions & 2 deletions packages/app-store/wipemycalother/lib/reschedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,10 @@ const Reschedule = async (bookingUid: string, cancellationReason: string) => {
(ref) => !!credentialsMap.get(ref.type)
);
try {
bookingRefsFiltered.forEach((bookingRef) => {
bookingRefsFiltered.forEach(async (bookingRef) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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);
Expand Down
15 changes: 8 additions & 7 deletions packages/core/CalendarManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const getCalendarCredentials = (credentials: Array<CredentialPayload>) =>
const calendar = getCalendar(credential);
return app.variant === "calendar" ? [{ integration: app, credential, calendar }] : [];
});

return credentials.length ? credentials : [];
});

Expand All @@ -43,8 +44,8 @@ export const getConnectedCalendars = async (
const connectedCalendars = await Promise.all(
calendarCredentials.map(async (item) => {
try {
const { calendar, integration, credential } = item;

const { integration, credential } = item;
const calendar = await item.calendar;
// Don't leak credentials to the client
const credentialId = credential.id;
if (!calendar) {
Expand Down Expand Up @@ -138,7 +139,7 @@ export const getCachedResults = async (
selectedCalendars: SelectedCalendar[]
): Promise<EventBusyDate[][]> => {
const calendarCredentials = withCredentials.filter((credential) => credential.type.endsWith("_calendar"));
const calendars = calendarCredentials.map((credential) => getCalendar(credential));
const calendars = await Promise.all(calendarCredentials.map((credential) => getCalendar(credential)));
performance.mark("getBusyCalendarTimesStart");
const results = calendars.map(async (c, i) => {
/** Filter out nulls */
Expand Down Expand Up @@ -229,7 +230,7 @@ export const createEvent = async (
calEvent: CalendarEvent
): Promise<EventResult<NewCalendarEventType>> => {
const uid: string = getUid(calEvent);
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
let success = true;
let calError: string | undefined = undefined;

Expand Down Expand Up @@ -280,7 +281,7 @@ export const updateEvent = async (
externalCalendarId: string | null
): Promise<EventResult<NewCalendarEventType>> => {
const uid = getUid(calEvent);
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
let success = false;
let calError: string | undefined = undefined;
let calWarnings: string[] | undefined = [];
Expand Down Expand Up @@ -326,12 +327,12 @@ export const updateEvent = async (
};
};

export const deleteEvent = (
export const deleteEvent = async (
credential: CredentialPayload,
uid: string,
event: CalendarEvent
): Promise<unknown> => {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
return calendar.deleteEvent(uid, event);
}
Expand Down
3 changes: 1 addition & 2 deletions packages/core/EventManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,7 @@ export default class EventManager {
id: oldCalendarEvent.credentialId,
},
});
const calendar = getCalendar(calendarCredential);

const calendar = await getCalendar(calendarCredential);
await calendar?.deleteEvent(oldCalendarEvent.uid, event, oldCalendarEvent.externalCalendarId);
}
}
Expand Down
36 changes: 20 additions & 16 deletions packages/core/videoClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,25 @@ const log = logger.getChildLogger({ prefix: ["[lib] videoClient"] });
const translator = short();

// factory
const getVideoAdapters = (withCredentials: CredentialPayload[]): VideoApiAdapter[] =>
withCredentials.reduce<VideoApiAdapter[]>((acc, cred) => {
const getVideoAdapters = async (withCredentials: CredentialPayload[]): Promise<VideoApiAdapter[]> => {
const videoAdapters: VideoApiAdapter[] = [];

for (const cred of withCredentials) {
const appName = cred.type.split("_").join(""); // Transform `zoom_video` to `zoomvideo`;
const app = appStore[appName as keyof typeof appStore];
const app = await appStore[appName as keyof typeof appStore];

if (app && "lib" in app && "VideoApiAdapter" in app.lib) {
const makeVideoApiAdapter = app.lib.VideoApiAdapter as VideoApiAdapterFactory;
const videoAdapter = makeVideoApiAdapter(cred);
acc.push(videoAdapter);
return acc;
videoAdapters.push(videoAdapter);
}
return acc;
}, []);
}

return videoAdapters;
};

const getBusyVideoTimes = (withCredentials: CredentialPayload[]) =>
Promise.all(getVideoAdapters(withCredentials).map((c) => c?.getAvailability())).then((results) =>
const getBusyVideoTimes = async (withCredentials: CredentialPayload[]) =>
Promise.all((await getVideoAdapters(withCredentials)).map((c) => c?.getAvailability())).then((results) =>
results.reduce((acc, availability) => acc.concat(availability), [] as (EventBusyDate | undefined)[])
);

Expand All @@ -45,7 +49,7 @@ const createMeeting = async (credential: CredentialWithAppName, calEvent: Calend
);
}

const videoAdapters = getVideoAdapters([credential]);
const videoAdapters = await getVideoAdapters([credential]);
const [firstVideoAdapter] = videoAdapters;
let createdMeeting;
let returnObject: {
Expand Down Expand Up @@ -104,7 +108,7 @@ const updateMeeting = async (

let success = true;

const [firstVideoAdapter] = getVideoAdapters([credential]);
const [firstVideoAdapter] = await getVideoAdapters([credential]);
const updatedMeeting =
credential && bookingRef
? await firstVideoAdapter?.updateMeeting(bookingRef, calEvent).catch(async (e) => {
Expand Down Expand Up @@ -135,9 +139,9 @@ const updateMeeting = async (
};
};

const deleteMeeting = (credential: CredentialPayload, uid: string): Promise<unknown> => {
const deleteMeeting = async (credential: CredentialPayload, uid: string): Promise<unknown> => {
if (credential) {
const videoAdapter = getVideoAdapters([credential])[0];
const videoAdapter = (await getVideoAdapters([credential]))[0];
// There are certain video apps with no video adapter defined. e.g. riverby,whereby
if (videoAdapter) {
return videoAdapter.deleteMeeting(uid);
Expand All @@ -155,7 +159,7 @@ const createMeetingWithCalVideo = async (calEvent: CalendarEvent) => {
} catch (e) {
return;
}
const [videoAdapter] = getVideoAdapters([
const [videoAdapter] = await getVideoAdapters([
{
id: 0,
appId: "daily-video",
Expand All @@ -178,7 +182,7 @@ const getRecordingsOfCalVideoByRoomName = async (
console.error("Error: Cal video provider is not installed.");
return;
}
const [videoAdapter] = getVideoAdapters([
const [videoAdapter] = await getVideoAdapters([
{
id: 0,
appId: "daily-video",
Expand All @@ -199,7 +203,7 @@ const getDownloadLinkOfCalVideoByRecordingId = async (recordingId: string) => {
console.error("Error: Cal video provider is not installed.");
return;
}
const [videoAdapter] = getVideoAdapters([
const [videoAdapter] = await getVideoAdapters([
{
id: 0,
appId: "daily-video",
Expand Down
23 changes: 12 additions & 11 deletions packages/features/bookings/lib/handleCancelBooking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ async function handler(req: CustomRequest) {
integrationsToDelete.push(deleteMeeting(credential, reference.uid));
}
if (reference.type.includes("_calendar")) {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
integrationsToDelete.push(
calendar?.deleteEvent(reference.uid, evt, reference.externalCalendarId)
Expand All @@ -262,7 +262,7 @@ async function handler(req: CustomRequest) {
);
}
if (reference.type.includes("_calendar")) {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
integrationsToDelete.push(
calendar?.updateEvent(reference.uid, updatedEvt, reference.externalCalendarId)
Expand Down Expand Up @@ -449,7 +449,7 @@ async function handler(req: CustomRequest) {
(credential) => credential.id === credentialId
);
if (calendarCredential) {
const calendar = getCalendar(calendarCredential);
const calendar = await getCalendar(calendarCredential);
if (
bookingToDelete.eventType?.recurringEvent &&
bookingToDelete.recurringEventId &&
Expand All @@ -458,7 +458,7 @@ async function handler(req: CustomRequest) {
bookingToDelete.user.credentials

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.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) {
Expand All @@ -474,12 +474,13 @@ async function handler(req: CustomRequest) {
}
} else {
// For bookings made before the refactor we go through the old behaviour of running through each calendar credential
bookingToDelete.user.credentials
.filter((credential) => credential.type.endsWith("_calendar"))
.forEach((credential) => {
const calendar = getCalendar(credential);
apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>);
});
const calendarCredentials = bookingToDelete.user.credentials.filter((credential) =>
credential.type.endsWith("_calendar")
);
for (const credential of calendarCredentials) {
const calendar = await getCalendar(credential);
apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>);
}
}
}

Expand Down Expand Up @@ -585,7 +586,7 @@ async function handler(req: CustomRequest) {
}

// Posible to refactor TODO:
const paymentApp = appStore[paymentAppCredential?.app?.dirName as keyof typeof appStore];
const paymentApp = await appStore[paymentAppCredential?.app?.dirName as keyof typeof appStore];
if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
console.warn(`payment App service of type ${paymentApp} is not implemented`);
return null;
Expand Down
2 changes: 1 addition & 1 deletion packages/features/bookings/lib/handleNewBooking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ async function handler(
integrationsToDelete.push(deleteMeeting(credential, reference.uid));
}
if (reference.type.includes("_calendar") && originalBookingEvt) {
const calendar = getCalendar(credential);
const calendar = await getCalendar(credential);
if (calendar) {
integrationsToDelete.push(
calendar?.deleteEvent(reference.uid, originalBookingEvt, reference.externalCalendarId)
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/payment/deletePayment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const deletePayment = async (
} | null;
}
): Promise<boolean> => {
const paymentApp = appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
console.warn(`payment App service of type ${paymentApp} is not implemented`);
return false;
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/payment/handlePayment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const handlePayment = async (
uid: string;
}
) => {
const paymentApp = appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
const paymentApp = await appStore[paymentAppCredentials?.app?.dirName as keyof typeof appStore];
if (!(paymentApp && "lib" in paymentApp && "PaymentService" in paymentApp.lib)) {
console.warn(`payment App service of type ${paymentApp} is not implemented`);
return null;
Expand Down
Loading