migrate() inside $transaction resolves after logged COMMIT, but changes are rolled back

[IlyaSemenov]

Summary

When migrate() is called inside an outer Orchid $transaction, the outer transaction can resolve successfully and Orchid logs COMMIT, but the changes made by migrate() are rolled back.

This happens in a normal user-land migration scenario: the migrations table does not exist yet, so migrate() first hits the expected relation does not exist SQL error, catches it internally, creates the migrations table, and applies the migration. The tables are visible inside the outer transaction, but disappear after the outer transaction resolves.

Environment

• orchid-orm: 1.68.6
• postgres: 3.4.8

Expected Behavior

After the outer $transaction resolves successfully, all tables created by migrate() should exist.

Actual Behavior

Inside the outer transaction both tables exist, but after the transaction resolves they are gone. For example, logs from the first reproduction script:

migration table exists inside transaction: true
migrated table exists inside transaction: true
COMMIT
migration table exists after transaction: false (expected: true)
migrated table exists after transaction: false (expected: true)

The COMMIT log is misleading here: the outer transaction resolves without throwing, but the database state is actually rolled back.

Notes

The likely trigger is the expected SQL error when migrate() probes the missing migrations table. migrate() handles this error internally, but postgres.js still appears to mark the transaction as failed. Orchid then logs COMMIT and resolves the outer transaction even though the transaction was rolled back.

Full reproduction (user-land problem example with migrate-in-transaction)

import process from "node:process"

import { createBaseTable } from "orchid-orm"
import { createMigrationChangeFn, migrate } from "orchid-orm/migrations"
import { orchidORM } from "orchid-orm/postgres-js"

const BaseTable = createBaseTable({
  snakeCase: true,
})

const change = createMigrationChangeFn({
  columnTypes: BaseTable.columnTypes,
})

const migrations = {
  "0001_repro_user": async () => ({
    default: change(async (db) => {
      await db.createTable("repro_user", t => ({
        id: t.serial().primaryKey(),
        name: t.varchar(),
      }))
    }),
  }),
}

const db = orchidORM({ log: true }, {})

async function tableExists(name: string) {
  return await db.$query.get<boolean>`select to_regclass(${name}) is not null`
}

async function main() {
  await db.$query`drop table if exists repro_user`
  await db.$query`drop table if exists repro_rake_migration`

  await db.$transaction(async () => {
    // migrate() first queries the missing migrations table and catches that SQL error internally.
    // postgres.js still marks the transaction as failed, while Orchid logs COMMIT and resolves the outer transaction.
    await migrate(db, {
      basePath: import.meta.url,
      migrationsTable: "repro_rake_migration",
      baseTable: BaseTable,
      migrations,
    })

    console.log(`migration table exists inside transaction: ${await tableExists("repro_rake_migration")}`)
    console.log(`migrated table exists inside transaction: ${await tableExists("repro_user")}`)
  })

  console.log(`migration table exists after transaction: ${await tableExists("repro_rake_migration")} (expected: true)`)
  console.log(`migrated table exists after transaction: ${await tableExists("repro_user")} (expected: true)`)

  await db.$query`drop table if exists repro_user`
  await db.$query`drop table if exists repro_rake_migration`
  await db.$close()
}

main().catch(async (err: unknown) => {
  console.error(err)
  await db.$query`drop table if exists repro_user`.catch(() => {})
  await db.$query`drop table if exists repro_rake_migration`.catch(() => {})
  await db.$close()
  process.exit(1)
})

In fact, the problem is broader than just handling migrations, the more generic reproduction (which I also think worth fixing) is:

Full reproduction (lower-level example showing the caught SQL error path)

import process from "node:process"

import { orchidORM } from "orchid-orm/postgres-js"

const db = orchidORM({ log: true }, {})

async function countRows() {
  return Number(await db.$query.get`select count(*) from repro_item`)
}

async function main() {
  await db.$query`drop table if exists repro_item`
  await db.$query`create table repro_item (id serial primary key, name text not null)`

  await db.$transaction(async () => {
    await db.$query`insert into repro_item (name) values ('created inside outer transaction')`
    console.log(`inside outer transaction before nested error: ${await countRows()}`)

    const adapter = db.$qb.$getAdapter()

    // The nested transaction catches the missing-table SQL error, but postgres.js still marks
    // the outer transaction as failed while Orchid logs COMMIT and resolves the transaction.
    await adapter.transaction(undefined, undefined, async (trx) => {
      await trx.query("select * from table_that_does_not_exist")
    }).catch((err: unknown) => {
      console.log("caught nested transaction error:", (err as { code?: string }).code)
    })

    console.log(`inside outer transaction after caught nested error: ${await countRows()}`)
  })

  console.log(`after outer transaction resolved: ${await countRows()} (expected: 1)`)

  await db.$query`drop table repro_item`
  await db.$close()
}

main().catch(async (err: unknown) => {
  console.error(err)
  await db.$query`drop table if exists repro_item`.catch(() => {})
  await db.$close()
  process.exit(1)
})

[romeerez]

I published a fix.

Thank you for posting the reproduction for postgres-js adapter itself - it did help to fix it.

const adapter = db.$qb.$getAdapter()

await adapter.transaction(undefined

Here the undefined stands for the async local storage so the adapter cannot get the current transaction here, so this reproduction won't work anyway, yet the bug existed and I fixed it.

To properly reproduce and verify I replaced that bit with db.$transaction:

await db.$transaction(...
  await db.$query`...`

Both reported problems - a rake-db migration and the driver issue - are fixed.