refactor: remove dup fetching position keys#312
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes add graceful null handling for GraphQL NOT_FOUND errors across the Morpho API data layer. The core fetcher now returns null instead of throwing on NOT_FOUND, and callers guard against falsy responses with early returns or empty defaults. GraphQL response types are relaxed to optional fields. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
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)
src/hooks/useUserPositions.ts (1)
64-89: Logic bug: Subgraph fallback broken for non-Morpho-API networks.When
supportsMorphoApi(network)is false, the code skips the Morpho block entirely.apiErrorstaysfalseandmarketsstays[]. The condition on line 79 (markets.length === 0 && apiError) is never true, so Subgraph is never called.The previous behavior likely fell back to Subgraph when
markets.length === 0regardless of error state.Proposed fix
// If Morpho API failed or not supported, try Subgraph - if (markets.length === 0 && apiError) { + if (markets.length === 0 && (apiError || !supportsMorphoApi(network))) { try { console.log(`Attempting to fetch positions via Subgraph for network ${network}`); markets = await fetchSubgraphUserPositionMarkets(user, network);
🤖 Fix all issues with AI agents
In `@src/data-sources/morpho-api/fetchers.ts`:
- Around line 23-29: The NOT_FOUND check currently inspects err.status which
doesn't exist, so update the guard to examine the message field instead: in the
notFoundError computation (the variable named notFoundError that iterates
(result as any).errors), change the predicate to check
err.message?.includes('NOT_FOUND') (and adjust the inline type from { status?:
string } to { message?: string } if present) so the branch that logs "Morpho API
return Not Found error" and returns null will correctly trigger for NOT_FOUND
errors.
🧹 Nitpick comments (1)
src/data-sources/morpho-api/vaults.ts (1)
34-41: Type updated, buterrorsfield no longer checked.The
errorsfield is still in the type (line 40) but no longer checked in the function. This is fine since the fetcher now handles GraphQL errors centrally, though you could removeerrorsfrom the type for clarity.
Summary by CodeRabbit
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.