Skip to content

retry rate-limited write requests #28867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
notcodesid wants to merge 1 commit into
base: main
Choose a base branch
from
+188 −25
Open

retry rate-limited write requests #28867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 59 additions & 7 deletions packages/app-store/googlecalendar/lib/CalendarAuth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { calendar_v3 } from "@googleapis/calendar";
import { OAuth2Client, JWT } from "googleapis-common";

import { triggerDelegationCredentialErrorWebhook } from "@calcom/features/webhooks/lib/triggerDelegationCredentialErrorWebhook";
import {
CalendarAppDelegationCredentialClientIdNotAuthorizedError,
CalendarAppDelegationCredentialInvalidGrantError,
CalendarAppDelegationCredentialError,
CalendarAppDelegationCredentialInvalidGrantError,
} from "@calcom/lib/CalendarAppError";
import {
APP_CREDENTIAL_SHARING_ENABLED,
Expand All @@ -16,7 +13,9 @@ import {
import logger from "@calcom/lib/logger";
import type { Prisma } from "@calcom/prisma/client";
import type { CredentialForCalendarServiceWithEmail } from "@calcom/types/Credential";

import { calendar_v3 } from "@googleapis/calendar";
import type { GaxiosError, RetryConfig } from "googleapis-common";
import { JWT, OAuth2Client } from "googleapis-common";
import { invalidateCredential } from "../../_utils/invalidateCredential";
import { OAuthManager } from "../../_utils/oauth/OAuthManager";
import { oAuthManagerHelper } from "../../_utils/oauth/oAuthManagerHelper";
Expand All @@ -25,7 +24,59 @@ import { metadata } from "../_metadata";
import { getGoogleAppKeys } from "./getGoogleAppKeys";

type DelegatedTo = NonNullable<CredentialForCalendarServiceWithEmail["delegatedTo"]>;
const log = logger.getSubLogger({ prefix: ["app-store/googlecalendar/lib/CalendarAuth"] });
const log: typeof logger = logger.getSubLogger({ prefix: ["app-store/googlecalendar/lib/CalendarAuth"] });

const GOOGLE_CALENDAR_RETRY_CONFIG: RetryConfig = {
retry: 3,
noResponseRetries: 2,
httpMethodsToRetry: ["GET", "HEAD", "PUT", "OPTIONS", "DELETE", "PATCH", "POST"],
statusCodesToRetry: [
[100, 199],
[403, 403],
[429, 429],
[500, 599],
],
shouldRetry: (error: GaxiosError): boolean => shouldRetryGoogleCalendarRequest(error),
};

function hasRetriesRemaining(retryConfig: RetryConfig): boolean {
const currentRetryAttempt = retryConfig.currentRetryAttempt ?? 0;
const maxRetries = retryConfig.retry ?? 0;
return currentRetryAttempt < maxRetries;
}

function isRetryableStatus(status: number, retryConfig: RetryConfig): boolean {
return retryConfig.statusCodesToRetry?.some(([min, max]) => status >= min && status <= max) ?? false;
}

function isGoogleRateLimitError(error: GaxiosError): boolean {
const reason = (error.response?.data as { error?: { errors?: Array<{ reason?: string }> } } | undefined)
?.error?.errors?.[0]?.reason;

return reason === "rateLimitExceeded" || reason === "userRateLimitExceeded";
}

function shouldRetryGoogleCalendarRequest(error: GaxiosError): boolean {
const retryConfig = error.config.retryConfig;

if (!retryConfig || error.name === "AbortError") return false;
if (!hasRetriesRemaining(retryConfig)) return false;

if (!error.response) {
return (retryConfig.currentRetryAttempt ?? 0) < (retryConfig.noResponseRetries ?? 0);
}

const method = error.config.method?.toUpperCase();
if (!method || !retryConfig.httpMethodsToRetry?.includes(method)) return false;

if (!isRetryableStatus(error.response.status, retryConfig)) return false;

if (error.response.status === 403) {
return isGoogleRateLimitError(error);
}

return true;
}

class MyGoogleOAuth2Client extends OAuth2Client {
constructor(client_id: string, client_secret: string, redirect_uri: string) {
Expand Down Expand Up @@ -216,7 +267,7 @@ export class CalendarAuth {
statusText: result.statusText,
});
},
isTokenObjectUnusable: async function (response) {
isTokenObjectUnusable: async (response) => {
// TODO: Confirm that if this logic should go to isAccessTokenUnusable
if (!response.ok || (response.status < 200 && response.status >= 300)) {
const responseBody = await response.json();
Expand Down Expand Up @@ -303,6 +354,7 @@ export class CalendarAuth {

return new calendar_v3.Calendar({
auth: googleAuthClient,
retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG,
});
Comment on lines 355 to 358
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect all Calendar client instantiations and nearby arguments
rg -n -C4 'new\s+calendar_v3\.Calendar\s*\(' packages/app-store/googlecalendar

# Inspect where retryConfig is actually supplied
rg -n -C2 'retryConfig\s*:' packages/app-store/googlecalendar

Repository: calcom/cal.com

Length of output: 3988

Add retryConfig to the Calendar client instantiation in callback.ts.

The gap is real. While CalendarAuth.ts:355 correctly passes retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG, the constructor at callback.ts:89 instantiates new calendar_v3.Calendar({ auth: oAuth2Client }) without it. This inconsistency leaves the callback path unprotected if it later performs write operations or is extended to do so.

Apply the same retryConfig: GOOGLE_CALENDAR_RETRY_CONFIG to the callback.ts constructor for consistency and resilience.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/app-store/googlecalendar/lib/CalendarAuth.ts` around lines 355 -
358, The callback path instantiates the Google Calendar client without the retry
config; update the calendar_v3.Calendar construction in callback.ts (where new
calendar_v3.Calendar({ auth: oAuth2Client }) is created) to include retryConfig:
GOOGLE_CALENDAR_RETRY_CONFIG so it matches CalendarAuth.ts usage and gains the
same retry behavior; ensure the symbol GOOGLE_CALENDAR_RETRY_CONFIG is
imported/available in callback.ts before adding it.

Copy link
Copy Markdown
Author

@notcodesid notcodesid Apr 13, 2026

Choose a reason for hiding this comment

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

yo thanks for this. i checked the callback path, but it is only used during OAuth setup to read the primary calendar via getPrimaryCalendar(), not for the event write calls involved in #28834.

im keeping this PR scoped to the Google Calendar event write path retry behavior (insert / patch / update) and not expanding it to unrelated client construction paths.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

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

@notcodesid, that's a fair scoping decision — since callback.ts only calls getPrimaryCalendar() during OAuth setup (a read), the retry config for write operations isn't applicable there. Thanks for the clarification!

(\_/)
(•ᴗ•)
/>  📋 noted!
✏️ Learnings added 
Learnt from: notcodesid
Repo: calcom/cal.com PR: 28867
File: packages/app-store/googlecalendar/lib/CalendarAuth.ts:355-358
Timestamp: 2026-04-13T12:17:26.280Z
Learning: In calcom/cal.com PR `#28867`, the `callback.ts` file in `packages/app-store/googlecalendar/` instantiates `new calendar_v3.Calendar({ auth: oAuth2Client })` without a `retryConfig`. This is intentional: the callback path is only used during OAuth setup to read the primary calendar via `getPrimaryCalendar()` and does not perform any write operations (`insert`/`patch`/`update`). Do not flag the absence of `retryConfig` in `callback.ts` as a gap — it is unrelated to the Google Calendar write-path retry behavior fixed in PR `#28867`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: yuvrajangadsingh
Repo: calcom/cal.com PR: 26811
File: packages/trpc/server/routers/viewer/slots/util.ts:884-893
Timestamp: 2026-04-08T12:26:02.746Z
Learning: In calcom/cal.com PR `#26811` (feat: check guest availability during host reschedule), the `_getGuestBusyTimesForReschedule` method in `packages/trpc/server/routers/viewer/slots/util.ts` intentionally only checks Cal.com ACCEPTED/PENDING bookings (via `BookingRepository.findBookingsByUserIdsAndDateRange`) rather than full calendar availability (calendar events, schedule rules, OOO/travel). This is a deliberate MVP scoping decision by the author (yuvrajangadsingh); accessing guest calendar credentials for full availability is deferred as a follow-up. Do not flag this as a bug.

Learnt from: vijayraghav-io
Repo: calcom/cal.com PR: 16878
File: packages/app-store/larkcalendar/api/callback.ts:109-117
Timestamp: 2025-09-08T10:07:30.026Z
Learning: In office365calendar and zohocalendar callback handlers, vijayraghav-io chose to manually call BookingReferenceRepository.reconnectWithNewCredential() after credential creation instead of using SelectedCalendarRepository.upsert(), to avoid disturbing existing error handling logic in catch blocks while still ensuring booking reference reconnection happens.

Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.

Learnt from: vijayraghav-io
Repo: calcom/cal.com PR: 16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.

Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR `#22919`, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).

Learnt from: ShashwatPS
Repo: calcom/cal.com PR: 23638
File: packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts:198-199
Timestamp: 2025-09-06T11:00:34.372Z
Learning: In calcom/cal.com PR `#23638`, the maintainer ShashwatPS determined that authorization checks in the setDestinationReminder handler are "not applicable" when CodeRabbit suggested adding user-scoped WHERE clauses to prevent users from modifying other users' destination calendar reminder settings.

Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.

Learnt from: Udit-takkar
Repo: calcom/cal.com PR: 22995
File: packages/features/ee/workflows/lib/reminders/aiPhoneCallManager.ts:167-193
Timestamp: 2025-08-19T08:45:41.834Z
Learning: In calcom/cal.com AI phone call scheduling (aiPhoneCallManager.ts), workflow reminder records are intentionally kept in the database even when task scheduling fails, as they provide valuable debugging information for troubleshooting scheduling issues, per maintainer Udit-takkar in PR `#22995`.

Learnt from: din-prajapati
Repo: calcom/cal.com PR: 21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.

Learnt from: CR
Repo: calcom/cal.com PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-06T12:40:06.867Z
Learning: Applies to apps/api/v2/**/*.{ts,tsx} : When importing from `calcom/features` or `calcom/trpc` into `apps/api/v2`, re-export from `packages/platform/libraries/index.ts` and import from `calcom/platform-libraries` instead of direct imports

}
}
147 changes: 129 additions & 18 deletions packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,91 @@ import prismock from "@calcom/testing/lib/__mocks__/prisma";
import "../__mocks__/features.repository";
import "../__mocks__/getGoogleAppKeys";
import {
setCredentialsMock,
adminMock,
calendarListMock,
calendarMock,
getLastCreatedJWT,
getLastCreatedOAuth2Client,
setLastCreatedJWT,
setLastCreatedOAuth2Client,
calendarMock,
adminMock,
MOCK_JWT_TOKEN,
MOCK_OAUTH2_TOKEN,
setCredentialsMock,
setLastCreatedJWT,
setLastCreatedOAuth2Client,
} from "../__mocks__/googleapis";

import { expect, test, beforeEach, vi, describe } from "vitest";
import { beforeEach, describe, expect, test, vi } from "vitest";
import "vitest-fetch-mock";

import type { CredentialForCalendarServiceWithEmail } from "@calcom/types/Credential";

import type { GaxiosError, RetryConfig } from "googleapis-common";
import BuildCalendarService from "../CalendarService";
import {
createMockJWTInstance,
createCredentialForCalendarService,
createInMemoryDelegationCredentialForCalendarService as createInMemoryDelegationCredentialForBuildCalendarService,
createMockJWTInstance,
defaultDelegatedCredential,
createCredentialForCalendarService,
} from "./utils";

function expectJWTInstanceToBeCreated() {
function expectJWTInstanceToBeCreated(): void {
expect(getLastCreatedJWT()).toBeDefined();
expect(setCredentialsMock).not.toHaveBeenCalled();
}

function expectOAuth2InstanceToBeCreated() {
function expectOAuth2InstanceToBeCreated(): void {
expect(setCredentialsMock).toHaveBeenCalled();
expect(getLastCreatedJWT()).toBeNull();
}

function mockSuccessfulCalendarListFetch() {
function mockSuccessfulCalendarListFetch(): void {
calendarListMock.mockImplementation(() => {
return {
data: { items: [] },
};
});
}

function getCalendarRetryConfig(): RetryConfig {
const config = calendarMock.calendar_v3.Calendar.mock.calls.at(-1)?.[0]?.retryConfig as
| RetryConfig
| undefined;
expect(config).toBeDefined();
return config as RetryConfig;
}

function createGoogleApiError({
method,
status,
reason,
}: {
method: "PATCH" | "POST" | "GET";
status: number;
reason?: string;
}): GaxiosError {
const retryConfig = getCalendarRetryConfig();
let data: { error: { errors: Array<{ reason: string }> } } | undefined;

if (reason) {
data = {
error: {
errors: [{ reason }],
},
};
}

return Object.assign(new Error("Google Calendar request failed"), {
config: {
method,
retryConfig: {
...retryConfig,
currentRetryAttempt: 0,
},
},
response: {
status,
data,
},
}) as GaxiosError;
}

beforeEach(() => {
vi.clearAllMocks();
setCredentialsMock.mockClear();
Expand All @@ -56,12 +98,12 @@ beforeEach(() => {
createMockJWTInstance({});
});

async function expectNoCredentialsInDb() {
async function expectNoCredentialsInDb(): Promise<void> {
const credentials = await prismock.credential.findMany({});
expect(credentials).toHaveLength(0);
}

async function expectCredentialsInDb(credentials: CredentialForCalendarServiceWithEmail[]) {
async function expectCredentialsInDb(credentials: CredentialForCalendarServiceWithEmail[]): Promise<void> {
const credentialsInDb = await prismock.credential.findMany({});
expect(credentialsInDb.length).toBe(credentials.length);
expect(credentialsInDb).toEqual(expect.arrayContaining(credentials));
Expand Down Expand Up @@ -193,16 +235,85 @@ describe("GoogleCalendarService credential handling", () => {

expectOAuth2InstanceToBeCreated();

expect(calendarMock.calendar_v3.Calendar).toHaveBeenCalledWith({
auth: getLastCreatedOAuth2Client(),
});
expect(calendarMock.calendar_v3.Calendar).toHaveBeenCalledWith(
expect.objectContaining({
auth: getLastCreatedOAuth2Client(),
})
);
await expectCredentialsInDb([
expect.objectContaining({
id: regularCredential.id,
key: MOCK_OAUTH2_TOKEN,
}),
]);
});

test("instantiates the calendar client with retry config for Google Calendar write requests", async () => {
const regularCredential = await createCredentialForCalendarService();
mockSuccessfulCalendarListFetch();
const calendarService = BuildCalendarService(regularCredential);

await calendarService.listCalendars();

expect(calendarMock.calendar_v3.Calendar).toHaveBeenCalledWith(
expect.objectContaining({
retryConfig: expect.objectContaining({
retry: 3,
noResponseRetries: 2,
httpMethodsToRetry: expect.arrayContaining(["PATCH", "POST"]),
statusCodesToRetry: expect.arrayContaining([
[403, 403],
[429, 429],
[500, 599],
]),
}),
})
);
});

test("retries Google 403 rate limit errors for PATCH requests", async () => {
const regularCredential = await createCredentialForCalendarService();
mockSuccessfulCalendarListFetch();
const calendarService = BuildCalendarService(regularCredential);

await calendarService.listCalendars();

const retryConfig = getCalendarRetryConfig();
const shouldRetry = retryConfig.shouldRetry;

expect(shouldRetry).toBeDefined();
expect(
shouldRetry?.(
createGoogleApiError({
method: "PATCH",
status: 403,
reason: "rateLimitExceeded",
})
)
).toBe(true);
});

test("does not retry non-rate-limit 403 errors for PATCH requests", async () => {
const regularCredential = await createCredentialForCalendarService();
mockSuccessfulCalendarListFetch();
const calendarService = BuildCalendarService(regularCredential);

await calendarService.listCalendars();

const retryConfig = getCalendarRetryConfig();
const shouldRetry = retryConfig.shouldRetry;

expect(shouldRetry).toBeDefined();
expect(
shouldRetry?.(
createGoogleApiError({
method: "PATCH",
status: 403,
reason: "forbidden",
})
)
).toBe(false);
});
});

describe("Delegation Credential Error handling", () => {
Expand Down
Loading