fix: Improve error handling for ecosystem seed script #1590
fix: Improve error handling for ecosystem seed script #1590
Conversation
Signed-off-by: sujitaw <[email protected]>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 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 docstrings
🧪 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.
🧹 Nitpick comments (1)
libs/prisma-service/prisma/seed.ts (1)
683-701: Good improvement for specific error messages; consider extracting to a utility.The individual validation provides clear, actionable error messages for debugging. However, this pattern is repetitive and inconsistent with the combined check in
getKeycloakToken()(line 636).Consider extracting a reusable utility function to validate required environment variables, which would reduce duplication and ensure consistent error handling across the codebase.
♻️ Proposed utility function
function validateRequiredEnvVars(varNames: string[]): void { for (const name of varNames) { if (!process.env[name]) { throw new Error(`Missing environment variable: ${name}`); } } }Then in
createKeycloakUser():- if (!KEYCLOAK_DOMAIN) { - throw new Error('Missing environment variable: KEYCLOAK_DOMAIN'); - } - - if (!KEYCLOAK_REALM) { - throw new Error('Missing environment variable: KEYCLOAK_REALM'); - } - - if (!PLATFORM_ADMIN_KEYCLOAK_ID) { - throw new Error('Missing environment variable: PLATFORM_ADMIN_KEYCLOAK_ID'); - } - - if (!PLATFORM_ADMIN_KEYCLOAK_SECRET) { - throw new Error('Missing environment variable: PLATFORM_ADMIN_KEYCLOAK_SECRET'); - } - - if (!CRYPTO_PRIVATE_KEY) { - throw new Error('Missing environment variable: CRYPTO_PRIVATE_KEY'); - } + validateRequiredEnvVars([ + 'KEYCLOAK_DOMAIN', + 'KEYCLOAK_REALM', + 'PLATFORM_ADMIN_KEYCLOAK_ID', + 'PLATFORM_ADMIN_KEYCLOAK_SECRET', + 'CRYPTO_PRIVATE_KEY' + ]);Based on learnings: "the team prefers to create utilities for validating required environment variables rather than implementing inline validation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/prisma-service/prisma/seed.ts` around lines 683 - 701, Extract a small utility function validateRequiredEnvVars(varNames: string[]) that iterates the list and throws `new Error(\`Missing environment variable: ${name}\`)` for any missing env var, then replace the inline repetitive checks in createKeycloakUser() with a single call to validateRequiredEnvVars([...]) including KEYCLOAK_DOMAIN, KEYCLOAK_REALM, PLATFORM_ADMIN_KEYCLOAK_ID, PLATFORM_ADMIN_KEYCLOAK_SECRET, CRYPTO_PRIVATE_KEY; also update getKeycloakToken() to reuse the same utility for consistent behavior and ensure the new function is exported/imported where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/prisma-service/prisma/seed.ts`:
- Around line 683-701: Extract a small utility function
validateRequiredEnvVars(varNames: string[]) that iterates the list and throws
`new Error(\`Missing environment variable: ${name}\`)` for any missing env var,
then replace the inline repetitive checks in createKeycloakUser() with a single
call to validateRequiredEnvVars([...]) including KEYCLOAK_DOMAIN,
KEYCLOAK_REALM, PLATFORM_ADMIN_KEYCLOAK_ID, PLATFORM_ADMIN_KEYCLOAK_SECRET,
CRYPTO_PRIVATE_KEY; also update getKeycloakToken() to reuse the same utility for
consistent behavior and ensure the new function is exported/imported where
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15ee697c-9230-4fc8-91d0-810af2f5c348
📒 Files selected for processing (1)
libs/prisma-service/prisma/seed.ts



What
Summary by CodeRabbit