Conversation
…tled and MailboxConcurrency rate limits. Update isRetryableError function and introduce isOutlookThrottlingError utility. Add corresponding tests for new error cases.
… message matching, improving error handling precision.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request extends Outlook throttling error detection and retry logic. A new utility function identifies Outlook throttling scenarios (429 status, ApplicationThrottled, TooManyRequests, MailboxConcurrency), integrates it into existing error classification, and expands the retry logic to recognize these additional rate-limit indicators. The version is bumped to v2.23.4. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/utils/outlook/retry.test.ts">
<violation number="1" location="apps/web/utils/outlook/retry.test.ts:83">
P2: Test doesn't properly isolate message-based detection because `status: 429` already triggers rate limit. Use a different status (e.g., 403) or omit status to ensure the test validates the MailboxConcurrency regex matching.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
|
||
| it("identifies MailboxConcurrency message as rate limit", () => { | ||
| const errorInfo = { | ||
| status: 429, |
There was a problem hiding this comment.
P2: Test doesn't properly isolate message-based detection because status: 429 already triggers rate limit. Use a different status (e.g., 403) or omit status to ensure the test validates the MailboxConcurrency regex matching.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/outlook/retry.test.ts, line 83:
<comment>Test doesn't properly isolate message-based detection because `status: 429` already triggers rate limit. Use a different status (e.g., 403) or omit status to ensure the test validates the MailboxConcurrency regex matching.</comment>
<file context>
@@ -68,6 +68,26 @@ describe("isRetryableError", () => {
+
+ it("identifies MailboxConcurrency message as rate limit", () => {
+ const errorInfo = {
+ status: 429,
+ errorMessage: "MailboxConcurrency limit exceeded",
+ };
</file context>
| status: 429, | |
| status: 403, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/utils/outlook/retry.test.ts (1)
71-89: Consider adding test coverage for the "quota exceeded" pattern.Line 130 of
retry.tsadds/quota exceeded/i.test(errorMessage)as a rate-limit indicator, but there's no test case covering this pattern. Adding a test would ensure complete coverage of the new detection logic.Example test case:
it("identifies quota exceeded message as rate limit", () => { const errorInfo = { errorMessage: "Quota exceeded for mailbox operations", }; const result = isRetryableError(errorInfo); expect(result.isRateLimit).toBe(true); expect(result.retryable).toBe(true); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/utils/error.ts(3 hunks)apps/web/utils/outlook/retry.test.ts(1 hunks)apps/web/utils/outlook/retry.ts(2 hunks)version.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Use@/path aliases for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Follow consistent naming conventions using PascalCase for components
Centralize shared types in dedicated type filesImport specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)
**/*.{ts,tsx}: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorfrom@/components/Toastfor error handling in async operations
**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls
**/*.{ts,tsx}: For early access feature flags, create hooks using the naming conventionuse[FeatureName]Enabledthat return a boolean fromuseFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming conventionuse[FeatureName]Variantthat define variant types, useuseFeatureFlagVariantKey()with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g.,inbox-cleaner,pricing-options-2)
Always define types for A/B test variant flags (e.g.,type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the!postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Useas constinstead of literal types and type annotations
Use eitherT[]orArray<T>consistently
Initialize each enum member value explicitly
Useexport typefor types
Use `impo...
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/{server,api,actions,utils}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/{server,api,actions,utils}/**/*.ts: UsecreateScopedLoggerfrom "@/utils/logger" for logging in backend code
Add thecreateScopedLoggerinstantiation at the top of the file with an appropriate scope name
Use.with()method to attach context variables only within specific functions, not on global loggers
For large functions with reused variables, usecreateScopedLogger().with()to attach context once and reuse the logger without passing variables repeatedly
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)
Always import Prisma enums from
@/generated/prisma/enumsinstead of@/generated/prisma/clientto avoid Next.js bundling errors in client componentsImport Prisma using the project's centralized utility:
import prisma from '@/utils/prisma'
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma'sselectoption. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. AllfindUnique/findFirstcalls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
AllfindManyqueries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g.,emailAccount: { id: emailAccountId }) to validate ownership
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Usenext/imagepackage for images
For API GET requests to server, use theswrpackage with hooks likeuseSWRto fetch data
For text inputs, use theInputcomponent withregisterPropsfor form integration and error handling
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Implement responsive design with Tailwind CSS using a mobile-first approach
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
!(pages/_document).{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't use the next/head module in pages/_document.js on Next.js projects
Files:
apps/web/utils/error.tsversion.txtapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g.,import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
**/*.test.{ts,tsx}: Usevitestfor testing the application
Tests should be colocated next to the tested file with.test.tsor.test.tsxextension (e.g.,dir/format.tsanddir/format.test.ts)
Mockserver-onlyusingvi.mock("server-only", () => ({}))
Mock Prisma usingvi.mock("@/utils/prisma")and import the mock from@/utils/__mocks__/prisma
Usevi.clearAllMocks()inbeforeEachto clean up mocks between tests
Each test should be independent
Use descriptive test names
Mock external dependencies in tests
Do not mock the Logger
Avoid testing implementation details
Use test helpersgetEmail,getEmailAccount, andgetRulefrom@/__tests__/helpersfor mocking emails, accounts, and rules
Files:
apps/web/utils/outlook/retry.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Don't nest describe() blocks too deeply in test files
Don't use callbacks in asynchronous tests and hooks
Don't have duplicate hooks in describe blocks
Don't use export or module.exports in test files
Don't use focused tests
Make sure the assertion function, like expect, is placed inside an it() function call
Don't use disabled tests
Files:
apps/web/utils/outlook/retry.test.ts
🧠 Learnings (9)
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using `withRetry`
Applied to files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.test.tsapps/web/utils/outlook/retry.ts
📚 Learning: 2025-11-25T14:39:27.909Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:27.909Z
Learning: Applies to **/app/api/**/*.ts : Maintain consistent error response format across all API routes to avoid information disclosure while providing meaningful error feedback
Applied to files:
apps/web/utils/error.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Applied to files:
apps/web/utils/error.tsapps/web/utils/outlook/retry.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must import from `zod` for schema validation, use `createScopedLogger` from `@/utils/logger`, `chatCompletionObject` and `createGenerateObject` from `@/utils/llms`, and import `EmailAccountWithAI` type from `@/utils/llms/types`
Applied to files:
apps/web/utils/error.ts
📚 Learning: 2025-11-25T14:42:11.919Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-25T14:42:11.919Z
Learning: Applies to utils/**/*.{js,ts,jsx,tsx} : The `utils` folder contains core app logic such as Next.js Server Actions and Gmail API requests
Applied to files:
apps/web/utils/error.ts
📚 Learning: 2025-11-25T14:37:09.306Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/fullstack-workflow.mdc:0-0
Timestamp: 2025-11-25T14:37:09.306Z
Learning: Applies to apps/web/components/**/*Form*.tsx : Use React Hook Form with Zod validation (`zodResolver`) for form handling, with form components using `register`, `handleSubmit`, and error handling from the hook
Applied to files:
apps/web/utils/error.ts
📚 Learning: 2025-11-25T14:37:11.434Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-11-25T14:37:11.434Z
Learning: Applies to **/app/**/route.ts : Always wrap GET API route handlers with `withAuth` or `withEmailAccount` middleware for consistent error handling and authentication in Next.js App Router
Applied to files:
apps/web/utils/error.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers
Applied to files:
apps/web/utils/outlook/retry.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helpers `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for mocking emails, accounts, and rules
Applied to files:
apps/web/utils/outlook/retry.test.ts
🧬 Code graph analysis (1)
apps/web/utils/outlook/retry.test.ts (1)
apps/web/utils/outlook/retry.ts (1)
isRetryableError(116-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
version.txt (1)
1-1: Version bump looks appropriate.The patch-level version increment aligns with the addition of enhanced Outlook throttling detection without breaking changes.
apps/web/utils/outlook/retry.test.ts (2)
71-79: LGTM! ApplicationThrottled detection test is correct.The test properly validates that
ApplicationThrottledcode is recognized as a rate-limit error. Note that the error message also contains "MailboxConcurrency", which means this test validates both the code-based and message-based detection paths simultaneously—this is realistic for actual Outlook errors.
81-89: LGTM! MailboxConcurrency message detection test is correct.The test validates the regex-based message matching for MailboxConcurrency, which aligns with the PR title's focus on improving precision through regex.
apps/web/utils/outlook/retry.ts (1)
16-16: Documentation accurately reflects expanded throttling detection.The comment now lists all Outlook throttling indicators, improving code maintainability.
apps/web/utils/error.ts (4)
119-130: LGTM! Outlook throttling detection is well-implemented.The function safely handles type checking with optional chaining and validates the message type before applying the regex. The case-insensitive regex for
MailboxConcurrencyimproves detection precision as stated in the PR title.
146-146: Proper integration into known API error detection.Outlook throttling is now correctly treated as a known API error (won't be sent to Sentry), consistent with Gmail rate limits and other provider-specific errors.
190-198: Excellent error handling with user-friendly messaging.The error response provides a clear, actionable message without exposing internal details, and uses the correct 429 status code. The logging includes context (url) for debugging while maintaining security best practices.
119-130: The suggested change conflates two different error types and is not recommended.While
retry.tsincludes/quota exceeded/iinisRetryableError, this is appropriate for its broader scope. However, "quota exceeded" is a storage/resource limit error, not a throttling error. TheisOutlookThrottlingErrorfunction specifically classifies rate-limit and concurrency throttling (429 status,ApplicationThrottled,TooManyRequests,MailboxConcurrency). Adding "quota exceeded" would semantically misclassify a different error type. The current divergence is intentional:isRetryableErrorincludes errors worth retrying, whileisOutlookThrottlingErrorspecifically identifies throttling errors.
1 participant
Refactor Outlook throttling handling by using regex for “MailboxConcurrency” and detecting
statusCode === 429,code === "ApplicationThrottled"or"TooManyRequests"acrossapps/web/utils/error.tsandapps/web/utils/outlook/retry.tsAdd
isOutlookThrottlingErrorand route Outlook 429s to a standardized ApiError; expand retry logic to treatApplicationThrottledandMailboxConcurrencyas rate limits; update tests for new rate limit cases; remove unused import and bump version to v2.23.4.📍Where to Start
Start with
isOutlookThrottlingErrorin error.ts, then review its use incheckCommonErrorsand the rate limit logic inisRetryableErrorin apps/web/utils/outlook/retry.ts.Macroscope summarized 047e9fd.
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.