Fix deployment issues #4
Conversation
Enhanced the migration script with better error visibility for production deployments: - Added diagnostic logging to verify migrations folder and files exist - Added console.error() fallback to ensure errors are visible in Railway logs - Added detailed error reporting including code, message, stack, and cause - Added success confirmation logs for better deployment monitoring This will help diagnose the silent migration failures happening on Railway by providing comprehensive error details in the deployment logs.
This addresses silent migration failures by adding detailed diagnostic steps: **Pre-Migration Checks:** - Verify migrations folder exists and contains SQL files - Test database connection before attempting migrations - Check for previously applied migrations to understand database state - Display all diagnostic info via console.log for Railway visibility **Enhanced Error Reporting:** - Database connection failures now show full error details - Migration errors include code, message, stack, and cause - All errors logged to both console and structured logger - Sanitized DATABASE_URL shown in logs (password redacted) **New Diagnostic Output:** - Migrations folder path verification - SQL file count - Database connection test result - Previous migration count (if any) - Detailed error context for all failure scenarios This will help identify the root cause of silent migration failures in Railway by providing comprehensive visibility into every step of the migration process.
## Root Cause Found Migration 0031_stormy_black_panther.sql was failing on Railway because it's NOT IDEMPOTENT. The migration has 111 DROP statements without IF EXISTS: - 27 DROP INDEX statements - 84 DROP COLUMN statements If this migration runs twice (e.g., if the drizzle migrations table wasn't properly tracking state), PostgreSQL throws errors: - Error 42704: "index does not exist" - Error 42703: "column does not exist" This causes the migration to fail and prevents the app from starting. ## The Fix Changed all DROP statements to be idempotent: - `DROP INDEX "name"` → `DROP INDEX IF EXISTS "name"` - `DROP COLUMN "name"` → `DROP COLUMN IF EXISTS "name"` Now the migration can safely run multiple times without errors. ## Why This Happened 1. Commit 26bd43a created migration 0031 (massive schema simplification) 2. Migration was applied to Railway database 3. Something caused migrations to try to rerun (possibly db state issue) 4. Non-idempotent DROP statements failed on already-dropped columns 5. App failed to start due to migration errors ## Impact Migration 0031 will now succeed even if: - Columns/indexes have already been dropped - Migration runs multiple times - Database is in an intermediate state This is a critical fix for production stability.
|
Caution Review failedThe pull request is closed. WalkthroughDatabase migration tooling enhancements introducing sanitized password logging, comprehensive pre-flight validation (migrations folder existence, database connectivity testing), structured error categorization, and idempotent SQL drop statements to improve safety and observability during migration execution. Changes
Sequence DiagramsequenceDiagram
actor User
participant migrate.ts as Migration Process
participant filesystem as File System
participant database as Database
User->>migrate.ts: Run migration
Note over migrate.ts: Pre-flight Checks
migrate.ts->>filesystem: Verify migrations folder exists
migrate.ts->>filesystem: Check for SQL files
alt Migrations folder missing or empty
migrate.ts-->>User: Exit with error
end
migrate.ts->>database: Test connectivity
alt Connection fails
migrate.ts-->>User: Log detailed failure & exit
end
migrate.ts->>database: Check for existing migrations
alt Migrations exist
migrate.ts->>migrate.ts: Log recent migration count
else Fresh database
migrate.ts->>migrate.ts: Log as fresh
end
Note over migrate.ts: Execute Migrations
migrate.ts->>database: Run migration files
alt Success
migrate.ts->>migrate.ts: Log completion
migrate.ts-->>User: Success
else Already exists error (42P07)
migrate.ts->>migrate.ts: Log as up-to-date
migrate.ts-->>User: Success
else Other error
migrate.ts->>migrate.ts: Log detailed error info
migrate.ts-->>User: Exit with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
Summary of ChangesHello @Dexploarer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the reliability and observability of the database migration process. It introduces improved logging and error handling within the migration script to provide clearer insights into the migration status and facilitate the diagnosis of potential issues during deployment. Additionally, it makes the latest SQL migration script more resilient by conditionally dropping database objects, preventing failures in environments where those objects might not exist. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| process.exit(1); | ||
| } | ||
| } catch (error: any) { | ||
| const errorMsg = `Failed to read migrations folder: ${error.message}`; | ||
| console.error(`[Migrations] ERROR: ${errorMsg}`); | ||
| console.error(`[Migrations] Stack: ${error.stack}`); | ||
| logger.error({ err: error }, errorMsg); | ||
| process.exit(1); |
There was a problem hiding this comment.
Resource Management on Early Exit:
When aborting the process due to missing migration files or a failed folder read, the code calls process.exit(1) without first closing the database connection (migrationClient.end()). This can lead to unclosed connections or resource leaks, especially in environments where process exit is not immediate or graceful.
Recommended Solution:
Before calling process.exit(1), ensure that await migrationClient.end() is called to properly close the database connection:
await migrationClient.end();
process.exit(1);| } catch (error: any) { | ||
| // Ignore errors here - schema might not exist yet | ||
| console.log( | ||
| "[Migrations] Could not check migration history (expected on first run)", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Error Handling for Migration History Check:
The catch block for the migration history check ignores all errors, assuming they are expected on first run. This could mask other, more serious issues (e.g., permission errors, network issues) that should be logged for visibility and troubleshooting.
Recommended Solution:
Log unexpected errors at a warning level to aid in debugging:
} catch (error: any) {
logger.warn({ err: error }, "Could not check migration history (expected on first run)");
console.log("[Migrations] Could not check migration history (expected on first run)");
}| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "dialogues" DROP COLUMN IF EXISTS "is_verified";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "locations" DROP COLUMN IF EXISTS "is_verified";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "lores" DROP COLUMN IF EXISTS "is_verified";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "music_tracks" DROP COLUMN IF EXISTS "is_verified";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "npcs" DROP COLUMN IF EXISTS "is_verified";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "quests" DROP COLUMN IF EXISTS "is_verified";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "deleted_at";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "deleted_by";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "view_count";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "favorite_count";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "last_viewed_at";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "is_public";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "is_featured";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "version";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "parent_id";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "is_template";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "quality_score";--> statement-breakpoint | ||
| ALTER TABLE "worlds" DROP COLUMN IF EXISTS "is_verified"; |
There was a problem hiding this comment.
Data Loss Risk: Dropping columns such as deleted_at, view_count, is_public, and others from multiple tables will permanently remove all data stored in these fields. If any application logic, analytics, or auditing relies on these columns, this migration will break those features and result in irreversible data loss.
Recommended Solution: Before applying this migration, ensure that all necessary data is backed up and that the application code is updated to handle the absence of these columns. Consider archiving the data if it may be needed in the future.
| DROP INDEX IF EXISTS "idx_dialogues_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_dialogues_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_dialogues_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_dialogues_created_by_date";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_locations_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_locations_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_locations_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_lores_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_lores_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_lores_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_lores_created_by_date";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_music_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_music_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_music_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_music_mood_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_npcs_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_npcs_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_npcs_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_npcs_created_by_date";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_quests_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_quests_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_quests_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_quests_created_by_date";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_worlds_deleted_at";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_worlds_is_public";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_worlds_view_count";--> statement-breakpoint | ||
| DROP INDEX IF EXISTS "idx_worlds_created_by_date";--> statement-breakpoint |
There was a problem hiding this comment.
Performance Impact from Index Removal: Dropping indexes such as idx_dialogues_deleted_at, idx_locations_is_public, and others may significantly degrade query performance, especially for large tables or frequently accessed columns. If these indexes are used by critical queries, their removal could lead to slowdowns and increased load on the database.
Recommended Solution: Review query plans and application usage to ensure that removing these indexes will not negatively impact performance. If necessary, retain or replace essential indexes to maintain efficient query execution.
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label
|
|||||||||||||||||||||||
codiumai-pr-agent-free
bot
added
Review effort 3/5
and removed
Review effort 2/5
labels
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label
|
||||||||||||||||||||||||
Greptile OverviewGreptile SummaryThis PR addresses critical deployment reliability issues by enhancing database migration robustness. The changes focus on two main areas: making migration 0031 idempotent and adding comprehensive diagnostics to the migration process. The migration file is updated with IF EXISTS clauses for all DROP INDEX (27 statements) and DROP COLUMN (84 statements) operations, preventing PostgreSQL errors when migrations run multiple times in deployment scenarios. The migration script receives significant enhancements including pre-flight checks for migrations folder existence, database connectivity tests, applied migration history logging, and dual console/structured logging for better visibility in Railway deployments. Important Files Changed
Confidence score: 4/5
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the robustness and debuggability of the database migration process, which is crucial for reliable deployments. The addition of pre-flight checks for migration files and database connectivity, along with more detailed error logging, will make troubleshooting deployment issues much easier. Furthermore, making the SQL migration script idempotent by using IF EXISTS is an excellent change that prevents failures on re-runs. My review includes a few suggestions to further improve maintainability and type safety in the migration script.
| console.log("[Migrations] Starting migration process"); | ||
| console.log("[Migrations] Migrations folder:", migrationsFolder); | ||
| console.log("[Migrations] Database URL:", sanitizedDbUrl); |
There was a problem hiding this comment.
The script uses both console.log and the structured logger.info for startup messages. While using console.log can be useful for immediate visibility in some deployment environments, this mix creates inconsistency. For better log management, it would be ideal to rely solely on the structured logger (pino). Pino can be configured with a synchronous transport for development or debugging scenarios if immediate output is a concern. If console.log is a hard requirement for your deployment platform, consider consolidating this logic so you don't have to call both console.log and logger.info for the same event.
| } catch (error: any) { | ||
| const errorMsg = `Failed to read migrations folder: ${error.message}`; | ||
| console.error(`[Migrations] ERROR: ${errorMsg}`); | ||
| console.error(`[Migrations] Stack: ${error.stack}`); | ||
| logger.error({ err: error }, errorMsg); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
It's a TypeScript best practice to type catch clause variables as unknown instead of any. This enforces that you safely check the error's type before trying to access its properties, preventing potential runtime errors.
| } catch (error: any) { | |
| const errorMsg = `Failed to read migrations folder: ${error.message}`; | |
| console.error(`[Migrations] ERROR: ${errorMsg}`); | |
| console.error(`[Migrations] Stack: ${error.stack}`); | |
| logger.error({ err: error }, errorMsg); | |
| process.exit(1); | |
| } | |
| } catch (error: unknown) { | |
| const errorMessage = error instanceof Error ? error.message : String(error); | |
| const errorMsg = `Failed to read migrations folder: ${errorMessage}`; | |
| console.error(`[Migrations] ERROR: ${errorMsg}`); | |
| if (error instanceof Error && error.stack) { | |
| console.error(`[Migrations] Stack: ${error.stack}`); | |
| } | |
| logger.error({ err: error }, errorMsg); | |
| process.exit(1); | |
| } |
| } catch (error: any) { | ||
| const errorMsg = `Database connection failed: ${error.message}`; | ||
| console.error("[Migrations] ✗ DATABASE CONNECTION FAILED"); | ||
| console.error("Error Code:", error?.code); | ||
| console.error("Error Message:", error?.message); | ||
| console.error("Database URL:", sanitizedDbUrl); | ||
| console.error("Full Error:", JSON.stringify(error, null, 2)); | ||
|
|
||
| logger.error( | ||
| { | ||
| err: error, | ||
| code: error?.code, | ||
| databaseUrl: sanitizedDbUrl, | ||
| }, | ||
| errorMsg, | ||
| ); | ||
| await migrationClient.end(); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
There's a recurring pattern of logging detailed error information to both console.error and logger.error before exiting the process. This is repeated in multiple error handling blocks (e.g., lines 85-90, 99-116, and 175-195). To improve maintainability and reduce code duplication, consider creating a centralized error handling function.
This function could take the error, a message, and any additional context, handle the logging to both console and the structured logger, and then exit the process. For example:
function exitWithError(message: string, error: unknown, context: Record<string, any> = {}) {
const errorDetails = {
message: error instanceof Error ? error.message : String(error),
stack: error instanceof Error ? error.stack : undefined,
...context,
};
console.error(`[Migrations] ✗ ERROR: ${message}`);
console.error("Details:", JSON.stringify(errorDetails, null, 2));
logger.error({ err: error, ...context }, `[Migrations] ✗ ${message}`);
// Consider ending the client connection here if it's always needed before exit
// await migrationClient.end();
process.exit(1);
}Using this would simplify the catch blocks significantly.
PR Code Suggestions ✨
Explore these optional code suggestions:
|
||||||||||||||
PR Type
Bug fix
Description
Make migration 0031 idempotent by adding IF EXISTS clauses
Add comprehensive database migration diagnostics
Enhance error logging and visibility for Railway deployments
Diagram Walkthrough
File Walkthrough
migrate.ts
Add comprehensive migration diagnostics and error loggingapps/core/server/db/migrate.ts
stack, cause)
visibility
0031_stormy_black_panther.sql
Make migration 0031 idempotent with IF EXISTS clausesapps/core/server/db/migrations/0031_stormy_black_panther.sql
times
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.