fix: retry PATCH requests and 403 rate limit errors#28866
fix: retry PATCH requests and 403 rate limit errors#28866Zopsss wants to merge 1 commit intocalcom:mainfrom
Conversation
📝 WalkthroughWalkthroughThe GoogleCalendar integration's 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts (1)
217-224: Broaden retryConfig assertions to lock the full contract.This currently verifies only PATCH/POST and 403. Consider asserting
retry,noResponseRetries, and 429 too, so future retry-policy regressions don’t pass silently.Suggested test tightening
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]]), + statusCodesToRetry: expect.arrayContaining([ + [403, 403], + [429, 429], + ]), }), }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts` around lines 217 - 224, The test currently only asserts httpMethodsToRetry and 403 in the retryConfig; update the assertion against calendarMock.calendar_v3.Calendar to lock the full retry contract by also asserting the top-level retryConfig properties (e.g., retry: true), noResponseRetries (expect the intended numeric value), and that statusCodesToRetry includes both [403,403] and [429,429] in addition to the existing checks for httpMethodsToRetry containing "PATCH" and "POST" so the test fails if any of these retry-policy fields change unexpectedly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts`:
- Around line 217-224: The test currently only asserts httpMethodsToRetry and
403 in the retryConfig; update the assertion against
calendarMock.calendar_v3.Calendar to lock the full retry contract by also
asserting the top-level retryConfig properties (e.g., retry: true),
noResponseRetries (expect the intended numeric value), and that
statusCodesToRetry includes both [403,403] and [429,429] in addition to the
existing checks for httpMethodsToRetry containing "PATCH" and "POST" so the test
fails if any of these retry-policy fields change unexpectedly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec9fb3f2-56e0-40c3-9907-80c89626b819
📒 Files selected for processing (2)
packages/app-store/googlecalendar/lib/CalendarAuth.tspackages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts
What does this PR do?
Google Calendar PATCH requests (e.g. updating hangout link/description after event creation) were silently failing on transient
403 rateLimitExceedederrors and never being retried. This caused a booking/calendar desync — the booking appeared successful in Cal.com but the calendar event was incomplete or missing the Meet link.Two gaps in the default
gaxiosretry config caused this:PATCHwas not inhttpMethodsToRetry(onlyGET,HEAD,PUT,OPTIONS,DELETEwere retried by default)403was not instatusCodesToRetry— but Google's API can return either403or429for rate limit errors per their error handling docsFix: explicitly pass a
retryConfigwhen instantiating thecalendar_v3.Calendarclient inCalendarAuth.tsto override the gaxios defaults.Visual Demo (For contributors especially)
N/A — this is a silent failure fix with no visual change. The bug only surfaces under transient rate limit conditions on self-hosted instances with low-quota Google Cloud projects.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Automated test: A unit test has been added in
packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.tsthat asserts thecalendar_v3.Calendarclient is instantiated with aretryConfigcontainingPATCH/POSTinhttpMethodsToRetryand[403, 403]instatusCodesToRetry.Run it with: