Conversation
🦋 Changeset detectedLatest commit: facf0ec The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
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. WalkthroughAdds a patch-level changeset and updates Intercom login flows to catch and report errors via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug related to Intercom login failures by implementing robust error handling. It integrates a dedicated error reporting utility to catch and log any issues that arise during the Intercom user authentication process, thereby enhancing the application's stability and providing better visibility into potential problems. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds error handling for Intercom login on native platforms. The change correctly catches errors and reports them. The feedback provided addresses a consistency issue with the return type of the login promise chain, which is important for code correctness and consistency across platforms.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/intercom.ts (1)
17-21: Consider adding error handling to the web path for consistency.The native login path now has comprehensive error handling (lines 54-59), but the web path doesn't handle potential synchronous throws from the
Intercom()call. While this PR focuses on native Intercom errors, adding similar error handling to the web path would ensure consistent behavior across platforms.🔎 Proposed enhancement for web path error handling
login: (userId: string, token: string) => { if (!appId) return Promise.resolve(false); - Intercom({ app_id: appId, user_id: userId, intercom_user_jwt: token }); - return Promise.resolve(true); + try { + Intercom({ app_id: appId, user_id: userId, intercom_user_jwt: token }); + return Promise.resolve(true); + } catch (error: unknown) { + reportError(error); + return Promise.resolve(false); + } },
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.changeset/cold-planes-stop.mdsrc/utils/intercom.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/app.mdc)
src/**/*.{ts,tsx}: Build complex components by composing smaller, single-purpose components. Avoid creating monolithic components with dozens of props.
Keep prop interfaces minimal. Do not pass entire objects as props if only a few fields are needed. Destructure and pass only what's required.
For boolean props, the name should clearly indicate the positive condition (e.g.,isActive,isLoading). Do not use negative names likeisNotActive.
Enforce@tanstack/eslint-plugin-queryrules to ensure best practices for tanstack query, such as using array query keys and handling mutation states correctly.
Enforcereact-hooks/recommendedESLint rules to enforce the rules of hooks, preventing common bugs related touseEffectanduseState.
Enforcereact-native/allESLint rules to apply comprehensive rules specific to React Native development.
Use TanStack Query for all server state management (fetching, caching, updating asynchronous data). It is the single source of truth for data from the API.
Query keys in TanStack Query must be an array. Start with a string identifying the domain, followed by any dynamic parameters. Be consistent. Examples:['user', userId],['cards', { status: 'active' }]. Do not use string formats like'user-details'.
Use TanStack Query mutations for any server-side effects (creating, updating, deleting data). HandleonSuccessandonErrorto invalidate relevant queries and provide user feedback.
All styling must use predefined design tokens fromtamagui.config.ts. Never use hardcoded values for colors, spacing, fonts, etc. Examples:padding="$4",color="$color.text.brand". Avoid:padding: 16,color: '#ff0000'.
Use Tamagui'sXStackandYStackfor layout instead of React Native'sView.
Prefer shorthand props for Tamagui styling for brevity and consistency. Examples:p="$4",m="$2",f={1}. Avoid:paddingLeft="$4",marginRight="$2".
Use Tamagui's responsive platform selectors ($platform-web, `$platform-ios...
Files:
src/utils/intercom.ts
**/*.{js,ts,tsx,jsx,sol}
📄 CodeRabbit inference engine (AGENTS.md)
Follow linter/formatter (eslint, prettier, solhint) strictly with high strictness level. No
anytype.
Files:
src/utils/intercom.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Omit redundant type names in variable declarations - let the type system explain itself
**/*.{ts,tsx}: Use PascalCase for TypeScript types and interfaces
Use valibot for all runtime validation of API inputs, environment variables, and other data; define schemas once and reuse them
Infer TypeScript types from valibot schemas usingtype User = v.Input<typeof UserSchema>instead of manually defining interfaces
Files:
src/utils/intercom.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Omit contextual names - don't repeat class/module names in members
Omit meaningless words like 'data', 'state', 'manager', 'engine', 'value' from variable and function names unless they add disambiguation
**/*.{ts,tsx,js,jsx}: Prefer function declarations for all multi-line functions; use function expressions or arrow functions only for single-line implementations
Preferconstfor all variable declarations by default; only useletif the variable's value will be reassigned
Declare each variable on its own line with its ownconstorletkeyword, not multiple declarations on one line
Use camelCase for TypeScript variables and functions
Always useimport type { ... }for type imports
Use relative paths for all imports within the project; avoid tsconfig path aliases
Follow eslint-plugin-import order: react, external libraries, then relative paths
Use object and array destructuring to access and use properties
Use object method shorthand syntax when a function is a property of an object
Prefer optional chaining (?.), nullish coalescing (??), object and array spreading (...), andfor...ofloops over traditional syntax
Do not use abbreviations or cryptic names; write out full words likeerror,parameters,requestinstead oferr,params,req
UseNumber.parseInt()instead of the globalparseInt()function when parsing numbers
All classes called withnewmust use PascalCase
UseBuffer.from(),Buffer.alloc(), orBuffer.allocUnsafe()instead of the deprecatednew Buffer()
Use@ts-expect-errorinstead of@ts-ignore; follow it immediately with a single-line lowercase comment explaining why the error is expected, without separators like-or:
Do not include the type in a variable's name; let the static type system do its job (e.g., useconst user: Usernotconst userObject: User)
Do not repeat the name of a class or module within its members; omit contextual names (e.g., use `class User { getProfil...
Files:
src/utils/intercom.ts
**/+(app|src)/**
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Use kebab-case for directory names and public Expo router routes
Files:
src/utils/intercom.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
For files with a single
defaultexport, name the file identically to the export; for files with multiple exports, use camelCase with a strong preference for a single word
Files:
src/utils/intercom.ts
**/.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Use a lowercase sentence in the imperative present tense for changeset summaries
Files:
.changeset/cold-planes-stop.md
🧠 Learnings (1)
📚 Learning: 2025-12-31T00:23:55.034Z
Learnt from: cruzdanilo
Repo: exactly/exa PR: 610
File: .changeset/ready-experts-fly.md:1-2
Timestamp: 2025-12-31T00:23:55.034Z
Learning: In the exactly/exa repository, allow and require empty changeset files (containing only --- separators) when changes are not user-facing and do not warrant a version bump. This is needed because CI runs changeset status --since origin/main and requires a changeset file to exist. Ensure such empty changesets are used only for non-user-facing changes and document the rationale in the commit or changelog notes.
Applied to files:
.changeset/cold-planes-stop.md
🧬 Code graph analysis (1)
src/utils/intercom.ts (1)
src/utils/reportError.ts (1)
reportError(3-6)
🔇 Additional comments (3)
.changeset/cold-planes-stop.md (1)
1-5: LGTM! Changeset format is correct.The changeset properly documents this bug fix as a patch-level change with a clear, imperative summary. The format follows the coding guidelines for changeset files.
src/utils/intercom.ts (2)
7-7: LGTM! Import statement is correct.The import uses a relative path and properly imports the
reportErrorutility for centralized error handling.
54-59: Good error handling implementation!The promise chain correctly catches and reports errors from both
setUserHashandloginUserWithUserAttributes, usingunknowntype and returningfalseon failure for consistent behavior.
|
@cursor review |
PR SummaryImproves resiliency of Intercom login on native by catching and reporting errors.
Written by Cursor Bugbot for commit 8b27c13. This will update automatically on new commits. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
dieguezguille
force-pushed
the
intercom
branch
from
8b27c13 to
facf0ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @.changeset/cold-planes-stop.md:
- Line 5: Update the changeset summary to remove the leading emoji and ensure it
is a lowercase imperative present-tense sentence; replace "🐛 handle intercom
login errors" with "handle intercom login errors" (no emoji, all lowercase) in
the .changeset entry so it complies with the guideline.
♻️ Duplicate comments (1)
src/utils/intercom.ts (1)
59-65: LGTM! Past review concern addressed.The native login error handling now correctly returns
Promise<boolean>in all paths:
.then(() => true)ensures booleantrueis returned on success.catch()reports errors and returnsfalseon failure- Type safety maintained with
error: unknown- Only returns
trueafter bothsetUserHashandloginUserWithUserAttributessucceedThis implementation fully addresses the concern raised in the previous review about the success path resolving to
undefinedinstead oftrue.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.changeset/cold-planes-stop.mdsrc/utils/intercom.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Use a lowercase sentence in the imperative present tense for changeset summaries
Files:
.changeset/cold-planes-stop.md
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/app.mdc)
src/**/*.{ts,tsx}: Build complex components by composing smaller, single-purpose components. Avoid creating monolithic components with dozens of props.
Keep prop interfaces minimal. Do not pass entire objects as props if only a few fields are needed. Destructure and pass only what's required.
For boolean props, the name should clearly indicate the positive condition (e.g.,isActive,isLoading). Do not use negative names likeisNotActive.
Enforce@tanstack/eslint-plugin-queryrules to ensure best practices for tanstack query, such as using array query keys and handling mutation states correctly.
Enforcereact-hooks/recommendedESLint rules to enforce the rules of hooks, preventing common bugs related touseEffectanduseState.
Enforcereact-native/allESLint rules to apply comprehensive rules specific to React Native development.
Use TanStack Query for all server state management (fetching, caching, updating asynchronous data). It is the single source of truth for data from the API.
Query keys in TanStack Query must be an array. Start with a string identifying the domain, followed by any dynamic parameters. Be consistent. Examples:['user', userId],['cards', { status: 'active' }]. Do not use string formats like'user-details'.
Use TanStack Query mutations for any server-side effects (creating, updating, deleting data). HandleonSuccessandonErrorto invalidate relevant queries and provide user feedback.
All styling must use predefined design tokens fromtamagui.config.ts. Never use hardcoded values for colors, spacing, fonts, etc. Examples:padding="$4",color="$color.text.brand". Avoid:padding: 16,color: '#ff0000'.
Use Tamagui'sXStackandYStackfor layout instead of React Native'sView.
Prefer shorthand props for Tamagui styling for brevity and consistency. Examples:p="$4",m="$2",f={1}. Avoid:paddingLeft="$4",marginRight="$2".
Use Tamagui's responsive platform selectors ($platform-web, `$platform-ios...
Files:
src/utils/intercom.ts
**/*.{js,ts,tsx,jsx,sol}
📄 CodeRabbit inference engine (AGENTS.md)
Follow linter/formatter (eslint, prettier, solhint) strictly with high strictness level. No
anytype.
Files:
src/utils/intercom.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Omit redundant type names in variable declarations - let the type system explain itself
**/*.{ts,tsx}: Use PascalCase for TypeScript types and interfaces
Use valibot for all runtime validation of API inputs, environment variables, and other data; define schemas once and reuse them
Infer TypeScript types from valibot schemas usingtype User = v.Input<typeof UserSchema>instead of manually defining interfaces
Files:
src/utils/intercom.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Omit contextual names - don't repeat class/module names in members
Omit meaningless words like 'data', 'state', 'manager', 'engine', 'value' from variable and function names unless they add disambiguation
**/*.{ts,tsx,js,jsx}: Prefer function declarations for all multi-line functions; use function expressions or arrow functions only for single-line implementations
Preferconstfor all variable declarations by default; only useletif the variable's value will be reassigned
Declare each variable on its own line with its ownconstorletkeyword, not multiple declarations on one line
Use camelCase for TypeScript variables and functions
Always useimport type { ... }for type imports
Use relative paths for all imports within the project; avoid tsconfig path aliases
Follow eslint-plugin-import order: react, external libraries, then relative paths
Use object and array destructuring to access and use properties
Use object method shorthand syntax when a function is a property of an object
Prefer optional chaining (?.), nullish coalescing (??), object and array spreading (...), andfor...ofloops over traditional syntax
Do not use abbreviations or cryptic names; write out full words likeerror,parameters,requestinstead oferr,params,req
UseNumber.parseInt()instead of the globalparseInt()function when parsing numbers
All classes called withnewmust use PascalCase
UseBuffer.from(),Buffer.alloc(), orBuffer.allocUnsafe()instead of the deprecatednew Buffer()
Use@ts-expect-errorinstead of@ts-ignore; follow it immediately with a single-line lowercase comment explaining why the error is expected, without separators like-or:
Do not include the type in a variable's name; let the static type system do its job (e.g., useconst user: Usernotconst userObject: User)
Do not repeat the name of a class or module within its members; omit contextual names (e.g., use `class User { getProfil...
Files:
src/utils/intercom.ts
**/+(app|src)/**
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Use kebab-case for directory names and public Expo router routes
Files:
src/utils/intercom.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
For files with a single
defaultexport, name the file identically to the export; for files with multiple exports, use camelCase with a strong preference for a single word
Files:
src/utils/intercom.ts
🧠 Learnings (1)
📚 Learning: 2025-12-31T00:23:55.034Z
Learnt from: cruzdanilo
Repo: exactly/exa PR: 610
File: .changeset/ready-experts-fly.md:1-2
Timestamp: 2025-12-31T00:23:55.034Z
Learning: In the exactly/exa repository, allow and require empty changeset files (containing only --- separators) when changes are not user-facing and do not warrant a version bump. This is needed because CI runs changeset status --since origin/main and requires a changeset file to exist. Ensure such empty changesets are used only for non-user-facing changes and document the rationale in the commit or changelog notes.
Applied to files:
.changeset/cold-planes-stop.md
🧬 Code graph analysis (1)
src/utils/intercom.ts (1)
src/utils/reportError.ts (1)
reportError(3-6)
🔇 Additional comments (2)
src/utils/intercom.ts (2)
7-7: LGTM!The import of
reportErroris correctly formatted using a relative path, and it's appropriately used throughout the error handling blocks.
19-25: LGTM!The web login error handling is well-implemented:
- Properly wraps the Intercom initialization in try/catch
- Uses
unknowntype for error (avoidingany)- Gracefully reports errors and returns
falseinstead of throwing- Maintains consistent
Promise<boolean>return type
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.