-
Notifications
You must be signed in to change notification settings - Fork 634
Improve portfolio fetch concurrency and error handling #8572
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.
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
Conversation
Refactored fetchAllPortfolios to use up to 50 concurrent workers instead of batching, improving throughput. Updated fetchWithRetry to return null on failure instead of throwing, and adjusted downstream logic to handle null responses gracefully. Errors for individual addresses are now logged and do not interrupt processing of others.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughModified Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8572 +/- ##
=======================================
Coverage 54.47% 54.47%
=======================================
Files 922 922
Lines 61361 61361
Branches 4149 4149
=======================================
Hits 33425 33425
Misses 27835 27835
Partials 101 101
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/dashboard/src/@/hooks/useWalletPortfolio.ts (1)
160-196: Consider reducing concurrency or documenting the rationale.With 50 concurrent workers, and each address fetching all chains in parallel (via
Promise.allinfetchWalletPortfolio), peak concurrent requests could be50 × chainIds.length. For 10+ chains, this means 500+ simultaneous requests, which may trigger API rate limiting despite the retry logic.Consider either:
- Reducing
concurrencyto a more conservative value (e.g., 10-20)- Adding per-chain concurrency limits within
fetchWalletPortfolio- Making concurrency configurable via a parameter
Also, per coding guidelines, add an explicit return type annotation to the
workerfunction:- async function worker() { + async function worker(): Promise<void> {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/dashboard/src/@/hooks/useWalletPortfolio.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
apps/{dashboard,playground-web}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{dashboard,playground-web}/src/**/*.{ts,tsx}: Import UI component primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground
Use Tailwind CSS only – no inline styles or CSS modules in dashboard and playground
Usecn()from@/lib/utilsfor conditional Tailwind class merging
Use design system tokens for styling (backgrounds:bg-card, borders:border-border, muted text:text-muted-foreground)
ExposeclassNameprop on root element for component overrides
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
apps/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/dashboard/src/**/*.{ts,tsx}: UseNavLinkfor internal navigation with automatic active states in dashboard
Start server component files withimport "server-only";in Next.js
Read cookies/headers withnext/headersin server components
Access server-only environment variables in server components
Perform heavy data fetching in server components
Implement redirect logic withredirect()fromnext/navigationin server components
Begin client component files with'use client';directive in Next.js
Handle interactive UI with React hooks (useState,useEffect, React Query, wallet hooks) in client components
Access browser APIs (localStorage,window,IntersectionObserver) in client components
Support fast transitions with prefetched data in client components
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader for API calls – never embed tokens in URLs
Return typed results (Project[],User[]) from server-side data fetches – avoidany
Wrap client-side API calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysin React Query for cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components – only use analytics client-side
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
apps/dashboard/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
apps/dashboard/**/*.{ts,tsx}: Always import from the central UI library under@/components/ui/*for reusable core UI components likeButton,Input,Select,Tabs,Card,Sidebar,Separator,Badge
UseNavLinkfrom@/components/ui/NavLinkfor internal navigation to ensure active states are handled automatically
For notices and skeletons, rely onAnnouncementBanner,GenericLoadingPage, andEmptyStateCardcomponents
Import icons fromlucide-reactor the project-specific…/iconsexports; never embed raw SVG
Keep components pure; fetch data outside using server components or hooks and pass it down via props
Use Tailwind CSS as the styling system; avoid inline styles or CSS modules
Merge class names withcnfrom@/lib/utilsto keep conditional logic readable
Stick to design tokens: usebg-card,border-border,text-muted-foregroundand other Tailwind variables instead of hard-coded colors
Use spacing utilities (px-*,py-*,gap-*) instead of custom margins
Follow mobile-first responsive design with Tailwind helpers (max-sm,md,lg,xl)
Never hard-code colors; always use Tailwind variables
Combine class names viacn, and exposeclassNameprop if useful in components
Use React Query (@tanstack/react-query) for all client-side data fetching with typed hooks
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
apps/dashboard/**/*use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
apps/dashboard/**/*use*.{ts,tsx}: KeepqueryKeystable and descriptive in React Query hooks for reliable cache hits
ConfigurestaleTimeandcacheTimein React Query according to data freshness requirements
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
apps/{dashboard,playground}/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{dashboard,playground}/**/*.{tsx,ts}: Import UI primitives from @/components/ui/_ (Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in Dashboard and Playground apps
Use NavLink for internal navigation so active states are handled automatically
Use Tailwind CSS for styling – no inline styles or CSS modules
Merge class names with cn() from @/lib/utils to keep conditional logic readable
Stick to design tokens for styling: backgrounds (bg-card), borders (border-border), muted text (text-muted-foreground), etc.
Server Components: Read cookies/headers with next/headers, access server-only environment variables or secrets, perform heavy data fetching, implement redirect logic with redirect() from next/navigation, and start files with import 'server-only'; to prevent client bundling
Client Components: Begin files with 'use client'; before imports, handle interactive UI relying on React hooks (useState, useEffect, React Query, wallet hooks), access browser APIs (localStorage, window, IntersectionObserver, etc.), and support fast transitions with client-side data prefetching
For client-side data fetching: Wrap calls in React Query (@tanstack/react-query), use descriptive and stable queryKeys for cache hits, configure staleTime / cacheTime based on freshness requirements (default ≥ 60 s), and keep tokens secret by calling internal API routes or server actions
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
apps/{dashboard,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{dashboard,playground}/**/*.{ts,tsx}: For server-side data fetching: Always call getAuthToken() to retrieve the JWT from cookies and inject the token as an Authorization: Bearer header – never embed it in the URL. Return typed results (Project[], User[], …) – avoid any
Never import posthog-js in server components; analytics reporting is client-side only
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
apps/dashboard/src/@/hooks/useWalletPortfolio.ts
⏰ 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). (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/dashboard/src/@/hooks/useWalletPortfolio.ts (3)
15-42: LGTM - Clean retry implementation with graceful degradation.The exponential backoff logic is correct (1s → 2s → 4s, capped at 10s), and returning
nullon failure instead of throwing enables the caller to handle failures gracefully without interrupting batch operations.
75-78: LGTM - Graceful handling of failed chain fetches.Returning an empty array for failed fetches is appropriate since it allows aggregation to continue without affecting totals for other successful chains.
201-231: LGTM - Hook correctly wraps the portfolio fetching logic.The
useMutationpattern is appropriate for this use case since it's a user-triggered action with progress tracking rather than a cached query.
size-limit report 📦
|
Refactored fetchAllPortfolios to use up to 50 concurrent workers instead of batching, improving throughput. Updated fetchWithRetry to return null on failure instead of throwing, and adjusted downstream logic to handle null responses gracefully. Errors for individual addresses are now logged and do not interrupt processing of others.
PR-Codex overview
This PR improves the
fetchWithRetryfunction to returnnullon failure instead of throwing an error, enhancing error handling. It also refines thefetchAllPortfoliosfunction to process up to 50 addresses concurrently, improving performance and error management during portfolio fetching.Detailed summary
fetchWithRetryto returnnullon failure instead of throwing an error.fetchTokensForChainto handlenullresponses.fetchAllPortfolios.Summary by CodeRabbit
Bug Fixes
Performance
✏️ Tip: You can customize this high-level summary in your review settings.