-
Notifications
You must be signed in to change notification settings - Fork 13
Feat/status #1022
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
Feat/status #1022
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughA suite of new API routes has been added under the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 5
🧹 Nitpick comments (4)
src/app/api/health/rpc/route.ts (1)
48-54
: Consider extracting provider identification into a helper function.The current string matching approach works but could be more maintainable with a dedicated helper function or mapping object.
+const getProviderName = (url: string, fallbackIndex: number): string => { + const providers = [ + { pattern: 'infura', name: 'infura' }, + { pattern: 'alchemy', name: 'alchemy' }, + { pattern: 'bnbchain', name: 'binance' }, + ] + + for (const provider of providers) { + if (url.includes(provider.pattern)) { + return provider.name + } + } + + return `provider_${fallbackIndex}` +} + - const providerName = rpcUrl.includes('infura') - ? 'infura' - : rpcUrl.includes('alchemy') - ? 'alchemy' - : rpcUrl.includes('bnbchain') - ? 'binance' - : `provider_${i}` + const providerName = getProviderName(rpcUrl, i)src/app/api/health/zerodev/route.ts (1)
33-43
: Remove redundant PROJECT_ID check in configuration objectSince the function returns early if
PROJECT_ID
is missing (line 20), the ternary check at line 37 is redundant and will always evaluate to 'configured'.configuration: { - projectId: PROJECT_ID ? 'configured' : 'missing', + projectId: 'configured', bundlerUrl: BUNDLER_URL ? 'configured' : 'missing', paymasterUrl: PAYMASTER_URL ? 'configured' : 'missing',src/app/api/health/route.ts (2)
73-73
: Consider adding TODO for missing servicesPer the PR objectives, bridge and manteca health endpoints are still pending. Consider adding a TODO comment to track this.
+ // TODO: Add 'bridge' and 'manteca' once their health endpoints are implemented const services = ['mobula', 'squid', 'zerodev', 'rpc', 'justaname', 'backend']
189-190
: Consider rate limiting Discord notificationsDuring an outage, frequent health checks could trigger many Discord notifications, potentially hitting API rate limits or causing notification spam.
Consider implementing a simple rate limiter or cooldown period between Discord notifications to prevent spam during extended outages. For example, only send notifications if the last one was sent more than 5 minutes ago.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/api/health/backend/route.ts
(1 hunks)src/app/api/health/frontend/route.ts
(1 hunks)src/app/api/health/justaname/route.ts
(1 hunks)src/app/api/health/mobula/route.ts
(1 hunks)src/app/api/health/route.ts
(1 hunks)src/app/api/health/rpc/route.ts
(1 hunks)src/app/api/health/squid/route.ts
(1 hunks)src/app/api/health/zerodev/route.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from `${SQUID_API_URL}/route` to `${SQUID_API_URL}/v2/route` in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#942
File: src/components/AddMoney/consts/index.ts:2151-2162
Timestamp: 2025-06-30T10:44:08.048Z
Learning: Hugo0 often agrees with refactoring suggestions but defers implementation due to time constraints, preferring to track improvements as follow-up issues when they're part of larger architectural changes.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#823
File: src/context/kernelClient.context.tsx:79-86
Timestamp: 2025-04-29T19:36:38.121Z
Learning: When Hugo0 asks to "resolve coderabbit comments", they want to acknowledge the comment without necessarily implementing the suggested changes, as the current implementation might be intentional for their specific use case.
src/app/api/health/mobula/route.ts (2)
Learnt from: Hugo0
PR: #1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from ${SQUID_API_URL}/route
to ${SQUID_API_URL}/v2/route
in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
Learnt from: jjramirezn
PR: #956
File: src/app/actions/tokens.ts:220-227
Timestamp: 2025-07-07T19:55:14.380Z
Learning: In the Mobula API integration within src/app/actions/tokens.ts
, the asset.asset.blockchains
array and asset.contracts_balances
array are synchronized, meaning for every blockchain in the blockchains array, there will be a corresponding entry in the contracts_balances array with matching address. This makes the non-null assertion operator safe to use when accessing contractInfo!.decimals
in the fetchWalletBalances
function.
src/app/api/health/zerodev/route.ts (1)
Learnt from: Hugo0
PR: #1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from ${SQUID_API_URL}/route
to ${SQUID_API_URL}/v2/route
in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
src/app/api/health/backend/route.ts (1)
Learnt from: Hugo0
PR: #1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from ${SQUID_API_URL}/route
to ${SQUID_API_URL}/v2/route
in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
src/app/api/health/frontend/route.ts (1)
Learnt from: Hugo0
PR: #1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from ${SQUID_API_URL}/route
to ${SQUID_API_URL}/v2/route
in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
src/app/api/health/route.ts (1)
Learnt from: Hugo0
PR: #1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from ${SQUID_API_URL}/route
to ${SQUID_API_URL}/v2/route
in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
src/app/api/health/squid/route.ts (1)
Learnt from: Hugo0
PR: #1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from ${SQUID_API_URL}/route
to ${SQUID_API_URL}/v2/route
in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
🪛 Gitleaks (8.27.2)
src/app/api/health/squid/route.ts
33-33: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (12)
src/app/api/health/rpc/route.ts (3)
16-27
: LGTM: Proper API key validation.The validation logic correctly ensures at least one RPC provider API key is configured before proceeding with health checks.
82-82
: LGTM: Proper API key sanitization.The regex pattern correctly masks API keys in URLs, preventing sensitive information from being exposed in health check responses.
123-138
: LGTM: Well-designed status aggregation logic.The hierarchical status determination (provider → chain → overall) correctly prioritizes unhealthy states and appropriately returns HTTP 500 for critical failures, enabling proper monitoring system integration.
src/app/api/health/backend/route.ts (2)
26-40
: LGTM: Sound connectivity testing approach.The logic correctly treats non-5xx responses (including 404) as healthy since they indicate successful connectivity. The hardcoded test username 'hugo' appears intentional for a consistent test case.
44-77
: LGTM: Comprehensive response structure and error handling.The detailed response provides excellent diagnostic information with proper timing metrics and status-specific messaging. Error handling is consistent and appropriate.
src/app/api/health/justaname/route.ts (2)
14-39
: LGTM: Well-chosen test case for ENS resolution.Using Vitalik's address as a test case is excellent - it's stable, well-known, and likely to have ENS records. The response structure validation ensures the API is functioning correctly.
41-68
: LGTM: Excellent graceful failure handling.The secondary lookup test is properly handled - failures are treated as degraded rather than unhealthy, providing diagnostic information without affecting the overall service health status when the primary ENS resolution is working.
src/app/api/health/squid/route.ts (2)
33-33
: Static analysis false positive - this is a contract address.The static analysis tool flagged this as an API key, but this is actually the USDC contract address on Arbitrum, which is public information and not sensitive.
26-109
: LGTM: Comprehensive Squid API health check implementation.The test parameters are realistic and the dual testing approach (regular + RFQ routes) provides excellent coverage of Squid API functionality. The response structure offers detailed diagnostic information.
src/app/api/health/zerodev/route.ts (1)
45-93
: Well-structured health check logicThe implementation correctly handles optional Polygon configuration and determines overall health status based on critical service availability.
src/app/api/health/mobula/route.ts (1)
28-90
: Well-implemented health check testsThe health check appropriately tests both critical Mobula endpoints with proper error handling and response validation.
src/app/api/health/route.ts (1)
147-170
: Well-designed health status determination logicThe implementation correctly prioritizes critical services and uses a reasonable threshold for overall system health. The health score calculation with weighted degraded services is a good approach.
export async function GET() { | ||
try { | ||
const healthData = { | ||
status: 'healthy', | ||
service: 'peanut-ui-frontend', | ||
timestamp: new Date().toISOString(), | ||
version: process.env.npm_package_version || 'unknown', | ||
environment: process.env.NODE_ENV, | ||
uptime: process.uptime(), | ||
} | ||
|
||
return NextResponse.json(healthData, { status: 200 }) | ||
} catch (error) { | ||
return NextResponse.json( | ||
{ | ||
status: 'unhealthy', | ||
service: 'peanut-ui-frontend', | ||
timestamp: new Date().toISOString(), | ||
error: error instanceof Error ? error.message : 'Unknown error', | ||
}, | ||
{ status: 500 } | ||
) | ||
} | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Verify version retrieval approach in production environments.
The health check implementation looks solid, but process.env.npm_package_version
may not be available in production deployments since this environment variable is typically only set during npm script execution.
Consider using a build-time approach or package.json reading for version info:
🏁 Script executed:
#!/bin/bash
# Check if npm_package_version is available in the build environment
echo "Checking if npm_package_version is available in different contexts..."
echo "Current npm_package_version: $npm_package_version"
# Search for other version handling approaches in the codebase
rg -A 3 -B 3 "version.*package\.json|npm_package_version" --type ts --type js
Length of output: 1822
Use a reliable version source in your health endpoints
The health‐check logic is solid, but process.env.npm_package_version
is empty at runtime (it’s only set during npm script runs). You’ll need to pull the version from a build‐time source instead. For example, you can import it directly from package.json
or inject it via a Next.js‐friendly env var:
Suggested approach 1 – JSON import (requires resolveJsonModule: true
in tsconfig):
// src/app/api/health/frontend/route.ts (and similarly in src/app/api/health/route.ts)
import pkg from '../../../../package.json';
export async function GET() {
const healthData = {
status: 'healthy',
service: 'peanut-ui-frontend',
timestamp: new Date().toISOString(),
version: pkg.version,
environment: process.env.NODE_ENV,
uptime: process.uptime(),
};
return NextResponse.json(healthData);
}
Suggested approach 2 – Build‐time env var (Next.js only exposes NEXT_PUBLIC_…
at runtime):
- In your build script, set:
NEXT_PUBLIC_APP_VERSION=$npm_package_version
- In code:
version: process.env.NEXT_PUBLIC_APP_VERSION ?? 'unknown'
Please update the version
field in:
- src/app/api/health/frontend/route.ts
- src/app/api/health/route.ts
🤖 Prompt for AI Agents
In src/app/api/health/frontend/route.ts lines 7 to 30, the version field uses
process.env.npm_package_version which is empty at runtime. Fix this by importing
the version directly from package.json using a JSON import with
resolveJsonModule enabled, or by using a build-time environment variable like
NEXT_PUBLIC_APP_VERSION set during build and accessed via
process.env.NEXT_PUBLIC_APP_VERSION. Update the version field accordingly to
ensure it reflects the actual app version at runtime.
const MOBULA_API_URL = process.env.MOBULA_API_URL! | ||
const MOBULA_API_KEY = process.env.MOBULA_API_KEY! | ||
|
||
/** | ||
* Health check for Mobula API | ||
* Tests both asset price endpoint and portfolio endpoint | ||
*/ | ||
export async function GET() { | ||
const startTime = Date.now() | ||
|
||
try { | ||
if (!MOBULA_API_KEY) { | ||
return NextResponse.json( | ||
{ | ||
status: 'unhealthy', | ||
service: 'mobula', | ||
timestamp: new Date().toISOString(), | ||
error: 'MOBULA_API_KEY not configured', | ||
responseTime: Date.now() - startTime, | ||
}, | ||
{ status: 500 } | ||
) | ||
} |
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.
Add runtime check for MOBULA_API_URL to prevent crashes
The code uses a non-null assertion for MOBULA_API_URL
but never validates it at runtime. This could cause a crash if the environment variable is missing.
if (!MOBULA_API_KEY) {
return NextResponse.json(
{
status: 'unhealthy',
service: 'mobula',
timestamp: new Date().toISOString(),
error: 'MOBULA_API_KEY not configured',
responseTime: Date.now() - startTime,
},
{ status: 500 }
)
}
+
+ if (!MOBULA_API_URL) {
+ return NextResponse.json(
+ {
+ status: 'unhealthy',
+ service: 'mobula',
+ timestamp: new Date().toISOString(),
+ error: 'MOBULA_API_URL not configured',
+ responseTime: Date.now() - startTime,
+ },
+ { status: 500 }
+ )
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const MOBULA_API_URL = process.env.MOBULA_API_URL! | |
const MOBULA_API_KEY = process.env.MOBULA_API_KEY! | |
/** | |
* Health check for Mobula API | |
* Tests both asset price endpoint and portfolio endpoint | |
*/ | |
export async function GET() { | |
const startTime = Date.now() | |
try { | |
if (!MOBULA_API_KEY) { | |
return NextResponse.json( | |
{ | |
status: 'unhealthy', | |
service: 'mobula', | |
timestamp: new Date().toISOString(), | |
error: 'MOBULA_API_KEY not configured', | |
responseTime: Date.now() - startTime, | |
}, | |
{ status: 500 } | |
) | |
} | |
const MOBULA_API_URL = process.env.MOBULA_API_URL! | |
const MOBULA_API_KEY = process.env.MOBULA_API_KEY! | |
/** | |
* Health check for Mobula API | |
* Tests both asset price endpoint and portfolio endpoint | |
*/ | |
export async function GET() { | |
const startTime = Date.now() | |
try { | |
if (!MOBULA_API_KEY) { | |
return NextResponse.json( | |
{ | |
status: 'unhealthy', | |
service: 'mobula', | |
timestamp: new Date().toISOString(), | |
error: 'MOBULA_API_KEY not configured', | |
responseTime: Date.now() - startTime, | |
}, | |
{ status: 500 } | |
) | |
} | |
if (!MOBULA_API_URL) { | |
return NextResponse.json( | |
{ | |
status: 'unhealthy', | |
service: 'mobula', | |
timestamp: new Date().toISOString(), | |
error: 'MOBULA_API_URL not configured', | |
responseTime: Date.now() - startTime, | |
}, | |
{ status: 500 } | |
) | |
} | |
// ...rest of the health check logic... | |
} catch (err) { | |
// existing catch logic | |
} | |
} |
🤖 Prompt for AI Agents
In src/app/api/health/mobula/route.ts around lines 4 to 26, the code uses a
non-null assertion for MOBULA_API_URL but does not check if it is actually set
at runtime, which can cause crashes if the environment variable is missing. Add
a runtime check for MOBULA_API_URL similar to the existing check for
MOBULA_API_KEY, returning a JSON response with an error and appropriate status
if MOBULA_API_URL is not configured.
(process.env.VERCEL_URL?.includes('peanut.me') && !process.env.VERCEL_URL?.includes('staging.peanut.me')) | ||
const shouldMentionRole = isProduction || isPeanutDomain | ||
|
||
const roleMention = shouldMentionRole ? '<@&1187109195389083739> ' : '' |
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.
🛠️ Refactor suggestion
Move hardcoded Discord role ID to environment variable
The Discord role ID is hardcoded, which makes it difficult to maintain and update across environments.
- const roleMention = shouldMentionRole ? '<@&1187109195389083739> ' : ''
+ const roleId = process.env.DISCORD_ALERT_ROLE_ID
+ const roleMention = shouldMentionRole && roleId ? `<@&${roleId}> ` : ''
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const roleMention = shouldMentionRole ? '<@&1187109195389083739> ' : '' | |
const roleId = process.env.DISCORD_ALERT_ROLE_ID | |
const roleMention = shouldMentionRole && roleId ? `<@&${roleId}> ` : '' |
🤖 Prompt for AI Agents
In src/app/api/health/route.ts at line 34, the Discord role ID is hardcoded as a
string literal. To improve maintainability and flexibility, move this hardcoded
role ID into an environment variable. Replace the hardcoded ID with a reference
to process.env containing the role ID, and ensure the environment variable is
properly set in your deployment environments.
console.log('hasRfqRoute', hasRfqRoute) | ||
console.log('rfqRouteData', rfqRouteData) | ||
} catch (e) { | ||
console.error('Error parsing RFQ route response:', e) | ||
console.error('RFQ response:', rfqRouteResponse) |
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.
🛠️ Refactor suggestion
Remove debug console.log statements.
The console.log statements appear to be debugging code that should be removed for production deployment.
- console.log('hasRfqRoute', hasRfqRoute)
- console.log('rfqRouteData', rfqRouteData)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
console.log('hasRfqRoute', hasRfqRoute) | |
console.log('rfqRouteData', rfqRouteData) | |
} catch (e) { | |
console.error('Error parsing RFQ route response:', e) | |
console.error('RFQ response:', rfqRouteResponse) | |
} catch (e) { | |
console.error('Error parsing RFQ route response:', e) | |
console.error('RFQ response:', rfqRouteResponse) |
🤖 Prompt for AI Agents
In src/app/api/health/squid/route.ts around lines 79 to 83, remove the
console.log statements used for debugging the variables hasRfqRoute and
rfqRouteData. These debug prints should be deleted to clean up the code for
production deployment, leaving only the error handling console.error statements
intact.
results[chainName].paymaster = { | ||
status: paymasterResponse.status >= 200 && paymasterResponse.status < 503 ? 'healthy' : 'unhealthy', // 500 is expected for basic calls | ||
responseTime: Date.now() - paymasterTestStart, | ||
httpStatus: paymasterResponse.status, | ||
} |
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.
Reconsider treating HTTP 500-502 as healthy status
The current logic treats HTTP status codes 500-502 as healthy for the paymaster endpoint. While the comment indicates "500 is expected for basic calls", this approach has risks:
- HTTP 501 (Not Implemented) and 502 (Bad Gateway) typically indicate service issues
- Treating server errors as healthy could mask real problems
Consider being more explicit about which error codes are expected and why.
results[chainName].paymaster = {
- status: paymasterResponse.status >= 200 && paymasterResponse.status < 503 ? 'healthy' : 'unhealthy', // 500 is expected for basic calls
+ status: (paymasterResponse.ok || paymasterResponse.status === 500) ? 'healthy' : 'unhealthy', // 500 is expected for eth_chainId on paymaster
responseTime: Date.now() - paymasterTestStart,
httpStatus: paymasterResponse.status,
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
results[chainName].paymaster = { | |
status: paymasterResponse.status >= 200 && paymasterResponse.status < 503 ? 'healthy' : 'unhealthy', // 500 is expected for basic calls | |
responseTime: Date.now() - paymasterTestStart, | |
httpStatus: paymasterResponse.status, | |
} | |
results[chainName].paymaster = { | |
status: (paymasterResponse.ok || paymasterResponse.status === 500) ? 'healthy' : 'unhealthy', // 500 is expected for eth_chainId on paymaster | |
responseTime: Date.now() - paymasterTestStart, | |
httpStatus: paymasterResponse.status, | |
} |
🤖 Prompt for AI Agents
In src/app/api/health/zerodev/route.ts around lines 166 to 170, the current code
treats HTTP status codes from 200 up to 502 as 'healthy', which incorrectly
includes 501 and 502 errors. Update the logic to explicitly mark only 200-499 as
healthy, or specifically allow 500 if justified, and treat 501 and 502 as
'unhealthy'. Adjust the condition to clearly reflect which status codes are
considered healthy and update the comment to explain the reasoning.
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.
LGTM
we need to know status of all our services - this creates endpoints for all our services + upstream providers.
missing: bridge & manteca