Skip to content

Commit 5ddb4dc

Browse files
volneivolnei
authored
Add calendar subscription error better management (calcom#27019)
1 parent 6c3b106 commit 5ddb4dc

12 files changed

Lines changed: 210 additions & 27 deletions

File tree

‎apps/api/v1/test/lib/selected-calendars/_post.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ describe("POST /api/selected-calendars", () => {
102102
channelResourceUri: null,
103103
channelExpiration: null,
104104
syncSubscribedAt: null,
105+
syncSubscribedErrorAt: null,
106+
syncSubscribedErrorCount: 0,
105107
syncToken: null,
106108
syncedAt: null,
107109
syncErrorAt: null,
@@ -157,6 +159,8 @@ describe("POST /api/selected-calendars", () => {
157159
channelResourceUri: null,
158160
channelExpiration: null,
159161
syncSubscribedAt: null,
162+
syncSubscribedErrorAt: null,
163+
syncSubscribedErrorCount: 0,
160164
syncToken: null,
161165
syncedAt: null,
162166
syncErrorAt: null,

‎packages/features/calendar-subscription/adapters/__tests__/GoogleCalendarSubscriptionAdapter.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ const mockSelectedCalendar: SelectedCalendar = {
5252
channelResourceUri: "test-resource-uri",
5353
channelExpiration: channelExpirationDate,
5454
syncSubscribedAt: today.toDate(),
55+
syncSubscribedErrorAt: null,
56+
syncSubscribedErrorCount: 0,
5557
syncToken: "test-sync-token",
5658
syncedAt: today.toDate(),
5759
syncErrorAt: null,

‎packages/features/calendar-subscription/adapters/__tests__/Office365CalendarSubscriptionAdapter.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ const _mockSelectedCalendar: SelectedCalendar = {
2929
channelResourceUri: "test-resource-uri",
3030
channelExpiration: new Date(Date.now() + 86400000),
3131
syncSubscribedAt: new Date(),
32+
syncSubscribedErrorAt: null,
33+
syncSubscribedErrorCount: 0,
3234
syncToken: "test-sync-token",
3335
syncedAt: new Date(),
3436
syncErrorAt: null,

‎packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const log = logger.getSubLogger({ prefix: ["CalendarSubscriptionService"] });
2121
export class CalendarSubscriptionService {
2222
static CALENDAR_SUBSCRIPTION_CACHE_FEATURE = "calendar-subscription-cache" as const;
2323
static CALENDAR_SUBSCRIPTION_SYNC_FEATURE = "calendar-subscription-sync" as const;
24+
static MAX_SUBSCRIBE_ERRORS = 3;
2425

2526
constructor(
2627
private deps: {
@@ -54,19 +55,41 @@ export class CalendarSubscriptionService {
5455
return;
5556
}
5657

58+
const subscribeErrorCount = selectedCalendar.syncSubscribedErrorCount ?? 0;
59+
if (subscribeErrorCount >= CalendarSubscriptionService.MAX_SUBSCRIBE_ERRORS) {
60+
log.debug("Selected calendar exceeded subscribe error threshold", { selectedCalendarId });
61+
return;
62+
}
63+
5764
const calendarSubscriptionAdapter = this.deps.adapterFactory.get(
5865
selectedCalendar.integration as CalendarSubscriptionProvider
5966
);
60-
const res = await calendarSubscriptionAdapter.subscribe(selectedCalendar, credential);
61-
62-
await this.deps.selectedCalendarRepository.updateSubscription(selectedCalendarId, {
63-
channelId: res?.id,
64-
channelResourceId: res?.resourceId,
65-
channelResourceUri: res?.resourceUri,
66-
channelKind: res?.provider,
67-
channelExpiration: res?.expiration,
68-
syncSubscribedAt: new Date(),
69-
});
67+
68+
try {
69+
const res = await calendarSubscriptionAdapter.subscribe(selectedCalendar, credential);
70+
await this.deps.selectedCalendarRepository.updateSubscription(selectedCalendarId, {
71+
channelId: res?.id,
72+
channelResourceId: res?.resourceId,
73+
channelResourceUri: res?.resourceUri,
74+
channelKind: res?.provider,
75+
channelExpiration: res?.expiration,
76+
syncSubscribedAt: new Date(),
77+
syncSubscribedErrorAt: null,
78+
syncSubscribedErrorCount: 0,
79+
});
80+
} catch (error: unknown) {
81+
const nextErrorCount = Math.min(
82+
CalendarSubscriptionService.MAX_SUBSCRIBE_ERRORS,
83+
(selectedCalendar.syncSubscribedErrorCount ?? 0) + 1
84+
);
85+
await this.deps.selectedCalendarRepository.updateSubscription(selectedCalendarId, {
86+
// don't need to cleanup other fields as they will only be used if syncSubscribedAt is not null
87+
syncSubscribedAt: null,
88+
syncSubscribedErrorAt: new Date(),
89+
syncSubscribedErrorCount: nextErrorCount,
90+
});
91+
throw error;
92+
}
7093

7194
await this.processEvents(selectedCalendar);
7295
}

‎packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ const mockSelectedCalendar: SelectedCalendar = {
4545
channelResourceUri: "test-resource-uri",
4646
channelExpiration: new Date(Date.now() + 86400000),
4747
syncSubscribedAt: new Date(),
48+
syncSubscribedErrorAt: null,
49+
syncSubscribedErrorCount: 0,
4850
syncToken: "test-sync-token",
4951
syncedAt: new Date(),
5052
syncErrorAt: null,
@@ -160,6 +162,8 @@ describe("CalendarSubscriptionService", () => {
160162
calendarSyncService: mockCalendarSyncService,
161163
});
162164

165+
mockSelectedCalendar.syncSubscribedErrorCount = 0;
166+
163167
const { getCredentialForSelectedCalendar } = await import("../__mocks__/delegationCredential");
164168
getCredentialForSelectedCalendar.mockResolvedValue(mockCredential);
165169
});
@@ -178,7 +182,34 @@ describe("CalendarSubscriptionService", () => {
178182
channelKind: "google_calendar",
179183
channelExpiration: mockSubscriptionResult.expiration,
180184
syncSubscribedAt: expect.any(Date),
185+
syncSubscribedErrorAt: null,
186+
syncSubscribedErrorCount: 0,
187+
});
188+
});
189+
190+
test("should record subscribe errors and retry later", async () => {
191+
const subscribeError = new Error("subscribe failed");
192+
mockAdapter.subscribe.mockRejectedValue(subscribeError);
193+
194+
await expect(service.subscribe("test-calendar-id")).rejects.toThrow(subscribeError);
195+
196+
expect(mockSelectedCalendarRepository.updateSubscription).toHaveBeenCalledWith("test-calendar-id", {
197+
syncSubscribedAt: null,
198+
syncSubscribedErrorAt: expect.any(Date),
199+
syncSubscribedErrorCount: 1,
200+
});
201+
});
202+
203+
test("should skip subscription attempts after hitting error threshold", async () => {
204+
mockSelectedCalendarRepository.findById.mockResolvedValue({
205+
...mockSelectedCalendar,
206+
syncSubscribedErrorCount: CalendarSubscriptionService.MAX_SUBSCRIBE_ERRORS,
181207
});
208+
209+
await service.subscribe("test-calendar-id");
210+
211+
expect(mockAdapter.subscribe).not.toHaveBeenCalled();
212+
expect(mockSelectedCalendarRepository.updateSubscription).not.toHaveBeenCalled();
182213
});
183214

184215
test("should return early if selected calendar not found", async () => {

‎packages/features/calendar-subscription/lib/cache/__tests__/CalendarCacheEventService.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ const mockSelectedCalendar: SelectedCalendar = {
3333
channelResourceUri: "test-resource-uri",
3434
channelExpiration: new Date(Date.now() + 86400000),
3535
syncSubscribedAt: new Date(),
36+
syncSubscribedErrorAt: null,
37+
syncSubscribedErrorCount: 0,
3638
syncToken: "test-sync-token",
3739
syncedAt: new Date(),
3840
syncErrorAt: null,

‎packages/features/calendars/lib/getCalendarsEvents.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,18 @@ function buildSelectedCalendar(credential: {
9999
watchAttempts: 0,
100100
unwatchAttempts: 0,
101101
maxAttempts: 3,
102+
channelId: null,
103+
channelKind: null,
104+
channelResourceId: null,
105+
channelResourceUri: null,
106+
channelExpiration: null,
107+
syncSubscribedAt: null,
108+
syncSubscribedErrorAt: null,
109+
syncSubscribedErrorCount: 0,
110+
syncToken: null,
111+
syncedAt: null,
112+
syncErrorAt: null,
113+
syncErrorCount: 0,
102114
...credential,
103115
};
104116
}

‎packages/features/selectedCalendar/repositories/SelectedCalendarRepository.interface.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export interface ISelectedCalendarRepository {
1818
/**
1919
* Find next batch of selected calendars
2020
* Will check if syncSubscribedAt is null or channelExpiration is greater than current date
21+
* Calendars with recent subscription errors (last 24h) are skipped
22+
* Calendars with 3+ subscription errors are skipped entirely
2123
*
2224
* @param take the number of calendars to take
2325
* @param integrations the list of integrations
@@ -31,7 +33,7 @@ export interface ISelectedCalendarRepository {
3133
}: {
3234
take: number;
3335
teamIds: number[];
34-
integrations?: string[];
36+
integrations: string[];
3537
genericCalendarSuffixes?: string[];
3638
}): Promise<SelectedCalendar[]>;
3739

@@ -62,6 +64,8 @@ export interface ISelectedCalendarRepository {
6264
| "channelKind"
6365
| "channelExpiration"
6466
| "syncSubscribedAt"
67+
| "syncSubscribedErrorAt"
68+
| "syncSubscribedErrorCount"
6569
>
6670
): Promise<SelectedCalendar>;
6771
}

‎packages/features/selectedCalendar/repositories/SelectedCalendarRepository.test.ts‎

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ const mockSelectedCalendar: SelectedCalendar = {
4040
channelResourceUri: "test-resource-uri",
4141
channelExpiration: new Date(Date.now() + 86400000),
4242
syncSubscribedAt: new Date(),
43+
syncSubscribedErrorAt: null,
44+
syncSubscribedErrorCount: 0,
4345
syncToken: "test-sync-token",
4446
syncedAt: new Date(),
4547
syncErrorAt: null,
@@ -122,7 +124,6 @@ describe("SelectedCalendarRepository", () => {
122124
expect(mockPrismaClient.selectedCalendar.findMany).toHaveBeenCalledWith({
123125
where: {
124126
integration: { in: ["google_calendar", "office365_calendar"] },
125-
OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: expect.any(Date) } }],
126127
user: {
127128
teams: {
128129
some: {
@@ -131,7 +132,24 @@ describe("SelectedCalendarRepository", () => {
131132
},
132133
},
133134
},
134-
AND: undefined,
135+
AND: [
136+
{
137+
OR: [
138+
{ syncSubscribedAt: null },
139+
{ channelExpiration: null },
140+
{ channelExpiration: { lte: expect.any(Date) } },
141+
],
142+
},
143+
{
144+
OR: [
145+
{ syncSubscribedErrorAt: null },
146+
{ syncSubscribedErrorAt: { lt: expect.any(Date) } },
147+
],
148+
},
149+
{
150+
syncSubscribedErrorCount: { lt: 3 },
151+
},
152+
],
135153
},
136154
take: 10,
137155
});
@@ -152,7 +170,6 @@ describe("SelectedCalendarRepository", () => {
152170
expect(mockPrismaClient.selectedCalendar.findMany).toHaveBeenCalledWith({
153171
where: {
154172
integration: { in: [] },
155-
OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: expect.any(Date) } }],
156173
user: {
157174
teams: {
158175
some: {
@@ -161,7 +178,24 @@ describe("SelectedCalendarRepository", () => {
161178
},
162179
},
163180
},
164-
AND: undefined,
181+
AND: [
182+
{
183+
OR: [
184+
{ syncSubscribedAt: null },
185+
{ channelExpiration: null },
186+
{ channelExpiration: { lte: expect.any(Date) } },
187+
],
188+
},
189+
{
190+
OR: [
191+
{ syncSubscribedErrorAt: null },
192+
{ syncSubscribedErrorAt: { lt: expect.any(Date) } },
193+
],
194+
},
195+
{
196+
syncSubscribedErrorCount: { lt: 3 },
197+
},
198+
],
165199
},
166200
take: 5,
167201
});
@@ -182,7 +216,6 @@ describe("SelectedCalendarRepository", () => {
182216
expect(mockPrismaClient.selectedCalendar.findMany).toHaveBeenCalledWith({
183217
where: {
184218
integration: { in: ["google_calendar"] },
185-
OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: expect.any(Date) } }],
186219
user: {
187220
teams: {
188221
some: {
@@ -191,7 +224,24 @@ describe("SelectedCalendarRepository", () => {
191224
},
192225
},
193226
},
194-
AND: undefined,
227+
AND: [
228+
{
229+
OR: [
230+
{ syncSubscribedAt: null },
231+
{ channelExpiration: null },
232+
{ channelExpiration: { lte: expect.any(Date) } },
233+
],
234+
},
235+
{
236+
OR: [
237+
{ syncSubscribedErrorAt: null },
238+
{ syncSubscribedErrorAt: { lt: expect.any(Date) } },
239+
],
240+
},
241+
{
242+
syncSubscribedErrorCount: { lt: 3 },
243+
},
244+
],
195245
},
196246
take: 10,
197247
});
@@ -215,7 +265,6 @@ describe("SelectedCalendarRepository", () => {
215265
expect(mockPrismaClient.selectedCalendar.findMany).toHaveBeenCalledWith({
216266
where: {
217267
integration: { in: ["google_calendar"] },
218-
OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: expect.any(Date) } }],
219268
user: {
220269
teams: {
221270
some: {
@@ -225,6 +274,22 @@ describe("SelectedCalendarRepository", () => {
225274
},
226275
},
227276
AND: [
277+
{
278+
OR: [
279+
{ syncSubscribedAt: null },
280+
{ channelExpiration: null },
281+
{ channelExpiration: { lte: expect.any(Date) } },
282+
],
283+
},
284+
{
285+
OR: [
286+
{ syncSubscribedErrorAt: null },
287+
{ syncSubscribedErrorAt: { lt: expect.any(Date) } },
288+
],
289+
},
290+
{
291+
syncSubscribedErrorCount: { lt: 3 },
292+
},
228293
{ NOT: { externalId: { endsWith: "@group.v.calendar.google.com" } } },
229294
{ NOT: { externalId: { endsWith: "@group.calendar.google.com" } } },
230295
],

0 commit comments

Comments
 (0)