From a72acc627d63645c3daf69d230c82d8c6f98db24 Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 07:56:06 +0200 Subject: [PATCH 1/7] feat: add ability to apply migrations in throwaway db to test if they apply successfully --- packages/database/dev/src/lib/migrations.ts | 150 ++++++++++++++------ packages/database/dev/src/main.ts | 55 ++++++- packages/dev/src/main.ts | 2 +- 3 files changed, 164 insertions(+), 43 deletions(-) diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index 9ae68af22..4702d65be 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -5,43 +5,42 @@ import type { Dirent } from 'node:fs' import type { SQLExecutor } from './sql-executor.js' -const MIGRATION_NAME_PATTERN = /^\d+_.+$/ +const MIGRATION_NAME_PATTERN = /^(\d+)_.+$/ const MIGRATION_FILE = 'migration.sql' const SQL_EXTENSION = '.sql' const TRACKING_SCHEMA = 'netlify' const TRACKING_TABLE = `${TRACKING_SCHEMA}.migrations` -interface Migration { +export interface Migration { name: string + version: number sqlPath: string } +function nameAndFileToMigration(name: string, sqlPath: string): Migration | null { + const match = MIGRATION_NAME_PATTERN.exec(name) + if (!match) { + return null + } + + return { + name, + version: parseInt(match[1], 10), + sqlPath, + } +} + // Maps a directory entry to a Migration, or null if it isn't one. Supports // both directory-style migrations (`_/migration.sql`) and flat // file migrations (`_.sql`). -function toMigration(entry: Dirent, migrationsDirectory: string): Migration | null { +function direntToMigration(entry: Dirent, migrationsDirectory: string): Migration | null { if (entry.isDirectory()) { - if (!MIGRATION_NAME_PATTERN.test(entry.name)) { - return null - } - - return { - name: entry.name, - sqlPath: join(migrationsDirectory, entry.name, MIGRATION_FILE), - } + return nameAndFileToMigration(entry.name, join(migrationsDirectory, entry.name, MIGRATION_FILE)) } if (entry.isFile() && entry.name.endsWith(SQL_EXTENSION)) { const name = entry.name.slice(0, -SQL_EXTENSION.length) - - if (!MIGRATION_NAME_PATTERN.test(name)) { - return null - } - - return { - name, - sqlPath: join(migrationsDirectory, entry.name), - } + return nameAndFileToMigration(name, join(migrationsDirectory, entry.name)) } return null @@ -59,11 +58,67 @@ export async function initializeTrackingTable(db: SQLExecutor): Promise { `) } -export async function applyMigrations( +export class MissingMigrationDirectoryError extends Error { + public readonly migrationsDirectory: string + constructor(options: ErrorOptions & { migrationsDirectory: string }) { + super(`Migration directory not found: ${options.migrationsDirectory}`, { cause: options.cause }) + this.name = 'MissingMigrationDirectoryError' + this.migrationsDirectory = options.migrationsDirectory + } +} + +export class DuplicateMigrationVersionsError extends Error { + public readonly versionsWithMultipleMigrations: Record + constructor(options: { versionsWithMultipleMigrations: Map> }) { + const versionsWithMultipleMigrationsObject = Object.fromEntries( + Array.from(options.versionsWithMultipleMigrations.entries()).map(([version, migrations]) => [ + version, + Array.from(migrations), + ]), + ) + + const duplicateVersionsDetails = Object.entries(versionsWithMultipleMigrationsObject) + .map(([version, migrations]) => { + return ` - Version ${version}:\n${migrations.map((m) => ` - ${m.sqlPath}`).join('\n')}` + }) + .join('\n') + + super(`Duplicate migration versions found:\n${duplicateVersionsDetails}`) + this.name = 'DuplicateMigrationVersionError' + this.versionsWithMultipleMigrations = versionsWithMultipleMigrationsObject + } +} + +export class MigrationsApplyError extends Error { + public readonly appliedMigrations: Migration[] + public readonly migrationCausingError: Migration + public readonly remainingMigrations: Migration[] + public readonly cause: Error + constructor(options: { + appliedMigrations: Migration[] + migrationCausingError: Migration + remainingMigrations: Migration[] + cause: unknown + }) { + super(`Failed to apply migrations`, { cause: options.cause }) + this.name = 'MigrationsApplyError' + this.appliedMigrations = options.appliedMigrations + this.migrationCausingError = options.migrationCausingError + this.remainingMigrations = options.remainingMigrations + this.cause = options.cause instanceof Error ? options.cause : new Error(String(options.cause)) + } +} + +export async function applyMigrations(...args: Parameters): Promise { + const migrations = await applyMigrationsWithDetails(...args) + return migrations.map((m) => m.name) +} + +export async function applyMigrationsWithDetails( db: SQLExecutor, migrationsDirectory: string, target?: string, -): Promise { +): Promise { await initializeTrackingTable(db) let migrations: Migration[] @@ -71,25 +126,31 @@ export async function applyMigrations( try { const dirents = await readdir(migrationsDirectory, { withFileTypes: true }) migrations = dirents - .map((entry) => toMigration(entry, migrationsDirectory)) + .map((entry) => direntToMigration(entry, migrationsDirectory)) .filter((m): m is Migration => m !== null) - } catch { - throw new Error(`Migration directory not found: ${migrationsDirectory}`) + } catch (error) { + throw new MissingMigrationDirectoryError({ migrationsDirectory, cause: error }) } - const seen = new Map() + const seenVersions = new Map>() + const versionsWithMultipleMigrations = new Map>() for (const migration of migrations) { - const existing = seen.get(migration.name) - if (existing) { - throw new Error( - `Duplicate migration name "${migration.name}" in ${migrationsDirectory}: ` + - `found both "${existing}" and "${migration.sqlPath}". Remove one before applying migrations.`, - ) + let existingVersions = seenVersions.get(migration.version) + if (!existingVersions) { + existingVersions = new Set() + seenVersions.set(migration.version, existingVersions) } - seen.set(migration.name, migration.sqlPath) + if (existingVersions.size === 1) { + versionsWithMultipleMigrations.set(migration.version, existingVersions) + } + existingVersions.add(migration) + } + + if (versionsWithMultipleMigrations.size > 0) { + throw new DuplicateMigrationVersionsError({ versionsWithMultipleMigrations }) } - migrations.sort((a, b) => a.name.localeCompare(b.name)) + migrations.sort((a, b) => a.version - b.version) let migrationsToConsider: Migration[] @@ -114,7 +175,7 @@ export async function applyMigrations( const result = await db.query<{ name: string }>(`SELECT name FROM ${TRACKING_TABLE} WHERE name = ANY($1)`, [names]) const alreadyApplied = new Set(result.rows.map((row) => row.name)) - const applied: string[] = [] + const applied: Migration[] = [] for (const migration of migrationsToConsider) { if (alreadyApplied.has(migration.name)) { @@ -134,12 +195,21 @@ export async function applyMigrations( throw new Error(`Failed to read migration "${migration.name}" at "${migration.sqlPath}": ${err.message}`) } - await db.transaction(async (tx) => { - await tx.exec(sql) - await tx.query(`INSERT INTO ${TRACKING_TABLE} (name) VALUES ($1)`, [migration.name]) - }) + try { + await db.transaction(async (tx) => { + await tx.exec(sql) + await tx.query(`INSERT INTO ${TRACKING_TABLE} (name) VALUES ($1)`, [migration.name]) + }) + } catch (error) { + throw new MigrationsApplyError({ + cause: error, + appliedMigrations: applied, + migrationCausingError: migration, + remainingMigrations: migrationsToConsider.slice(applied.length + 1), + }) + } - applied.push(migration.name) + applied.push(migration) } return applied diff --git a/packages/database/dev/src/main.ts b/packages/database/dev/src/main.ts index e33f379af..627476cfe 100644 --- a/packages/database/dev/src/main.ts +++ b/packages/database/dev/src/main.ts @@ -6,13 +6,21 @@ import type { ConnectionState, MessageResponse } from 'pg-gateway' import { fromNodeSocket } from 'pg-gateway/node' import { broadcastNotifications } from './lib/notifications.js' -import { applyMigrations, initializeTrackingTable } from './lib/migrations.js' +import { + applyMigrations, + applyMigrationsWithDetails, + DuplicateMigrationVersionsError, + initializeTrackingTable, + MigrationsApplyError, + MissingMigrationDirectoryError, +} from './lib/migrations.js' import type { SQLExecutor } from './lib/sql-executor.js' export { applyMigrations, initializeTrackingTable } from './lib/migrations.js' export type { SQLExecutor } from './lib/sql-executor.js' const DEFAULT_HOST = 'localhost' +const IN_MEMORY_DIRECTORY = 'memory://' type Logger = (...message: unknown[]) => void @@ -71,7 +79,7 @@ export class NetlifyDB implements SQLExecutor { } async start(): Promise { - if (this.directory) { + if (this.directory && this.directory !== IN_MEMORY_DIRECTORY) { await mkdir(this.directory, { recursive: true }) } @@ -220,3 +228,46 @@ export class NetlifyDB implements SQLExecutor { }) } } + +export async function testMigrationsApply(migrationsDirectory: string) { + const inMemoryNetlifyDB = new NetlifyDB({ directory: IN_MEMORY_DIRECTORY }) + await inMemoryNetlifyDB.start() + + try { + return { + status: 'success' as const, + successfullyApplied: await applyMigrationsWithDetails(inMemoryNetlifyDB, migrationsDirectory), + } + } catch (error) { + if (error instanceof MissingMigrationDirectoryError) { + return { + status: 'success' as const, + successfullyApplied: [], + } + } + + if (error instanceof DuplicateMigrationVersionsError) { + return { + status: 'duplicate_versions' as const, + duplicateVersions: error.versionsWithMultipleMigrations, + } + } + + if (error instanceof MigrationsApplyError) { + return { + status: 'dry_run_apply_error' as const, + appliedMigrations: error.appliedMigrations, + migrationCausingError: error.migrationCausingError, + remainingMigrations: error.remainingMigrations, + cause: error.cause, + } + } + + return { + status: 'error' as const, + error, + } + } finally { + await inMemoryNetlifyDB.stop() + } +} diff --git a/packages/dev/src/main.ts b/packages/dev/src/main.ts index 23999cf7c..75deacd30 100644 --- a/packages/dev/src/main.ts +++ b/packages/dev/src/main.ts @@ -24,7 +24,7 @@ import { RedirectsHandler } from '@netlify/redirects' import { StaticHandler } from '@netlify/static' import { NetlifyDB } from '@netlify/database-dev' -export { applyMigrations, initializeTrackingTable, resetDatabase } from '@netlify/database-dev' +export { applyMigrations, initializeTrackingTable, resetDatabase, testMigrationsApply } from '@netlify/database-dev' export type { SQLExecutor } from '@netlify/database-dev' import { InjectedEnvironmentVariable, injectEnvVariables } from './lib/env.js' From 1688c1f72f0f83a1a61576b00d3075ebcfddae77 Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 08:26:49 +0200 Subject: [PATCH 2/7] load migration content before we apply any of migration to catch those problems ahead of time --- packages/database/dev/src/lib/migrations.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index 4702d65be..dc6ee44cd 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -175,17 +175,15 @@ export async function applyMigrationsWithDetails( const result = await db.query<{ name: string }>(`SELECT name FROM ${TRACKING_TABLE} WHERE name = ANY($1)`, [names]) const alreadyApplied = new Set(result.rows.map((row) => row.name)) - const applied: Migration[] = [] - + const migrationsToApplyWithContent: { migration: Migration; sql: string }[] = [] for (const migration of migrationsToConsider) { if (alreadyApplied.has(migration.name)) { continue } - let sql: string - try { - sql = await readFile(migration.sqlPath, 'utf-8') + const sql = await readFile(migration.sqlPath, 'utf-8') + migrationsToApplyWithContent.push({ migration, sql }) } catch (error) { const err = error as NodeJS.ErrnoException if (err.code === 'ENOENT') { @@ -194,7 +192,10 @@ export async function applyMigrationsWithDetails( throw new Error(`Failed to read migration "${migration.name}" at "${migration.sqlPath}": ${err.message}`) } + } + const applied: Migration[] = [] + for (const { migration, sql } of migrationsToApplyWithContent) { try { await db.transaction(async (tx) => { await tx.exec(sql) From b913b3d0854237f775b7345401c722d514ae9815 Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 08:55:24 +0200 Subject: [PATCH 3/7] smarter duplicate version/name error --- .../database/dev/src/lib/migrations.test.ts | 15 +++- packages/database/dev/src/lib/migrations.ts | 78 ++++++++++++------- packages/database/dev/src/main.ts | 2 +- 3 files changed, 67 insertions(+), 28 deletions(-) diff --git a/packages/database/dev/src/lib/migrations.test.ts b/packages/database/dev/src/lib/migrations.test.ts index 61a3fa5c8..7abd4643c 100644 --- a/packages/database/dev/src/lib/migrations.test.ts +++ b/packages/database/dev/src/lib/migrations.test.ts @@ -280,7 +280,20 @@ test('Throws when a directory and flat file share the same name', async () => { db = await PGlite.create() - await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow(/Duplicate migration name "0001_create_users"/) + await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow( + /Name "0001_create_users" is used by multiple migrations/, + ) +}) + +test('Throws when distinct migrations share a version', async () => { + tmpDir = await tmp.dir() + + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + db = await PGlite.create() + + await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow(/Version 1 is used by multiple migrations/) }) test('Ignores flat .sql files whose name does not match the migration pattern', async () => { diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index dc6ee44cd..f88afea97 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -67,25 +67,29 @@ export class MissingMigrationDirectoryError extends Error { } } -export class DuplicateMigrationVersionsError extends Error { - public readonly versionsWithMultipleMigrations: Record - constructor(options: { versionsWithMultipleMigrations: Map> }) { - const versionsWithMultipleMigrationsObject = Object.fromEntries( - Array.from(options.versionsWithMultipleMigrations.entries()).map(([version, migrations]) => [ - version, - Array.from(migrations), - ]), - ) +export type MigrationConflict = + | { kind: 'duplicate-name'; version: number; name: string; migrations: Migration[] } + | { kind: 'duplicate-version'; version: number; migrations: Migration[] } - const duplicateVersionsDetails = Object.entries(versionsWithMultipleMigrationsObject) - .map(([version, migrations]) => { - return ` - Version ${version}:\n${migrations.map((m) => ` - ${m.sqlPath}`).join('\n')}` +export class DuplicateMigrationVersionsError extends Error { + public readonly conflicts: MigrationConflict[] + public readonly migrationsDirectory: string + constructor(options: { conflicts: MigrationConflict[]; migrationsDirectory: string }) { + const conflictDetails = options.conflicts + .map((conflict) => { + if (conflict.kind === 'duplicate-name') { + const paths = conflict.migrations.map((m) => ` - ${m.sqlPath}`).join('\n') + return ` - Name "${conflict.name}" is used by multiple migrations. Remove one before applying migrations:\n${paths}` + } + const paths = conflict.migrations.map((m) => ` - ${m.sqlPath} (${m.name})`).join('\n') + return ` - Version ${conflict.version} is used by multiple migrations. If these are duplicates, remove one; otherwise increment one migration's version, ensuring the resulting order applies them correctly:\n${paths}` }) .join('\n') - super(`Duplicate migration versions found:\n${duplicateVersionsDetails}`) - this.name = 'DuplicateMigrationVersionError' - this.versionsWithMultipleMigrations = versionsWithMultipleMigrationsObject + super(`Duplicate migrations found in ${options.migrationsDirectory}:\n${conflictDetails}`) + this.name = 'DuplicateMigrationVersionsError' + this.conflicts = options.conflicts + this.migrationsDirectory = options.migrationsDirectory } } @@ -132,22 +136,44 @@ export async function applyMigrationsWithDetails( throw new MissingMigrationDirectoryError({ migrationsDirectory, cause: error }) } - const seenVersions = new Map>() - const versionsWithMultipleMigrations = new Map>() + const migrationsByVersion = new Map() for (const migration of migrations) { - let existingVersions = seenVersions.get(migration.version) - if (!existingVersions) { - existingVersions = new Set() - seenVersions.set(migration.version, existingVersions) + const existing = migrationsByVersion.get(migration.version) + if (existing) { + existing.push(migration) + } else { + migrationsByVersion.set(migration.version, [migration]) } - if (existingVersions.size === 1) { - versionsWithMultipleMigrations.set(migration.version, existingVersions) + } + + const conflicts: MigrationConflict[] = [] + for (const [version, migrationsForVersion] of migrationsByVersion) { + if (migrationsForVersion.length < 2) continue + + const byName = new Map() + for (const m of migrationsForVersion) { + const list = byName.get(m.name) + if (list) { + list.push(m) + } else { + byName.set(m.name, [m]) + } + } + + for (const [name, migrationsWithName] of byName) { + if (migrationsWithName.length >= 2) { + conflicts.push({ kind: 'duplicate-name', version, name, migrations: migrationsWithName }) + } + } + + if (byName.size >= 2) { + const representatives = Array.from(byName.values()).map((migs) => migs[0]) + conflicts.push({ kind: 'duplicate-version', version, migrations: representatives }) } - existingVersions.add(migration) } - if (versionsWithMultipleMigrations.size > 0) { - throw new DuplicateMigrationVersionsError({ versionsWithMultipleMigrations }) + if (conflicts.length > 0) { + throw new DuplicateMigrationVersionsError({ conflicts, migrationsDirectory }) } migrations.sort((a, b) => a.version - b.version) diff --git a/packages/database/dev/src/main.ts b/packages/database/dev/src/main.ts index 627476cfe..d3b3c752a 100644 --- a/packages/database/dev/src/main.ts +++ b/packages/database/dev/src/main.ts @@ -249,7 +249,7 @@ export async function testMigrationsApply(migrationsDirectory: string) { if (error instanceof DuplicateMigrationVersionsError) { return { status: 'duplicate_versions' as const, - duplicateVersions: error.versionsWithMultipleMigrations, + conflicts: error.conflicts, } } From cd8953b2b093a1c2f2313ad2c266e064b89b64fc Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 09:26:03 +0200 Subject: [PATCH 4/7] fix lint --- packages/database/dev/src/lib/migrations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index f88afea97..f3c6409aa 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -82,7 +82,7 @@ export class DuplicateMigrationVersionsError extends Error { return ` - Name "${conflict.name}" is used by multiple migrations. Remove one before applying migrations:\n${paths}` } const paths = conflict.migrations.map((m) => ` - ${m.sqlPath} (${m.name})`).join('\n') - return ` - Version ${conflict.version} is used by multiple migrations. If these are duplicates, remove one; otherwise increment one migration's version, ensuring the resulting order applies them correctly:\n${paths}` + return ` - Version ${conflict.version.toString()} is used by multiple migrations. If these are duplicates, remove one; otherwise increment one migration's version, ensuring the resulting order applies them correctly:\n${paths}` }) .join('\n') From 3bbec1e225b635f79b3a467109e56b41bc2f3b21 Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 12:07:01 +0200 Subject: [PATCH 5/7] restructure errors, formalize missing/unreadable migration files, sync with netlify/build migration name matching --- .../database/dev/src/lib/migrations.test.ts | 19 +- packages/database/dev/src/lib/migrations.ts | 274 +++++++++++++++--- .../testMigrationsInEphemeralDatabase.test.ts | 203 +++++++++++++ packages/database/dev/src/main.ts | 51 +--- packages/dev/src/main.ts | 15 +- 5 files changed, 475 insertions(+), 87 deletions(-) create mode 100644 packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts diff --git a/packages/database/dev/src/lib/migrations.test.ts b/packages/database/dev/src/lib/migrations.test.ts index 7abd4643c..7eac8d9f0 100644 --- a/packages/database/dev/src/lib/migrations.test.ts +++ b/packages/database/dev/src/lib/migrations.test.ts @@ -177,7 +177,7 @@ test('Throws MigrationFileNotFoundError for directory missing migration.sql', as db = await PGlite.create() await expect(applyMigrations(db, tmpDir.path, '0001_missing_file')).rejects.toThrow( - /Migration SQL file not found: .*0001_missing_file[/\\]migration.sql/, + /Migration "0001_missing_file" is missing its SQL file at .*0001_missing_file[/\\]migration\.sql/, ) }) @@ -281,7 +281,7 @@ test('Throws when a directory and flat file share the same name', async () => { db = await PGlite.create() await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow( - /Name "0001_create_users" is used by multiple migrations/, + /Two or more migrations share the name "0001_create_users"/, ) }) @@ -310,3 +310,18 @@ test('Ignores flat .sql files whose name does not match the migration pattern', expect(applied).toEqual(['0001_create_users']) }) + +test('Skips entries with disallowed characters (uppercase, spaces, special) in the slug', async () => { + tmpDir = await tmp.dir() + + await createFlatMigration(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0002_CAPS_NOT_OK', 'CREATE TABLE caps (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0003_special!chars', 'CREATE TABLE bad (id SERIAL PRIMARY KEY)') + await fs.writeFile(join(tmpDir.path, '0004_with space.sql'), 'CREATE TABLE bad (id SERIAL PRIMARY KEY)') + + db = await PGlite.create() + + const applied = await applyMigrations(db, tmpDir.path) + + expect(applied).toEqual(['0001_create_users']) +}) diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index f3c6409aa..daf37fc2d 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -5,7 +5,7 @@ import type { Dirent } from 'node:fs' import type { SQLExecutor } from './sql-executor.js' -const MIGRATION_NAME_PATTERN = /^(\d+)_.+$/ +const MIGRATION_NAME_PATTERN = /^(\d+)_[a-z0-9_-]+$/ const MIGRATION_FILE = 'migration.sql' const SQL_EXTENSION = '.sql' const TRACKING_SCHEMA = 'netlify' @@ -17,6 +17,171 @@ export interface Migration { sqlPath: string } +export interface PgErrorDetails { + message: string + code?: string + position?: number + hint?: string + detail?: string + severity?: string +} + +export type MigrationIssue = + | { + kind: 'duplicate-name' + version: number + name: string + files: Migration[] + summary: string + remediation: string + } + | { + kind: 'duplicate-version' + version: number + files: Migration[] + summary: string + remediation: string + } + | { + kind: 'apply-failure' + migration: Migration + appliedBefore: Migration[] + remaining: Migration[] + pgError: PgErrorDetails + summary: string + remediation: string + } + | { + kind: 'migration-file-error' + reason: 'missing' | 'unreadable' + migrationName: string + sqlPath: string + summary: string + remediation: string + cause?: unknown + } + | { + kind: 'unknown' + summary: string + remediation: string + cause?: unknown + } + +const optionalString = (value: unknown): string | undefined => + typeof value === 'string' && value !== '' ? value : undefined + +const parseOptionalInt = (value: unknown): number | undefined => { + if (typeof value === 'number' && Number.isFinite(value)) return value + if (typeof value === 'string') { + const n = parseInt(value, 10) + return Number.isFinite(n) ? n : undefined + } + return undefined +} + +export function normalizePgError(error: unknown): PgErrorDetails { + if (!(error instanceof Error)) { + return { message: String(error) } + } + const e = error as Error & Record + return { + message: e.message, + code: optionalString(e.code), + position: parseOptionalInt(e.position), + hint: optionalString(e.hint), + detail: optionalString(e.detail), + severity: optionalString(e.severity), + } +} + +const buildDuplicateNameIssue = (version: number, name: string, files: Migration[]): MigrationIssue => ({ + kind: 'duplicate-name', + version, + name, + files, + summary: `Two or more migrations share the name "${name}".`, + remediation: `Delete one of the duplicate files before applying.`, +}) + +const buildDuplicateVersionIssue = (version: number, files: Migration[]): MigrationIssue => { + const fileList = files.map((m) => `"${m.name}"`).join(', ') + return { + kind: 'duplicate-version', + version, + files, + summary: `Version ${String(version)} is used by multiple migrations: ${fileList}.`, + remediation: `Increment one of the prefixes so ordering is unambiguous, or delete a file if it was an unintended duplicate.`, + } +} + +const buildApplyFailureIssue = (options: { + migration: Migration + appliedBefore: Migration[] + remaining: Migration[] + cause: unknown +}): MigrationIssue & { kind: 'apply-failure' } => { + const pgError = normalizePgError(options.cause) + const codeNote = pgError.code ? ` Postgres returned SQLSTATE ${pgError.code}; look that up for common causes.` : '' + return { + kind: 'apply-failure', + migration: options.migration, + appliedBefore: options.appliedBefore, + remaining: options.remaining, + pgError, + summary: `Migration "${options.migration.name}" failed to apply: ${pgError.message}`, + remediation: + `This may be a problem in the SQL of "${options.migration.name}" itself (for example, a syntax error), ` + + `or in the cumulative database state left by previously applied migrations ` + + `(for example, the migration tries to create an object that an earlier migration already created, ` + + `or references one that was never created).${codeNote} ` + + `Resolve the issue in the failing migration or in the prior ones before deploying.`, + } +} + +const buildMigrationFileIssue = (options: { + migrationName: string + sqlPath: string + reason: 'missing' | 'unreadable' + cause?: unknown +}): MigrationIssue & { kind: 'migration-file-error' } => { + if (options.reason === 'missing') { + return { + kind: 'migration-file-error', + reason: 'missing', + migrationName: options.migrationName, + sqlPath: options.sqlPath, + summary: `Migration "${options.migrationName}" is missing its SQL file at ${options.sqlPath}.`, + remediation: `Create the file at ${options.sqlPath}, or remove the migration's directory if it isn't intended.`, + cause: options.cause, + } + } + const causeMessage = options.cause instanceof Error ? options.cause.message : String(options.cause) + return { + kind: 'migration-file-error', + reason: 'unreadable', + migrationName: options.migrationName, + sqlPath: options.sqlPath, + summary: `Could not read migration "${options.migrationName}" at ${options.sqlPath}: ${causeMessage}`, + remediation: `Check filesystem permissions on ${options.sqlPath} and that it isn't held open by another process.`, + cause: options.cause, + } +} + +const formatIssueAsErrorLine = (issue: MigrationIssue): string => { + if (issue.kind === 'duplicate-name') { + const paths = issue.files.map((m) => ` - ${m.sqlPath}`).join('\n') + return ` - ${issue.summary} ${issue.remediation}\n${paths}` + } + if (issue.kind === 'duplicate-version') { + const paths = issue.files.map((m) => ` - ${m.sqlPath} (${m.name})`).join('\n') + return ` - ${issue.summary} ${issue.remediation}\n${paths}` + } + if (issue.kind === 'apply-failure' || issue.kind === 'migration-file-error') { + return ` - ${issue.summary}\n ${issue.remediation}` + } + return ` - ${issue.summary} ${issue.remediation}` +} + function nameAndFileToMigration(name: string, sqlPath: string): Migration | null { const match = MIGRATION_NAME_PATTERN.exec(name) if (!match) { @@ -67,52 +232,60 @@ export class MissingMigrationDirectoryError extends Error { } } -export type MigrationConflict = - | { kind: 'duplicate-name'; version: number; name: string; migrations: Migration[] } - | { kind: 'duplicate-version'; version: number; migrations: Migration[] } - export class DuplicateMigrationVersionsError extends Error { - public readonly conflicts: MigrationConflict[] + public readonly issues: MigrationIssue[] public readonly migrationsDirectory: string - constructor(options: { conflicts: MigrationConflict[]; migrationsDirectory: string }) { - const conflictDetails = options.conflicts - .map((conflict) => { - if (conflict.kind === 'duplicate-name') { - const paths = conflict.migrations.map((m) => ` - ${m.sqlPath}`).join('\n') - return ` - Name "${conflict.name}" is used by multiple migrations. Remove one before applying migrations:\n${paths}` - } - const paths = conflict.migrations.map((m) => ` - ${m.sqlPath} (${m.name})`).join('\n') - return ` - Version ${conflict.version.toString()} is used by multiple migrations. If these are duplicates, remove one; otherwise increment one migration's version, ensuring the resulting order applies them correctly:\n${paths}` - }) - .join('\n') - - super(`Duplicate migrations found in ${options.migrationsDirectory}:\n${conflictDetails}`) + constructor(options: { issues: MigrationIssue[]; migrationsDirectory: string }) { + const detail = options.issues.map(formatIssueAsErrorLine).join('\n') + super(`Duplicate migrations found in ${options.migrationsDirectory}:\n${detail}`) this.name = 'DuplicateMigrationVersionsError' - this.conflicts = options.conflicts + this.issues = options.issues this.migrationsDirectory = options.migrationsDirectory } } export class MigrationsApplyError extends Error { - public readonly appliedMigrations: Migration[] - public readonly migrationCausingError: Migration - public readonly remainingMigrations: Migration[] + public readonly issue: MigrationIssue & { kind: 'apply-failure' } public readonly cause: Error - constructor(options: { - appliedMigrations: Migration[] - migrationCausingError: Migration - remainingMigrations: Migration[] - cause: unknown - }) { - super(`Failed to apply migrations`, { cause: options.cause }) + constructor(options: { issue: MigrationIssue & { kind: 'apply-failure' } }) { + super(`${options.issue.summary}\n${options.issue.remediation}`, { cause: options.issue.pgError }) this.name = 'MigrationsApplyError' - this.appliedMigrations = options.appliedMigrations - this.migrationCausingError = options.migrationCausingError - this.remainingMigrations = options.remainingMigrations - this.cause = options.cause instanceof Error ? options.cause : new Error(String(options.cause)) + this.issue = options.issue + this.cause = new Error(options.issue.pgError.message) + } +} + +export class MigrationFileError extends Error { + public readonly issue: MigrationIssue & { kind: 'migration-file-error' } + constructor(options: { issue: MigrationIssue & { kind: 'migration-file-error' } }) { + super(`${options.issue.summary}\n${options.issue.remediation}`, { cause: options.issue.cause }) + this.name = 'MigrationFileError' + this.issue = options.issue } } +export function errorToIssues(error: unknown): MigrationIssue[] { + if (error instanceof MissingMigrationDirectoryError) { + return [] + } + if (error instanceof DuplicateMigrationVersionsError) { + return error.issues + } + if (error instanceof MigrationsApplyError || error instanceof MigrationFileError) { + return [error.issue] + } + + const message = error instanceof Error ? error.message : String(error) + return [ + { + kind: 'unknown', + summary: `An unexpected error occurred while validating migrations: ${message}`, + remediation: `Re-run the command. If the problem persists, file a bug report with the error details above.`, + cause: error, + }, + ] +} + export async function applyMigrations(...args: Parameters): Promise { const migrations = await applyMigrationsWithDetails(...args) return migrations.map((m) => m.name) @@ -146,7 +319,7 @@ export async function applyMigrationsWithDetails( } } - const conflicts: MigrationConflict[] = [] + const issues: MigrationIssue[] = [] for (const [version, migrationsForVersion] of migrationsByVersion) { if (migrationsForVersion.length < 2) continue @@ -162,18 +335,18 @@ export async function applyMigrationsWithDetails( for (const [name, migrationsWithName] of byName) { if (migrationsWithName.length >= 2) { - conflicts.push({ kind: 'duplicate-name', version, name, migrations: migrationsWithName }) + issues.push(buildDuplicateNameIssue(version, name, migrationsWithName)) } } if (byName.size >= 2) { const representatives = Array.from(byName.values()).map((migs) => migs[0]) - conflicts.push({ kind: 'duplicate-version', version, migrations: representatives }) + issues.push(buildDuplicateVersionIssue(version, representatives)) } } - if (conflicts.length > 0) { - throw new DuplicateMigrationVersionsError({ conflicts, migrationsDirectory }) + if (issues.length > 0) { + throw new DuplicateMigrationVersionsError({ issues, migrationsDirectory }) } migrations.sort((a, b) => a.version - b.version) @@ -212,11 +385,14 @@ export async function applyMigrationsWithDetails( migrationsToApplyWithContent.push({ migration, sql }) } catch (error) { const err = error as NodeJS.ErrnoException - if (err.code === 'ENOENT') { - throw new Error(`Migration SQL file not found: ${migration.sqlPath}`) - } - - throw new Error(`Failed to read migration "${migration.name}" at "${migration.sqlPath}": ${err.message}`) + throw new MigrationFileError({ + issue: buildMigrationFileIssue({ + migrationName: migration.name, + sqlPath: migration.sqlPath, + reason: err.code === 'ENOENT' ? 'missing' : 'unreadable', + cause: err, + }), + }) } } @@ -229,10 +405,12 @@ export async function applyMigrationsWithDetails( }) } catch (error) { throw new MigrationsApplyError({ - cause: error, - appliedMigrations: applied, - migrationCausingError: migration, - remainingMigrations: migrationsToConsider.slice(applied.length + 1), + issue: buildApplyFailureIssue({ + migration, + appliedBefore: applied, + remaining: migrationsToConsider.slice(applied.length + 1), + cause: error, + }), }) } diff --git a/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts new file mode 100644 index 000000000..72aafe142 --- /dev/null +++ b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts @@ -0,0 +1,203 @@ +import { promises as fs } from 'node:fs' +import { join } from 'node:path' + +import tmp from 'tmp-promise' +import { test, expect, afterEach } from 'vitest' + +import { testMigrationsInEphemeralDatabase } from '../main.js' + +let tmpDir: tmp.DirectoryResult | undefined + +afterEach(async () => { + if (tmpDir) { + await tmpDir.cleanup() + tmpDir = undefined + } +}) + +async function createMigrationDir(basePath: string, name: string, sql: string) { + const dir = join(basePath, name) + await fs.mkdir(dir, { recursive: true }) + await fs.writeFile(join(dir, 'migration.sql'), sql) +} + +async function createFlatMigration(basePath: string, name: string, sql: string) { + await fs.writeFile(join(basePath, `${name}.sql`), sql) +} + +test('Returns success with applied migrations when all apply cleanly', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY, name TEXT)') + await createMigrationDir(tmpDir.path, '0002_add_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY, title TEXT)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('success') + if (result.status !== 'success') return + expect(result.applied).toHaveLength(2) + expect(result.applied.map((m) => m.name)).toEqual(['0001_create_users', '0002_add_posts']) +}) + +test('Returns success with empty applied when migrations directory is missing', async () => { + const result = await testMigrationsInEphemeralDatabase('/nonexistent/path/to/migrations') + + expect(result.status).toBe('success') + if (result.status !== 'success') return + expect(result.applied).toEqual([]) +}) + +test('Returns success with empty applied when migrations directory is empty', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('success') + if (result.status !== 'success') return + expect(result.applied).toEqual([]) +}) + +test('Returns failure with duplicate-name issue when a directory and flat file share a name', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('duplicate-name') + if (issue.kind !== 'duplicate-name') return + expect(issue.name).toBe('0001_create_users') + expect(issue.version).toBe(1) + expect(issue.files).toHaveLength(2) + expect(issue.summary).toBe('Two or more migrations share the name "0001_create_users".') + expect(issue.remediation).toBe('Delete one of the duplicate files before applying.') +}) + +test('Returns failure with duplicate-version issue when distinct migrations share a version', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('duplicate-version') + if (issue.kind !== 'duplicate-version') return + expect(issue.version).toBe(1) + expect(issue.files.map((f) => f.name).sort()).toEqual(['0001_create_posts', '0001_create_users']) + // File order in the summary depends on readdir order, so allow either. + expect(issue.summary).toMatch( + /^Version 1 is used by multiple migrations: ("0001_create_users", "0001_create_posts"|"0001_create_posts", "0001_create_users")\.$/, + ) + expect(issue.remediation).toBe( + 'Increment one of the prefixes so ordering is unambiguous, or delete a file if it was an unintended duplicate.', + ) +}) + +test('Reports both duplicate-name and duplicate-version issues at once', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + // Two with the same name + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + // A third with a different name but the same version — triggers duplicate-version too + await createFlatMigration(tmpDir.path, '0001_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + const kinds = result.issues.map((i) => i.kind).sort() + expect(kinds).toEqual(['duplicate-name', 'duplicate-version']) +}) + +test('Returns failure with migration-file-error (missing) when a migration directory has no migration.sql', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await fs.mkdir(join(tmpDir.path, '0001_create_users')) + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('migration-file-error') + if (issue.kind !== 'migration-file-error') return + expect(issue.reason).toBe('missing') + expect(issue.migrationName).toBe('0001_create_users') + expect(issue.sqlPath).toMatch(/0001_create_users[/\\]migration\.sql$/) + // The path is absolute and varies per run, so anchor the rest of the wording around it. + expect(issue.summary).toMatch( + /^Migration "0001_create_users" is missing its SQL file at .+0001_create_users[/\\]migration\.sql\.$/, + ) + expect(issue.remediation).toMatch( + /^Create the file at .+0001_create_users[/\\]migration\.sql, or remove the migration's directory if it isn't intended\.$/, + ) +}) + +test('Returns failure with apply-failure issue when a migration has a syntax error', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0002_bad', 'INVALID SQL SYNTAX HERE') + await createMigrationDir(tmpDir.path, '0003_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('apply-failure') + if (issue.kind !== 'apply-failure') return + expect(issue.migration.name).toBe('0002_bad') + expect(issue.appliedBefore.map((m) => m.name)).toEqual(['0001_create_users']) + expect(issue.remaining.map((m) => m.name)).toEqual(['0003_create_posts']) + expect(issue.pgError.code).toBe('42601') + expect(issue.pgError.message).toBe('syntax error at or near "INVALID"') + expect(issue.summary).toBe('Migration "0002_bad" failed to apply: syntax error at or near "INVALID"') + expect(issue.remediation).toBe( + 'This may be a problem in the SQL of "0002_bad" itself (for example, a syntax error), ' + + 'or in the cumulative database state left by previously applied migrations ' + + '(for example, the migration tries to create an object that an earlier migration already created, ' + + "or references one that was never created). Postgres returned SQLSTATE 42601; look that up for common causes. " + + 'Resolve the issue in the failing migration or in the prior ones before deploying.', + ) +}) + +test('Returns failure with apply-failure issue when a migration creates a relation that already exists', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0002_create_users_again', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + + const issue = result.issues[0] + expect(issue.kind).toBe('apply-failure') + if (issue.kind !== 'apply-failure') return + expect(issue.appliedBefore.map((m) => m.name)).toEqual(['0001_create_users']) + expect(issue.migration.name).toBe('0002_create_users_again') + expect(issue.pgError.code).toBe('42P07') + expect(issue.pgError.message).toBe('relation "users" already exists') + expect(issue.summary).toBe( + 'Migration "0002_create_users_again" failed to apply: relation "users" already exists', + ) + expect(issue.remediation).toBe( + 'This may be a problem in the SQL of "0002_create_users_again" itself (for example, a syntax error), ' + + 'or in the cumulative database state left by previously applied migrations ' + + '(for example, the migration tries to create an object that an earlier migration already created, ' + + "or references one that was never created). Postgres returned SQLSTATE 42P07; look that up for common causes. " + + 'Resolve the issue in the failing migration or in the prior ones before deploying.', + ) +}) diff --git a/packages/database/dev/src/main.ts b/packages/database/dev/src/main.ts index d3b3c752a..237211c8e 100644 --- a/packages/database/dev/src/main.ts +++ b/packages/database/dev/src/main.ts @@ -9,15 +9,19 @@ import { broadcastNotifications } from './lib/notifications.js' import { applyMigrations, applyMigrationsWithDetails, - DuplicateMigrationVersionsError, + errorToIssues, initializeTrackingTable, - MigrationsApplyError, - MissingMigrationDirectoryError, + type Migration, + type MigrationIssue, } from './lib/migrations.js' import type { SQLExecutor } from './lib/sql-executor.js' export { applyMigrations, initializeTrackingTable } from './lib/migrations.js' +export type { Migration, MigrationIssue, PgErrorDetails } from './lib/migrations.js' export type { SQLExecutor } from './lib/sql-executor.js' +export type TestMigrationsInEphemeralDatabaseResult = + | { status: 'success'; applied: Migration[] } + | { status: 'failure'; issues: MigrationIssue[] } const DEFAULT_HOST = 'localhost' const IN_MEMORY_DIRECTORY = 'memory://' @@ -229,44 +233,21 @@ export class NetlifyDB implements SQLExecutor { } } -export async function testMigrationsApply(migrationsDirectory: string) { +export async function testMigrationsInEphemeralDatabase( + migrationsDirectory: string, +): Promise { const inMemoryNetlifyDB = new NetlifyDB({ directory: IN_MEMORY_DIRECTORY }) await inMemoryNetlifyDB.start() try { - return { - status: 'success' as const, - successfullyApplied: await applyMigrationsWithDetails(inMemoryNetlifyDB, migrationsDirectory), - } + const applied = await applyMigrationsWithDetails(inMemoryNetlifyDB, migrationsDirectory) + return { status: 'success', applied } } catch (error) { - if (error instanceof MissingMigrationDirectoryError) { - return { - status: 'success' as const, - successfullyApplied: [], - } - } - - if (error instanceof DuplicateMigrationVersionsError) { - return { - status: 'duplicate_versions' as const, - conflicts: error.conflicts, - } - } - - if (error instanceof MigrationsApplyError) { - return { - status: 'dry_run_apply_error' as const, - appliedMigrations: error.appliedMigrations, - migrationCausingError: error.migrationCausingError, - remainingMigrations: error.remainingMigrations, - cause: error.cause, - } - } - - return { - status: 'error' as const, - error, + const issues = errorToIssues(error) + if (issues.length === 0) { + return { status: 'success', applied: [] } } + return { status: 'failure', issues } } finally { await inMemoryNetlifyDB.stop() } diff --git a/packages/dev/src/main.ts b/packages/dev/src/main.ts index 75deacd30..31c143ae9 100644 --- a/packages/dev/src/main.ts +++ b/packages/dev/src/main.ts @@ -24,8 +24,19 @@ import { RedirectsHandler } from '@netlify/redirects' import { StaticHandler } from '@netlify/static' import { NetlifyDB } from '@netlify/database-dev' -export { applyMigrations, initializeTrackingTable, resetDatabase, testMigrationsApply } from '@netlify/database-dev' -export type { SQLExecutor } from '@netlify/database-dev' +export { + applyMigrations, + initializeTrackingTable, + resetDatabase, + testMigrationsInEphemeralDatabase, +} from '@netlify/database-dev' +export type { + Migration, + MigrationIssue, + PgErrorDetails, + SQLExecutor, + TestMigrationsInEphemeralDatabaseResult, +} from '@netlify/database-dev' import { InjectedEnvironmentVariable, injectEnvVariables } from './lib/env.js' import { isDirectory, isFile } from './lib/fs.js' From fbfd8e906f105e4c2a2a2ef63aa69b667d7b1c2f Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 12:29:23 +0200 Subject: [PATCH 6/7] format --- .../dev/src/lib/testMigrationsInEphemeralDatabase.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts index 72aafe142..3eb848cb1 100644 --- a/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts +++ b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts @@ -168,7 +168,7 @@ test('Returns failure with apply-failure issue when a migration has a syntax err 'This may be a problem in the SQL of "0002_bad" itself (for example, a syntax error), ' + 'or in the cumulative database state left by previously applied migrations ' + '(for example, the migration tries to create an object that an earlier migration already created, ' + - "or references one that was never created). Postgres returned SQLSTATE 42601; look that up for common causes. " + + 'or references one that was never created). Postgres returned SQLSTATE 42601; look that up for common causes. ' + 'Resolve the issue in the failing migration or in the prior ones before deploying.', ) }) @@ -190,14 +190,12 @@ test('Returns failure with apply-failure issue when a migration creates a relati expect(issue.migration.name).toBe('0002_create_users_again') expect(issue.pgError.code).toBe('42P07') expect(issue.pgError.message).toBe('relation "users" already exists') - expect(issue.summary).toBe( - 'Migration "0002_create_users_again" failed to apply: relation "users" already exists', - ) + expect(issue.summary).toBe('Migration "0002_create_users_again" failed to apply: relation "users" already exists') expect(issue.remediation).toBe( 'This may be a problem in the SQL of "0002_create_users_again" itself (for example, a syntax error), ' + 'or in the cumulative database state left by previously applied migrations ' + '(for example, the migration tries to create an object that an earlier migration already created, ' + - "or references one that was never created). Postgres returned SQLSTATE 42P07; look that up for common causes. " + + 'or references one that was never created). Postgres returned SQLSTATE 42P07; look that up for common causes. ' + 'Resolve the issue in the failing migration or in the prior ones before deploying.', ) }) From 54b22458bbd3da09b99471fe5e0981e9cf68d824 Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 6 May 2026 12:52:25 +0200 Subject: [PATCH 7/7] adjust remediation that suggest editing to watch out for applied migrations --- packages/database/dev/src/lib/migrations.ts | 11 +++++++++-- .../lib/testMigrationsInEphemeralDatabase.test.ts | 14 +++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index daf37fc2d..77c8d6e56 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -134,7 +134,11 @@ const buildApplyFailureIssue = (options: { `or in the cumulative database state left by previously applied migrations ` + `(for example, the migration tries to create an object that an earlier migration already created, ` + `or references one that was never created).${codeNote} ` + - `Resolve the issue in the failing migration or in the prior ones before deploying.`, + `If the failing migration (or a prior one whose state is implicated) has not yet been applied to any database, ` + + `fix its SQL. If it has already been applied, you likely edited the file after the fact — applied migrations ` + + `are immutable, so restore it to its applied version (for example via version control or ` + + `\`netlify database migrations pull --force\`). If neither situation matches, this may indicate a divergence ` + + `between Postgres and the embedded engine used for this check — please file a bug.`, } } @@ -151,7 +155,10 @@ const buildMigrationFileIssue = (options: { migrationName: options.migrationName, sqlPath: options.sqlPath, summary: `Migration "${options.migrationName}" is missing its SQL file at ${options.sqlPath}.`, - remediation: `Create the file at ${options.sqlPath}, or remove the migration's directory if it isn't intended.`, + remediation: + `Create the file at ${options.sqlPath}, or remove the migration's directory if it isn't intended. ` + + `Only remove the directory if the migration has not yet been applied to any database — if it was applied, ` + + `restore the file from version control instead.`, cause: options.cause, } } diff --git a/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts index 3eb848cb1..73b8a4b1c 100644 --- a/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts +++ b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts @@ -139,7 +139,7 @@ test('Returns failure with migration-file-error (missing) when a migration direc /^Migration "0001_create_users" is missing its SQL file at .+0001_create_users[/\\]migration\.sql\.$/, ) expect(issue.remediation).toMatch( - /^Create the file at .+0001_create_users[/\\]migration\.sql, or remove the migration's directory if it isn't intended\.$/, + /^Create the file at .+0001_create_users[/\\]migration\.sql, or remove the migration's directory if it isn't intended\. Only remove the directory if the migration has not yet been applied to any database — if it was applied, restore the file from version control instead\.$/, ) }) @@ -169,7 +169,11 @@ test('Returns failure with apply-failure issue when a migration has a syntax err 'or in the cumulative database state left by previously applied migrations ' + '(for example, the migration tries to create an object that an earlier migration already created, ' + 'or references one that was never created). Postgres returned SQLSTATE 42601; look that up for common causes. ' + - 'Resolve the issue in the failing migration or in the prior ones before deploying.', + 'If the failing migration (or a prior one whose state is implicated) has not yet been applied to any database, ' + + 'fix its SQL. If it has already been applied, you likely edited the file after the fact — applied migrations ' + + 'are immutable, so restore it to its applied version (for example via version control or ' + + '`netlify database migrations pull --force`). If neither situation matches, this may indicate a divergence ' + + 'between Postgres and the embedded engine used for this check — please file a bug.', ) }) @@ -196,6 +200,10 @@ test('Returns failure with apply-failure issue when a migration creates a relati 'or in the cumulative database state left by previously applied migrations ' + '(for example, the migration tries to create an object that an earlier migration already created, ' + 'or references one that was never created). Postgres returned SQLSTATE 42P07; look that up for common causes. ' + - 'Resolve the issue in the failing migration or in the prior ones before deploying.', + 'If the failing migration (or a prior one whose state is implicated) has not yet been applied to any database, ' + + 'fix its SQL. If it has already been applied, you likely edited the file after the fact — applied migrations ' + + 'are immutable, so restore it to its applied version (for example via version control or ' + + '`netlify database migrations pull --force`). If neither situation matches, this may indicate a divergence ' + + 'between Postgres and the embedded engine used for this check — please file a bug.', ) })