-
Notifications
You must be signed in to change notification settings - Fork 0
feat: ✨ add automatic database migrations and error handling #6
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
Conversation
- Introduced DB_AUTO_MIGRATE configuration to enable automatic migrations on server startup. - Implemented migration error handling to prevent server startup on failure. - Updated dependencies: drizzle-orm to 0.45.0, elysia to 1.4.18, and drizzle-kit to 0.31.8. - Modified docker-compose to rename postgres service and volume for clarity.
|
Warning Rate limit exceeded@truehazker has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds a DB_AUTO_MIGRATE flag and bootstrap flow to optionally run database migrations on startup; renames Docker Postgres service and volume; bumps several dependencies; and introduces namespaced child loggers in a few modules. Changes
Sequence DiagramsequenceDiagram
participant Server as Server (main.ts)
participant Config as Config (src/common/config.ts)
participant Migrator as Migrator (src/db/index.ts)
participant Logger as Logger
Server->>Config: read DB_AUTO_MIGRATE
alt DB_AUTO_MIGRATE == true
Server->>Logger: log "Running migrations..."
Server->>Migrator: call migrateDb()
Migrator->>Logger: log before/after migration
Migrator-->>Server: migration result
else DB_AUTO_MIGRATE == false
Server->>Logger: log "Skipping migrations"
end
Server->>Server: app.listen(...)
Server->>Logger: log server URL
Server->>Server: register SIGINT/SIGTERM handlers
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
…ul shutdown logging - Simplified logger transport configuration for development and test environments. - Updated the child logger name in graceful shutdown utility for better clarity.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
13-37: Fix DATABASE_URL hostname after Postgres service renameYou renamed the Postgres service to
elysia-boilerplate-postgres, butDATABASE_URLstill points to@postgres:5432. In a default Compose network, the hostname must match the service name, so the app will fail to connect to the DB in this setup.Recommend updating the hostname:
- DATABASE_URL: postgresql://postgres:postgres@postgres:5432/elysia-boilerplate + DATABASE_URL: postgresql://postgres:postgres@elysia-boilerplate-postgres:5432/elysia-boilerplateThe rest of the changes (healthcheck tweaks, volume rename, depends_on) look good and improve clarity/reliability.
🧹 Nitpick comments (2)
.env.example (1)
15-16: Mirror the safety warning from config.ts for DB_AUTO_MIGRATEThe
DB_AUTO_MIGRATEflag is exposed here (good), but the comment is quite minimal compared to the detailed caution insrc/common/config.ts. To reduce the risk of someone blindly enabling this in production, consider expanding the comment:-# Enable migrations on server startup -DB_AUTO_MIGRATE=false +# Enable automatic database migrations on server startup +# ⚠️ CAUTION: +# - Keep this disabled in production; run migrations via CI/CD instead. +# - Useful to enable in development/staging for convenience. +# - If migrations fail on startup, the server will NOT start. +DB_AUTO_MIGRATE=falseThis keeps the docs consistent and makes the behavior explicit right where people tend to copy env vars from.
src/modules/users/index.ts (1)
2-7: Module-scoped child logger foruserslooks goodUsing a
users-scoped child logger is a solid improvement for log filtering and aligns with pino's recommended patterns for stable contextual fields. Existing log handlers remain behaviorally unchanged.Consider adopting structured logging with object-first pattern:
log.info({ total: users.total }, 'Got users')rather than string interpolation. This keeps logs queryable, improves performance (especially when log level is disabled), and leverages pino's strength in structured field aggregation and filtering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.env.example(1 hunks)CHANGELOG.md(1 hunks)docker-compose.yml(3 hunks)package.json(2 hunks)src/common/config.ts(2 hunks)src/db/index.ts(2 hunks)src/main.ts(2 hunks)src/modules/users/index.ts(1 hunks)src/util/graceful-shutdown.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Strictly follow Biome linter rules and runbun run lint:fixto auto-fix issues
Use explicit imports instead of wildcard imports
Files:
src/common/config.tssrc/util/graceful-shutdown.tssrc/modules/users/index.tssrc/db/index.tssrc/main.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: TypeScript strict mode is enabled. Do not useanytypes unless absolutely necessary
Usesrc/common/config.tsfor environment variables with Envalid. Do not useprocess.envdirectly
All new features must include unit/integration tests
Files:
src/common/config.tssrc/util/graceful-shutdown.tssrc/modules/users/index.tssrc/db/index.tssrc/main.ts
🧠 Learnings (2)
📚 Learning: 2025-11-27T13:14:34.782Z
Learnt from: CR
Repo: truehazker/elysia-boilerplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T13:14:34.782Z
Learning: Applies to src/**/*.{ts,tsx} : Use `src/common/config.ts` for environment variables with Envalid. Do not use `process.env` directly
Applied to files:
src/common/config.ts
📚 Learning: 2025-11-27T13:14:34.782Z
Learnt from: CR
Repo: truehazker/elysia-boilerplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T13:14:34.782Z
Learning: Applies to src/db/schema/**/*.{ts,tsx} : Generate migrations after modifying schema in `src/db/schema` by running `bun run db:generate`
Applied to files:
src/db/index.ts
🧬 Code graph analysis (4)
src/util/graceful-shutdown.ts (1)
src/common/logger.ts (1)
log(4-19)
src/modules/users/index.ts (1)
src/common/logger.ts (1)
log(4-19)
src/db/index.ts (1)
src/common/logger.ts (1)
log(4-19)
src/main.ts (3)
src/db/index.ts (1)
migrateDb(20-24)src/common/logger.ts (1)
log(4-19)src/util/graceful-shutdown.ts (1)
gracefulShutdown(9-37)
🔇 Additional comments (6)
CHANGELOG.md (1)
8-18: Changelog updates align well with the new behaviorThe Unreleased entries for automatic migrations, migration error handling, and dependency updates accurately reflect the scope of this PR and match the existing changelog structure.
src/common/config.ts (1)
1-24: DB_AUTO_MIGRATE config is well-integrated and safely defaultedUsing
bool({ default: false })with a clear cautionary comment is exactly the right pattern here, and it respects the guideline of centralizing env access throughconfiginstead ofprocess.env. The envalidbool()validator accepts"1","0","true","false","t", and"f"as string tokens, so deployment manifests can safely use any of these standard boolean representations.src/util/graceful-shutdown.ts (1)
1-6: This review comment is obsolete. The code insrc/util/graceful-shutdown.tsalready implements the suggested refactor withconst log = logger.child({ name: 'graceful-shutdown' }), consistent with other modules in the codebase (db,users). Pino best practices support this approach for stable module-level context. No action required.Likely an incorrect or invalid review comment.
package.json (1)
29-41: Dependency bumps look consistent; confirm compatibility via install and lintThe drizzle-orm, elysia, and drizzle-kit minor bumps are reasonable and consistent with the stack. Please run
bun installandbun run lintto confirm no regressions (especially around migrations and app bootstrap). Note: this project has no test script configured.#!/bin/bash bun install bun run lintsrc/main.ts (2)
6-6: LGTM!The import is explicit and correctly references the migration function used in the bootstrap process.
83-86: LGTM!The error handling correctly prevents the server from starting if migrations or initialization fail. Using
log.fatalwith the error object and exiting with code 1 provides clear diagnostics for debugging startup issues.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.