feat(up): improve error handling and presentation

.changeset/angry-chicken-thank.md

@@ -0,0 +1,5 @@
+---
+'@emigrate/cli': patch
+---
+
+Return a non zero exit code in case a migration fails (or for a dry-run if there's a failed migration in the history)

.changeset/cold-points-rule.md

@@ -0,0 +1,5 @@
+---
+'@emigrate/cli': minor
+---
+
+Show any failed migration from the history in the "up" dry-run output

.changeset/giant-files-serve.md

@@ -0,0 +1,5 @@
+---
+'@emigrate/cli': patch
+---
+
+Don't pass the EmigrateError instance to the storage for each failed migration but only the real cause. This is so that errors from failed migrations are not wrapped twice in EmigrateError instances when presenting failed migrations during an "up" dry-run or the "list" command.

.changeset/happy-yaks-impress.md

@@ -0,0 +1,5 @@
+---
+'@emigrate/cli': minor
+---
+
+Improve the looks of the "up" dry-run default output by showing pending migrations in a different color

packages/cli/src/commands/up.ts

@@ -1,3 +1,4 @@
+import path from 'node:path';
 import process from 'node:process';
 import { getOrLoadPlugins, getOrLoadReporter, getOrLoadStorage } from '@emigrate/plugin-tools';
 import {
@@ -61,19 +62,30 @@ export default async function upCommand({
   await reporter.onInit?.({ command: 'up', cwd, dry, directory });
 
   const migrationFiles = await getMigrations(cwd, directory);
-
-  let migrationHistoryError: MigrationHistoryError | undefined;
+  const failedEntries: MigrationMetadataFinished[] = [];
 
   for await (const migrationHistoryEntry of storage.getHistory()) {
+    const index = migrationFiles.findIndex((migrationFile) => migrationFile.name === migrationHistoryEntry.name);
+
     if (migrationHistoryEntry.status === 'failed') {
-      migrationHistoryError = new MigrationHistoryError(
+      const filePath = path.resolve(cwd, directory, migrationHistoryEntry.name);
+      const finishedMigration: MigrationMetadataFinished = {
+        name: migrationHistoryEntry.name,
+        status: migrationHistoryEntry.status,
+        filePath,
+        relativeFilePath: path.relative(cwd, filePath),
+        extension: withLeadingPeriod(path.extname(migrationHistoryEntry.name)),
+        error: new MigrationHistoryError(
           `Migration ${migrationHistoryEntry.name} is in a failed state, please fix it first`,
           migrationHistoryEntry,
-      );
+        ),
+        directory,
+        cwd,
+        duration: 0,
+      };
+      failedEntries.push(finishedMigration);
     }
 
-    const index = migrationFiles.findIndex((migrationFile) => migrationFile.name === migrationHistoryEntry.name);
-
     if (index !== -1) {
       migrationFiles.splice(index, 1);
     }
@@ -100,22 +112,29 @@ export default async function upCommand({
     }
   }
 
-  await reporter.onCollectedMigrations?.(migrationFiles);
+  await reporter.onCollectedMigrations?.([...failedEntries, ...migrationFiles]);
 
-  if (migrationFiles.length === 0 || dry || migrationHistoryError) {
+  if (migrationFiles.length === 0 || dry || failedEntries.length > 0) {
+    const error = failedEntries.find((migration) => migration.status === 'failed')?.error;
     await reporter.onLockedMigrations?.(migrationFiles);
 
     const finishedMigrations: MigrationMetadataFinished[] = migrationFiles.map((migration) => ({
       ...migration,
       duration: 0,
-      status: 'skipped',
+      status: 'pending',
     }));
 
+    for await (const failedMigration of failedEntries) {
+      await reporter.onMigrationError?.(failedMigration, failedMigration.error!);
+    }
+
     for await (const migration of finishedMigrations) {
       await reporter.onMigrationSkip?.(migration);
     }
 
-    await reporter.onFinished?.(finishedMigrations, migrationHistoryError);
+    await reporter.onFinished?.([...failedEntries, ...finishedMigrations], error);
+
+    process.exitCode = failedEntries.length > 0 ? 1 : 0;
     return;
   }
 
@@ -197,14 +216,7 @@ export default async function upCommand({
 
         finishedMigrations.push(finishedMigration);
       } catch (error) {
-        let errorInstance = error instanceof Error ? error : new Error(String(error));
-
-        if (!(errorInstance instanceof EmigrateError)) {
-          errorInstance = new MigrationRunError(`Failed to run migration: ${migration.relativeFilePath}`, migration, {
-            cause: error,
-          });
-        }
-
+        const errorInstance = error instanceof Error ? error : new Error(String(error));
         const duration = getDuration(start);
         const finishedMigration: MigrationMetadataFinished = {
           ...migration,
@@ -220,9 +232,19 @@ export default async function upCommand({
       }
     }
   } finally {
-    const firstError = finishedMigrations.find((migration) => migration.status === 'failed')?.error;
+    const firstFailed = finishedMigrations.find((migration) => migration.status === 'failed');
+    const firstError =
+      firstFailed?.error instanceof EmigrateError
+        ? firstFailed.error
+        : firstFailed
+          ? new MigrationRunError(`Failed to run migration: ${firstFailed.relativeFilePath}`, firstFailed, {
+              cause: firstFailed?.error,
+            })
+          : undefined;
 
     await cleanup();
     await reporter.onFinished?.(finishedMigrations, firstError);
+
+    process.exitCode = firstError ? 1 : 0;
   }
 }

packages/cli/src/reporters/default.ts

@@ -207,7 +207,10 @@ const getSummary = (
   return `  ${statusLine}${showTotal ? gray(` (${total} total)`) : ''}`;
 };
 
-const getHeaderMessage = (migrations?: MigrationMetadata[], lockedMigrations?: MigrationMetadata[]) => {
+const getHeaderMessage = (
+  migrations?: Array<MigrationMetadata | MigrationMetadataFinished>,
+  lockedMigrations?: Array<MigrationMetadata | MigrationMetadataFinished>,
+) => {
   if (!migrations || !lockedMigrations) {
     return '';
   }
@@ -220,13 +223,22 @@ const getHeaderMessage = (migrations?: MigrationMetadata[], lockedMigrations?: M
     return `  ${bold(migrations.length.toString())} ${dim('pending migrations to run')}`;
   }
 
-  if (lockedMigrations.length === 0) {
-    return `  ${bold(`0 of ${migrations.length}`)} ${dim('pending migrations to run')} ${redBright('(all locked)')}`;
-  }
+  const nonLockedMigrations = migrations.filter(
+    (migration) => !lockedMigrations.some((lockedMigration) => lockedMigration.name === migration.name),
+  );
+  const failedMigrations = nonLockedMigrations.filter(
+    (migration) => 'status' in migration && migration.status === 'failed',
+  );
+  const unlockableCount = nonLockedMigrations.length - failedMigrations.length;
 
-  return `  ${bold(`${lockedMigrations.length} of ${migrations.length}`)} ${dim('pending migrations to run')} ${yellow(
-    `(${migrations.length - lockedMigrations.length} locked)`,
-  )}`;
+  const parts = [
+    bold(`${lockedMigrations.length} of ${migrations.length}`),
+    dim`pending migrations to run`,
+    unlockableCount > 0 ? yellow(`(${unlockableCount} locked)`) : '',
+    failedMigrations.length > 0 ? redBright(`(${failedMigrations.length} failed)`) : '',
+  ].filter(Boolean);
+
+  return `  ${parts.join(' ')}`;
 };
 
 class DefaultFancyReporter implements Required<EmigrateReporter> {