-
Notifications
You must be signed in to change notification settings - Fork 86
Nango secret key error message #1483
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
base: main
Are you sure you want to change the base?
Nango secret key error message #1483
Conversation
- Add Nango health check endpoint in manage-api - Create API route for Nango config check in manage-ui - Add useNangoConfig hook to check Nango configuration - Display warning banner on MCP servers page when Nango is not configured - Improve OAuth login error messages to indicate Nango configuration issues - Link to Nango setup documentation in error messages Co-authored-by: andrew <andrew@inkeep.com>
Co-authored-by: andrew <andrew@inkeep.com>
Co-authored-by: andrew <andrew@inkeep.com>
|
Cursor Agent can help with this pull request. Just |
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @amikofalvy's task —— View job Code Review: Nango Secret Key Error HandlingI've conducted a comprehensive code review of this PR that implements better error handling for missing Nango Secret Key. Here's my detailed analysis: Review Tasks:
Overall AssessmentThis PR effectively addresses a user experience problem by transforming a cryptic error message into clear, actionable guidance. The implementation follows existing patterns well (mirroring the SIGNOZ health check approach) and maintains architectural consistency. Major Strengths
Critical Issues & Areas for Improvement🔴 Critical: Logging Security VulnerabilityFile: logger.info({
hasSecretKey: !!nangoSecretKey,
}, 'Checking Nango configuration');Issue: This logs Recommendation: logger.debug({ configCheck: 'nango' }, 'Checking Nango configuration');🟡 Architecture: Inconsistent Error Response FormatThe Nango health check returns different response structures compared to SIGNOZ: Nango ( return c.json({
status: 'not_configured',
configured: false,
error: 'NANGO_SECRET_KEY not set',
});SIGNOZ ( return c.json({
status: 'not_configured',
configured: false,
error: 'SIGNOZ_URL or SIGNOZ_API_KEY not set',
});Recommendation: Create a shared type/schema for health check responses to ensure consistency: interface HealthCheckResponse {
status: 'ok' | 'not_configured' | 'invalid_format' | 'connection_failed';
configured: boolean;
error?: string;
}🟡 Code Quality: Hard-coded Key Format ValidationFile: if (!nangoSecretKey.startsWith('sk-')) {Issue: This validation logic is duplicated from Recommendation: Extract this validation into a shared utility function in 🟡 Error Handling: Inconsistent Error Message PatternsFile: The nested conditionals for error message detection are fragile: if (error.message.includes('NANGO_SECRET_KEY')) {
errorMessage = 'NANGO_SECRET_KEY environment variable is not configured...';
} else if (error.message.includes('query') || error.message.includes('undefined')) {
errorMessage = 'OAuth configuration error. This may be caused by a missing NANGO_SECRET_KEY...';
}Issue: This relies on string matching which is brittle and could break if underlying error messages change. Recommendation: Consider structured error handling where possible, or create specific error types in the OAuth service layer. Minor Issues🟢 Performance: Redundant API CallsFile:
🟢 User Experience: Loading StateFile: 🟢 Code Style: Prefer Early ReturnsFile: Missing Considerations
Customer Impact Analysis✅ Positive: This significantly improves the developer experience by providing clear, actionable error messages instead of cryptic JavaScript errors. ✅ No Breaking Changes: All changes are additive and maintain backward compatibility. ✅ Proper Error Guidance: The UI banner includes a link to documentation, which is excellent for self-service resolution. Recommendations Summary
This is a well-executed improvement that significantly enhances the user experience while maintaining architectural consistency. The identified issues are primarily around consistency and security hygiene rather than fundamental design flaws. |
Implement a warning banner and clearer error messages for missing or invalid Nango Secret Key to improve user guidance.
The original "OAuth Error: Cannot read properties of undefined (reading 'query')" was cryptic when
NANGO_SECRET_KEYwas not set. This PR introduces a dedicated health check, a frontend banner, and specific backend error message handling to guide users to correctly configure Nango, mirroring the existing SIGNOZ environment variable setup guidance.Linear Issue: PRD-5732