(SP: 1) [FIX] Hot fix index APP ENV#421
Conversation
liudmylasovetovs
requested review from
AM1007,
LesiaUKR and
ViktorSvertoka
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughNormalized and trimmed environment variables in database config: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/db/index.ts (2)
14-15: Consider case-insensitive comparison forAPP_ENV.The comparison
APP_ENV === 'local'is case-sensitive. IfAPP_ENVis set to'LOCAL'or'Local',IS_LOCAL_ENVwill befalse, causing the production branch to be used unexpectedly.Other files in the codebase use
.toLowerCase()for this check:
frontend/lib/tests/helpers/db-safety.tsuses(process.env.APP_ENV ?? 'local').toLowerCase()frontend/tests/e2e/shop-checkout-local.spec.tsuses.trim().toLowerCase()♻️ Suggested fix for case-insensitive comparison
-const APP_ENV = process.env.APP_ENV?.trim(); -const IS_LOCAL_ENV = APP_ENV === 'local'; +const APP_ENV = process.env.APP_ENV?.trim().toLowerCase(); +const IS_LOCAL_ENV = APP_ENV === 'local';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/index.ts` around lines 14 - 15, The equality check for APP_ENV in frontend/db/index.ts is case-sensitive (const IS_LOCAL_ENV = APP_ENV === 'local'), so values like 'LOCAL' or 'Local' fail; update the logic to normalize APP_ENV (e.g., call .trim().toLowerCase() or (APP_ENV ?? '').trim().toLowerCase()) before comparing and set IS_LOCAL_ENV based on the lowercased value to ensure a case-insensitive check while preserving the APP_ENV variable name.
14-15: db-safety.ts has different APP_ENV fallback behavior than the new code.The test helper
frontend/lib/tests/helpers/db-safety.ts(line 6) defaults unsetAPP_ENVto'local', whereas the code here treats it as undefined. This differs from most other test helpers (shop-checkout-local.spec.ts,setup.ts) which default to empty string instead.If intentional (test helpers more permissive), consider adding a comment to clarify. Otherwise, align
db-safety.tsto match the stricter behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/db/index.ts` around lines 14 - 15, The test helper db-safety.ts currently defaults APP_ENV to 'local' which diverges from the new behaviour used by APP_ENV and IS_LOCAL_ENV in frontend/db/index.ts; update db-safety.ts to match by reading APP_ENV with the same semantics (e.g. use process.env.APP_ENV?.trim() and compute IS_LOCAL_ENV as APP_ENV === 'local') so unset APP_ENV is treated as undefined/empty string, or alternatively add a clear comment in db-safety.ts explaining the intentional permissive default if you want to keep it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/db/index.ts`:
- Around line 14-15: The equality check for APP_ENV in frontend/db/index.ts is
case-sensitive (const IS_LOCAL_ENV = APP_ENV === 'local'), so values like
'LOCAL' or 'Local' fail; update the logic to normalize APP_ENV (e.g., call
.trim().toLowerCase() or (APP_ENV ?? '').trim().toLowerCase()) before comparing
and set IS_LOCAL_ENV based on the lowercased value to ensure a case-insensitive
check while preserving the APP_ENV variable name.
- Around line 14-15: The test helper db-safety.ts currently defaults APP_ENV to
'local' which diverges from the new behaviour used by APP_ENV and IS_LOCAL_ENV
in frontend/db/index.ts; update db-safety.ts to match by reading APP_ENV with
the same semantics (e.g. use process.env.APP_ENV?.trim() and compute
IS_LOCAL_ENV as APP_ENV === 'local') so unset APP_ENV is treated as
undefined/empty string, or alternatively add a clear comment in db-safety.ts
explaining the intentional permissive default if you want to keep it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ef58e934-7db5-4927-a6a1-e2a3b73d6cce
📒 Files selected for processing (1)
frontend/db/index.ts
2 participants
Description
Fixes a runtime crash in Netlify caused by
frontend/db/index.tsdefaultingAPP_ENVto'local'when the environment variable is missing or unreadable at function runtime.This change makes local database mode opt-in only, trims env values before use, and prevents deploy/runtime environments from silently falling back to local PostgreSQL logic.
Related Issue
Issue: #<issue_number>
Changes
APP_ENV ?? 'local'fallback infrontend/db/index.tsIS_LOCAL_ENVdetection so local DB mode is enabled only whenAPP_ENV === 'local'DATABASE_URL/DATABASE_URL_LOCALbefore initializing DrizzleAPP_ENVvalues in runtimeDatabase Changes (if applicable)
How Has This Been Tested?
Screenshots (if applicable)
Not applicable — backend/runtime env fix only.
Checklist
Before submitting
Reviewers
Summary by CodeRabbit