Skip to content

improvement(migrations): log better errors#4260

Merged
icecrasher321 merged 1 commit intostagingfrom
improvement/logging-migrations
Apr 22, 2026
Merged

improvement(migrations): log better errors#4260
icecrasher321 merged 1 commit intostagingfrom
improvement/logging-migrations

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

Log improved migration errors

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Building Building Preview, Comment Apr 22, 2026 5:05am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 22, 2026

PR Summary

Medium Risk
Medium risk because it changes a schema migration to drop a uniqueness constraint earlier, which affects production data constraints and could allow unexpected duplicates if the later composite indexes don’t apply as intended. The migrate script change is low risk but will increase log verbosity and may expose query text/parameters in logs.

Overview
Updates migration 0194_careless_pete_wisdom.sql to drop the legacy global unique index on permission_group_member.user_id earlier (before the clone insert), preventing uniqueness violations during permission-group/member cloning, and removes the later redundant drop step.

Enhances packages/db/scripts/migrate.ts error handling by printing Postgres driver diagnostic fields (e.g., code, constraint/table/column, hint) plus failing query/parameters, nested causes, and stack traces to make failed migrations easier to debug.

Reviewed by Cursor Bugbot for commit 96a3b51. Configure here.

@icecrasher321 icecrasher321 merged commit 41a1b50 into staging Apr 22, 2026
9 of 10 checks passed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

This PR makes two improvements to the migration tooling: it moves the DROP INDEX permission_group_member_user_id_unique earlier in migration 0194 (before the workspace-clone INSERT) to prevent a constraint violation during cloning, and it replaces the bare error.message log in migrate.ts with a comprehensive printMigrationError helper that surfaces all PG-specific error fields (code, detail, hint, constraint, table, etc.) as well as the failing query and cause chain.

Confidence Score: 5/5

Safe to merge; the SQL fix is correct and the only finding is a P2 naming mismatch in the logging helper

Both changes are low-risk: the SQL migration fix is a correctness improvement to an existing migration, and the migrate.ts change only affects error logging output. The one identified issue (snake_case vs camelCase field names) means a few diagnostic fields would silently not print, but does not affect migration correctness or CI.

packages/db/scripts/migrate.ts — camelCase field names should be verified against the postgres.js driver

Important Files Changed

Filename Overview
packages/db/scripts/migrate.ts Adds printMigrationError to emit all PG error fields; a few snake_case field names won't match the postgres.js driver's camelCase properties
packages/db/migrations/0194_careless_pete_wisdom.sql Moves DROP INDEX permission_group_member_user_id_unique earlier (step 1c) so it runs before the workspace-clone INSERT, preventing a constraint violation; replaces the now-redundant late drop with a comment

Sequence Diagram

sequenceDiagram
    participant Runner as migrate.ts
    participant Drizzle as drizzle migrate()
    participant PG as PostgreSQL

    Runner->>Drizzle: migrate(client, { migrationsFolder })
    Drizzle->>PG: Apply SQL statements
    alt Success
        PG-->>Drizzle: OK
        Drizzle-->>Runner: resolved
        Runner->>Runner: console.log("Migrations applied successfully.")
    else Error
        PG-->>Drizzle: PostgresError (code, detail, hint, constraint, ...)
        Drizzle-->>Runner: throws error
        Runner->>Runner: printMigrationError(error)
        Note over Runner: Logs message, PG fields,<br/>failing query, cause chain, stack
        Runner->>Runner: process.exit(1)
    end
    Runner->>PG: client.end()
Loading

Reviews (1): Last reviewed commit: "improvement(migrations): log better erro..." | Re-trigger Greptile

Comment on lines +38 to +60
const pgFields = [
'code',
'severity',
'severity_local',
'detail',
'hint',
'schema',
'schema_name',
'table',
'table_name',
'column',
'column_name',
'constraint',
'constraint_name',
'data_type',
'where',
'internal_query',
'internal_position',
'position',
'routine',
'file',
'line',
] as const
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Some snake_case field names won't match postgres.js error fields

The postgres (porsager/postgres) driver uses camelCase for several extended error fields. The entries internal_query, internal_position, data_type, and severity_local will never match a property on a PostgresError object — the driver actually exposes internalQuery, internalPosition, dataType, and severityLocal. These fields are silent no-ops today, meaning useful diagnostics (especially internalQuery, which shows the PL/pgSQL statement that failed) are silently dropped.

Suggested change
const pgFields = [
'code',
'severity',
'severity_local',
'detail',
'hint',
'schema',
'schema_name',
'table',
'table_name',
'column',
'column_name',
'constraint',
'constraint_name',
'data_type',
'where',
'internal_query',
'internal_position',
'position',
'routine',
'file',
'line',
] as const
const pgFields = [
'code',
'severity',
'severityLocal',
'detail',
'hint',
'schema',
'schemaName',
'table',
'tableName',
'column',
'columnName',
'constraint',
'constraintName',
'dataType',
'dataTypeName',
'where',
'internalQuery',
'internalPosition',
'position',
'routine',
'file',
'line',
] as const

@waleedlatif1 waleedlatif1 deleted the improvement/logging-migrations branch April 22, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant