Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. WalkthroughRefactors error reporting to use a unified captureException(context) signature, adds Sentry tagging/user context (emailAccountId, userId/email) in middleware and request/action flows, and updates many call sites to pass structured context; also a small config change and version bump. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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.
3 issues found across 17 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/utils/ai/actions.ts">
<violation number="1" location="apps/web/utils/ai/actions.ts:360">
P2: Removing `senderEmail` from the error context reduces debugging information when investigating cold email notification failures. Consider keeping it in the `extra` object to maintain observability.</violation>
</file>
<file name="apps/web/utils/user/delete.ts">
<violation number="1" location="apps/web/utils/user/delete.ts:68">
P2: Removing `userId` from the exception context loses valuable debugging information. Consider passing userId to help identify which user's deletion failed: `captureException(error, { userId });`</violation>
<violation number="2" location="apps/web/utils/user/delete.ts:92">
P2: Removing `userId` from the exception context loses valuable debugging information for identifying which user's deletion failed.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/resend/digest/route.ts (1)
339-353: Add emailAccountId to error capture for consistency.The
captureExceptioncall on line 351 should includeemailAccountIdfor consistency with the error reporting pattern used elsewhere in this file (line 69) and throughout the PR. TheemailAccountIdis available in the function scope.🔎 Proposed fix
logger.error("Error sending digest email", { error }); - captureException(error); + captureException(error, { emailAccountId }); throw new SafeError("Error sending digest email", 500);
🧹 Nitpick comments (5)
apps/web/utils/user/delete.ts (5)
63-69: Include userId context in error reporting for consistency.The
userIdis available and relevant to this error (it's included in the logger call). Including it incaptureExceptionwould provide valuable debugging context in Sentry.🔎 Suggested improvement
deleteTinybirdAiCalls({ userId }).catch((error) => { logger.error("Error deleting Tinybird AI calls", { error, userId, }); - captureException(error); + captureException(error, { userId }); });
71-74: Include userId context for user-specific error.This error relates to clearing cached data for a specific user. Including
userIdin the context would aid debugging.🔎 Suggested improvement
clearCachedPerplexityResearchForUser(userId).catch((error) => { logger.error("Error clearing cached Perplexity research", { error }); - captureException(error); + captureException(error, { userId }); });
82-93: Include userId in error context for user resource deletion failures.When user resource deletion fails, the
userIdis essential context for debugging which user was affected. The newCaptureExceptionContextsignature supportsuserIdas a top-level field.🔎 Suggested improvement
const originalError = (failures[0] as PromiseRejectedResult)?.reason; const customError = new Error("User deletion error"); customError.cause = originalError; - captureException(customError, { extra: { failures } }); + captureException(customError, { userId, extra: { failures } }); }
94-99: Include userId context in outer error handler.This catches errors from the entire user deletion process. Including
userIdis critical for debugging which user's deletion encountered issues.🔎 Suggested improvement
} catch (error) { logger.error("Error during user resources deletion process", { error, }); - captureException(error); + captureException(error, { userId }); }
142-148: Include userId in error context for complete debugging information.Good practice including
emailAccountIdanduserEmail, butuserIdis also available in scope and should be included for complete context in Sentry.🔎 Suggested improvement
} catch (error) { logger.error("Error during database user deletion process", { error, }); - captureException(error, { emailAccountId, userEmail: email }); + captureException(error, { userId, emailAccountId, userEmail: email }); throw error; }
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/(app)/sentry-identify.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{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/app/(app)/sentry-identify.tsx
apps/web/app/(app)/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/page-structure.mdc)
apps/web/app/(app)/**/*.{ts,tsx}: Components for the page are either put inpage.tsx, or in theapps/web/app/(app)/PAGE_NAMEfolder
If we're in a deeply nested component we will useswrto fetch via API
If you need to useonClickin a component, that component is a client component and file must start withuse client
Files:
apps/web/app/(app)/sentry-identify.tsx
**/*.{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/app/(app)/sentry-identify.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Use@/path aliases for imports from project root
Follow tailwindcss patterns with prettier-plugin-tailwindcss class organization
Prefix client-side environment variables withNEXT_PUBLIC_
Leverage TypeScript inference for better developer experience with type exports from API routes
Files:
apps/web/app/(app)/sentry-identify.tsx
**/*.{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/app/(app)/sentry-identify.tsx
**/*.{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/app/(app)/sentry-identify.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.tsx: Use theLoadingContentcomponent to handle loading states instead of manual loading state management
For text areas, use theInputcomponent withtype='text',autosizeTextareaprop set to true, andregisterPropsfor form integration
Files:
apps/web/app/(app)/sentry-identify.tsx
**/*.{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/app/(app)/sentry-identify.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Make sure all dependencies are correctly specified in React hooks
Make sure all React hooks are called from the top level of component functions
Don't forget key props in iterators and collection literals
Don't define React components inside other components
Don't use event handlers on non-interactive elements
Don't assign to React component props
Don't use bothchildrenanddangerouslySetInnerHTMLprops on the same element
Don't use dangerous JSX props
Don't use Array index in keys
Don't insert comments as text nodes
Don't assign JSX properties multiple times
Don't add extra closing tags for components without children
Use<>...</>instead of<Fragment>...</Fragment>
Watch out for possible "wrong" semicolons inside JSX elements
Make sure void (self-closing) elements don't have children
Don't usetarget="_blank"withoutrel="noopener"
Don't use<img>elements in Next.js projects
Don't use<head>elements in Next.js projects
Files:
apps/web/app/(app)/sentry-identify.tsx
!(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/app/(app)/sentry-identify.tsx
**/*.{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/app/(app)/sentry-identify.tsx
apps/web/**/app/**
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Follow NextJS app router structure with (app) directory
Files:
apps/web/app/(app)/sentry-identify.tsx
apps/web/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{tsx,jsx}: Prefer functional components with hooks over class components
Use shadcn/ui components when available
Follow consistent naming conventions with PascalCase for component names
Use LoadingContent component for async data with loading and error states
Useresult?.serverErrorwithtoastErrorandtoastSuccessfor mutation error handling
Files:
apps/web/app/(app)/sentry-identify.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx,js,jsx}: Use proper error handling with try/catch blocks
Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments
Add helper functions to the bottom of files, not the top
Files:
apps/web/app/(app)/sentry-identify.tsx
apps/web/**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Format code with Prettier
Files:
apps/web/app/(app)/sentry-identify.tsx
apps/web/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Ensure responsive design with mobile-first approach
Files:
apps/web/app/(app)/sentry-identify.tsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/logging.mdc:0-0
Timestamp: 2025-12-18T16:37:47.947Z
Learning: Applies to **/{app,pages}/**/{route,+page}.{ts,tsx} : Use middleware wrappers (withError, withAuth, withEmailAccount, withEmailProvider) that automatically create loggers with request context in API routes
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 Learning: 2025-11-25T14:39:49.448Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-11-25T14:39:49.448Z
Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific emailAccountId are needed, with emailAccountId bound when calling from the client
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 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 : Use `withEmailAccount` middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using `emailAccountId`
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 Learning: 2025-12-17T11:18:06.818Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-12-17T11:18:06.818Z
Learning: Applies to apps/web/**/{app/api,utils/actions}/**/*.ts : Use `withAuth` for user-level operations and `withEmailAccount` for email-account-level operations
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 Learning: 2025-11-25T14:39:08.150Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security-audit.mdc:0-0
Timestamp: 2025-11-25T14:39:08.150Z
Learning: Applies to apps/web/app/api/**/*.{ts,tsx} : All database queries must include user scoping with `emailAccountId` or `userId` filtering in WHERE clauses
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 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/utils/actions/*.ts : Server actions should use 'use server' directive and automatically receive authentication context (`emailAccountId`) from the `actionClient`
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 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/app/(app)/sentry-identify.tsx
📚 Learning: 2025-11-25T14:39:49.448Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-11-25T14:39:49.448Z
Learning: Applies to apps/web/utils/actions/*.ts : Access context (userId, emailAccountId, etc.) via the `ctx` object parameter in the `.action()` handler
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 Learning: 2025-11-25T14:37:35.343Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:35.343Z
Learning: Applies to apps/web/hooks/use*.ts : Create dedicated hooks for specific data types (e.g., `useAccounts`, `useLabels`) to wrap `useSWR` for individual API endpoints
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 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/app/(app)/sentry-identify.tsx
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 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 : Use `withAuth` middleware to get the authenticated user or `withEmailAccount` middleware to get the currently active email account in GET API routes
Applied to files:
apps/web/app/(app)/sentry-identify.tsx
📚 Learning: 2025-11-25T14:37:22.822Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-11-25T14:37:22.822Z
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/app/(app)/sentry-identify.tsx
🧬 Code graph analysis (1)
apps/web/app/(app)/sentry-identify.tsx (1)
apps/web/providers/EmailAccountProvider.tsx (1)
useAccount(79-89)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/web/app/(app)/sentry-identify.tsx (1)
5-9: LGTM! Clean integration with EmailAccountProvider.The
useAccounthook is correctly imported and used to extractemailAccountId. The hook will appropriately throw if used outside the provider context, and since this component is in the authenticated(app)directory, the provider should be available.
| useEffect(() => { | ||
| if (emailAccountId) { | ||
| Sentry.setTag("emailAccountId", emailAccountId); | ||
| } else { | ||
| Sentry.setTag("emailAccountId", undefined); | ||
| } | ||
| }, [emailAccountId]); |
There was a problem hiding this comment.
Remove the else branch that attempts to clear the tag with undefined.
Sentry does not provide an API to remove individual tags. Instead, only set the tag when emailAccountId is truthy:
useEffect(() => {
if (emailAccountId) {
Sentry.setTag("emailAccountId", emailAccountId);
}
}, [emailAccountId]);
🤖 Prompt for AI Agents
In apps/web/app/(app)/sentry-identify.tsx around lines 14 to 20, the useEffect
currently sets the Sentry tag to undefined in the else branch to try to clear
it; Sentry doesn't support removing individual tags, so remove the else branch
and only call Sentry.setTag("emailAccountId", emailAccountId) when
emailAccountId is truthy (i.e., wrap the setTag in an if (emailAccountId) check
and delete the else block) so the tag is only set when present.
Adopt structured Sentry context and tag
emailAccountIdacross web app components, middleware, and webhooks to improve SentryUnify
captureExceptionto a single-context signature and set Sentry user andemailAccountIdtags across request handlers, middleware, server actions, and UI identification; also switch service worker disabling toprocess.env.NODE_ENVand bump version to v2.25.6.📍Where to Start
Start with the new
captureExceptionAPI in apps/web/utils/error.ts, then review its usage in middleware at apps/web/utils/middleware.ts and webhook handlers at apps/web/app/api/google/webhook/process-history.ts and apps/web/app/api/outlook/webhook/process-history.ts.Macroscope summarized 8508e1b.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.