fix(migrate): fix cases where migrate engine was not stopped after an…

… error (#8482)

* fix(migrate): fix cases where migrate engine was not stopped after an error

* more migrate stop

src/packages/client/src/__tests__/integration/__helpers__/migrateDb.ts

@@ -13,9 +13,11 @@ export async function migrateDb({
   await createDatabase(connectionString)
   const migrate = new Migrate(schemaPath)
 
-  await migrate.push({
-    force: true,
-  })
-
-  migrate.stop()
+  try {
+    await migrate.push({
+      force: true,
+    })
+  } finally {
+    migrate.stop()
+  }
 }

src/packages/client/src/utils/setupMSSQL.ts

@@ -34,5 +34,5 @@ export async function setupMSSQL(options: SetupParams): Promise<void> {
   const connection = await connectionPool.connect()
 
   await connection.query(schema)
-  void connection.close()
+  await connection.close()
 }

src/packages/migrate/package.json

@@ -74,7 +74,7 @@
     "format": "prettier --write .",
     "lint": "eslint --cache --fix --ext .ts .",
     "lint-ci": "eslint --ext .ts .",
-    "test": "jest --forceExit",
+    "test": "jest --verbose",
     "build": "tsc -d -p tsconfig.build.json",
     "prepublishOnly": "pnpm run build"
   },

src/packages/migrate/src/__tests__/MigrateDev.test.ts

@@ -228,7 +228,7 @@ describe('sqlite', () => {
       migrationDirList![0],
       'migration.sql',
     )
-    const migrationFile = await fs.read(migrationFilePath)
+    const migrationFile = fs.read(migrationFilePath)
     expect(migrationFile).toMatchInlineSnapshot(`
       -- CreateTable
       CREATE TABLE "Blog" (

src/packages/migrate/src/__tests__/MigrateResolve.test.ts

@@ -82,7 +82,12 @@ describe('sqlite', () => {
   it("--applied should fail if migration doesn't exist", async () => {
     ctx.fixture('existing-db-1-failed-migration')
     const result = MigrateResolve.new().parse(['--applied=does_not_exist'])
-    await expect(result).rejects.toThrowError()
+    await expect(result).rejects.toThrowErrorMatchingInlineSnapshot(`
+P3017
+
+The migration does_not_exist could not be found. Please make sure that the migration exists, and that you included the whole name of the directory. (example: "20201231000000_initial_migration")
+
+`)
   })
 
   it('--applied should fail if migration is already applied', async () => {

src/packages/migrate/src/commands/DbPush.ts

@@ -186,7 +186,13 @@ ${chalk.bold.redBright('All data will be lost.')}
         return ``
       }
 
-      await migrate.reset()
+      try {
+        await migrate.reset()
+      } catch (e) {
+        migrate.stop()
+        throw e
+      }
+
       if (dbInfo.dbName && dbInfo.dbLocation) {
         console.info(
           `The ${dbInfo.dbType} ${dbInfo.schemaWord} "${dbInfo.dbName}" from "${dbInfo.dbLocation}" was successfully reset.`,
@@ -233,9 +239,14 @@ ${chalk.bold.redBright('All data will be lost.')}
           return ``
         }
 
-        await migrate.push({
-          force: true,
-        })
+        try {
+          await migrate.push({
+            force: true,
+          })
+        } catch (e) {
+          migrate.stop()
+          throw e
+        }
       }
     }
 

src/packages/migrate/src/commands/MigrateDeploy.ts

@@ -136,10 +136,13 @@ ${editedMigrationNames.join('\n')}`,
       )
     }
 
-    const { appliedMigrationNames: migrationIds } =
-      await migrate.applyMigrations()
-
-    migrate.stop()
+    let migrationIds: string[]
+    try {
+      const { appliedMigrationNames } = await migrate.applyMigrations()
+      migrationIds = appliedMigrationNames
+    } finally {
+      migrate.stop()
+    }
 
     console.info() // empty line
     if (migrationIds.length === 0) {

src/packages/migrate/src/commands/MigrateDev.ts

@@ -31,14 +31,12 @@ import {
 } from '../utils/errors'
 import { printMigrationId } from '../utils/printMigrationId'
 import { printFilesFromMigrationIds } from '../utils/printFiles'
-import {
-  handleUnexecutableSteps,
-  handleWarnings,
-} from '../utils/handleEvaluateDataloss'
+import { handleUnexecutableSteps } from '../utils/handleEvaluateDataloss'
 import { getMigrationName } from '../utils/promptForMigrationName'
 import { throwUpgradeErrorIfOldMigrate } from '../utils/detectOldMigrate'
 import { printDatasource } from '../utils/printDatasource'
 import { tryToRunSeed, detectSeedFiles } from '../utils/seed'
+import { EngineResults } from '../types'
 
 const debug = Debug('prisma:migrate:dev')
 
@@ -146,15 +144,22 @@ ${chalk.bold('Examples')}
 
     const migrate = new Migrate(schemaPath)
 
-    const devDiagnostic = await migrate.devDiagnostic()
-    debug({ devDiagnostic: JSON.stringify(devDiagnostic, null, 2) })
+    let devDiagnostic: EngineResults.DevDiagnosticOutput
+    try {
+      devDiagnostic = await migrate.devDiagnostic()
+      debug({ devDiagnostic: JSON.stringify(devDiagnostic, null, 2) })
+    } catch (e) {
+      migrate.stop()
+      throw e
+    }
 
     const migrationIdsApplied: string[] = []
 
     if (devDiagnostic.action.tag === 'reset') {
       if (!args['--force']) {
         // We use prompts.inject() for testing in our CI
         if (isCi() && Boolean((prompt as any)._injected?.length) === false) {
+          migrate.stop()
           throw new MigrateDevEnvNonInteractiveError()
         }
 
@@ -167,27 +172,39 @@ ${chalk.bold('Examples')}
 
         if (!confirmedReset) {
           console.info('Reset cancelled.')
+          migrate.stop()
           process.exit(0)
           // For snapshot test, because exit() is mocked
           return ``
         }
       }
 
-      // Do the reset
-      await migrate.reset()
+      try {
+        // Do the reset
+        await migrate.reset()
+      } catch (e) {
+        migrate.stop()
+        throw e
+      }
     }
 
-    const { appliedMigrationNames } = await migrate.applyMigrations()
-    migrationIdsApplied.push(...appliedMigrationNames)
-    // Inform user about applied migrations now
-    if (appliedMigrationNames.length > 0) {
-      console.info(
-        `The following migration(s) have been applied:\n\n${chalk(
-          printFilesFromMigrationIds('migrations', appliedMigrationNames, {
-            'migration.sql': '',
-          }),
-        )}`,
-      )
+    try {
+      const { appliedMigrationNames } = await migrate.applyMigrations()
+      migrationIdsApplied.push(...appliedMigrationNames)
+
+      // Inform user about applied migrations now
+      if (appliedMigrationNames.length > 0) {
+        console.info(
+          `The following migration(s) have been applied:\n\n${chalk(
+            printFilesFromMigrationIds('migrations', appliedMigrationNames, {
+              'migration.sql': '',
+            }),
+          )}`,
+        )
+      }
+    } catch (e) {
+      migrate.stop()
+      throw e
     }
 
     // If database was reset we want to run the seed if not skipped
@@ -209,25 +226,58 @@ ${chalk.bold('Examples')}
       }
     }
 
-    const evaluateDataLossResult = await migrate.evaluateDataLoss()
-    debug({ evaluateDataLossResult })
+    let evaluateDataLossResult: EngineResults.EvaluateDataLossOutput
+    try {
+      evaluateDataLossResult = await migrate.evaluateDataLoss()
+      debug({ evaluateDataLossResult })
+    } catch (e) {
+      migrate.stop()
+      throw e
+    }
 
     // display unexecutableSteps
     // throws error if not create-only
-    handleUnexecutableSteps(
+    const unexecutableStepsError = handleUnexecutableSteps(
       evaluateDataLossResult.unexecutableSteps,
       args['--create-only'],
     )
+    if (unexecutableStepsError) {
+      migrate.stop()
+      throw new Error(unexecutableStepsError)
+    }
 
     // log warnings and prompt user to continue if needed
-    const userCancelled = await handleWarnings(
-      evaluateDataLossResult.warnings,
-      args['--force'],
-      args['--create-only'],
-    )
-    if (userCancelled) {
-      migrate.stop()
-      return `Migration cancelled.`
+    if (
+      evaluateDataLossResult.warnings &&
+      evaluateDataLossResult.warnings.length > 0
+    ) {
+      console.log(chalk.bold(`\n⚠️  Warnings for the current datasource:\n`))
+      for (const warning of evaluateDataLossResult.warnings) {
+        console.log(chalk(`  • ${warning.message}`))
+      }
+      console.info() // empty line
+
+      if (!args['--force']) {
+        // We use prompts.inject() for testing in our CI
+        if (isCi() && Boolean((prompt as any)._injected?.length) === false) {
+          migrate.stop()
+          throw new MigrateDevEnvNonInteractiveError()
+        }
+
+        const message = args['--create-only']
+          ? 'Are you sure you want create this migration?'
+          : 'Are you sure you want create and apply this migration?'
+        const confirmation = await prompt({
+          type: 'confirm',
+          name: 'value',
+          message,
+        })
+
+        if (!confirmation.value) {
+          migrate.stop()
+          return `Migration cancelled.`
+        }
+      }
     }
 
     let migrationName: undefined | string = undefined
@@ -242,30 +292,32 @@ ${chalk.bold('Examples')}
       }
     }
 
-    const createMigrationResult = await migrate.createMigration({
-      migrationsDirectoryPath: migrate.migrationsDirectoryPath,
-      migrationName: migrationName || '',
-      draft: args['--create-only'] ? true : false,
-      prismaSchema: migrate.getDatamodel(),
-    })
-    debug({ createMigrationResult })
+    let migrationIds: string[]
+    try {
+      const createMigrationResult = await migrate.createMigration({
+        migrationsDirectoryPath: migrate.migrationsDirectoryPath,
+        migrationName: migrationName || '',
+        draft: args['--create-only'] ? true : false,
+        prismaSchema: migrate.getDatamodel(),
+      })
+      debug({ createMigrationResult })
+
+      if (args['--create-only']) {
+        migrate.stop()
 
-    if (args['--create-only']) {
-      migrate.stop()
+        return `Prisma Migrate created the following migration without applying it ${printMigrationId(
+          createMigrationResult.generatedMigrationName!,
+        )}\n\nYou can now edit it and apply it by running ${chalk.greenBright(
+          getCommandWithExecutor('prisma migrate dev'),
+        )}.`
+      }
 
-      // console.info() // empty line
-      return `Prisma Migrate created the following migration without applying it ${printMigrationId(
-        createMigrationResult.generatedMigrationName!,
-      )}\n\nYou can now edit it and apply it by running ${chalk.greenBright(
-        getCommandWithExecutor('prisma migrate dev'),
-      )}.`
+      const { appliedMigrationNames } = await migrate.applyMigrations()
+      migrationIds = appliedMigrationNames
+    } finally {
+      migrate.stop()
     }
 
-    const { appliedMigrationNames: migrationIds } =
-      await migrate.applyMigrations()
-
-    migrate.stop()
-
     // For display only, empty line
     migrationIdsApplied.length > 0 && console.info()
 

src/packages/migrate/src/commands/MigrateReset.ts

@@ -137,11 +137,15 @@ ${chalk.bold('Examples')}
 
     const migrate = new Migrate(schemaPath)
 
-    await migrate.reset()
-
-    const { appliedMigrationNames: migrationIds } =
-      await migrate.applyMigrations()
-    migrate.stop()
+    let migrationIds: string[]
+    try {
+      await migrate.reset()
+
+      const { appliedMigrationNames } = await migrate.applyMigrations()
+      migrationIds = appliedMigrationNames
+    } finally {
+      migrate.stop()
+    }
 
     if (migrationIds.length === 0) {
       console.info(`${chalk.green('Database reset successful')}`)

src/packages/migrate/src/commands/MigrateResolve.ts

@@ -151,11 +151,13 @@ ${chalk.bold.green(
       await ensureCanConnectToDatabase(schemaPath)
 
       const migrate = new Migrate(schemaPath)
-
-      await migrate.markMigrationApplied({
-        migrationId: args['--applied'],
-      })
-      migrate.stop()
+      try {
+        await migrate.markMigrationApplied({
+          migrationId: args['--applied'],
+        })
+      } finally {
+        migrate.stop()
+      }
 
       return `Migration ${args['--applied']} marked as applied.`
     } else {
@@ -175,11 +177,13 @@ ${chalk.bold.green(
       await ensureCanConnectToDatabase(schemaPath)
 
       const migrate = new Migrate(schemaPath)
-
-      await migrate.markMigrationRolledBack({
-        migrationId: args['--rolled-back'],
-      })
-      migrate.stop()
+      try {
+        await migrate.markMigrationRolledBack({
+          migrationId: args['--rolled-back'],
+        })
+      } finally {
+        migrate.stop()
+      }
 
       return `Migration ${args['--rolled-back']} marked as rolled back.`
     }