EFF-744 Fix sqlite migrator lock errors being silently swallowed (#1829)

Author: tim-smart

.changeset/eff-744-sqlite-migrator-lock.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Fix sql migrator lock handling to only treat duplicate migration-row inserts as a concurrent migration lock.

packages/effect/src/unstable/sql/Migrator.ts

@@ -236,11 +236,13 @@ export const make = <RD = never>({
       if (required.length > 0) {
         yield* pipe(
           insertMigrations(required.map(([id, name]) => [id, name])),
-          Effect.mapError((_) =>
-            new MigrationError({
-              kind: "Locked",
-              message: "Migrations already running"
-            })
+          Effect.mapError((error): MigrationError | SqlError =>
+            error.reason._tag === "ConstraintError"
+              ? new MigrationError({
+                kind: "Locked",
+                message: "Migrations already running"
+              })
+              : error
           )
         )
       }

packages/sql/sqlite-node/test/SqliteMigrator.test.ts

@@ -0,0 +1,48 @@
+import { NodeFileSystem } from "@effect/platform-node"
+import { SqliteClient, SqliteMigrator } from "@effect/sql-sqlite-node"
+import { assert, describe, it } from "@effect/vitest"
+import { Effect, FileSystem } from "effect"
+import { Reactivity } from "effect/unstable/reactivity"
+import * as SqlClient from "effect/unstable/sql/SqlClient"
+import * as SqlError from "effect/unstable/sql/SqlError"
+
+const makeClients = Effect.gen(function*() {
+  const fs = yield* FileSystem.FileSystem
+  const dir = yield* fs.makeTempDirectoryScoped()
+  const filename = dir + "/test.db"
+
+  return {
+    lockClient: yield* SqliteClient.make({ filename }),
+    migratorClient: yield* SqliteClient.make({ filename })
+  }
+}).pipe(Effect.provide([NodeFileSystem.layer, Reactivity.layer]))
+
+describe("SqliteMigrator", () => {
+  it.effect("fails on lock errors", () =>
+    Effect.gen(function*() {
+      const { lockClient, migratorClient } = yield* makeClients
+
+      yield* migratorClient`PRAGMA busy_timeout = 1`
+
+      yield* SqliteMigrator.run({
+        loader: SqliteMigrator.fromRecord({})
+      }).pipe(Effect.provideService(SqlClient.SqlClient, migratorClient))
+
+      yield* Effect.acquireRelease(
+        lockClient`BEGIN IMMEDIATE`,
+        () => lockClient`ROLLBACK`.pipe(Effect.ignore)
+      )
+
+      const error = yield* Effect.flip(
+        SqliteMigrator.run({
+          loader: SqliteMigrator.fromRecord({
+            "1_test": Effect.void
+          })
+        }).pipe(Effect.provideService(SqlClient.SqlClient, migratorClient))
+      )
+
+      assert.strictEqual(error._tag, "SqlError")
+      assert(SqlError.isSqlError(error))
+      assert.strictEqual(error.reason._tag, "LockTimeoutError")
+    }))
+})