Skip to content

fix(migrate): surface lost SQL migration finalizer errors (#1389) - #1390

Merged
vmihailenco merged 1 commit into
uptrace:masterfrom
Yusufihsangorgel:fix/migrate-finalizer-errors
Jul 17, 2026
Merged

fix(migrate): surface lost SQL migration finalizer errors (#1389)#1390
vmihailenco merged 1 commit into
uptrace:masterfrom
Yusufihsangorgel:fix/migrate-finalizer-errors

Conversation

@Yusufihsangorgel

Copy link
Copy Markdown
Contributor

Fixes #1389.

SQL migrations run via an unnamed return, so the finalizer's error (tx.Commit / tx.Rollback / bun.Conn.Close) was assigned to a local retErr inside the deferred function after return had already evaluated the result. In Go, the return expression is copied to the (unnamed) result before deferred functions run, so those finalizer errors were silently dropped:

  • On a successful .tx.up.sql, a Commit error was lost. With WithMarkAppliedOnSuccess(true) the migrator could then mark the migration applied even though the commit's outcome was uncertain.
  • On failure, the Rollback error was lost.
  • For non-transactional SQL migrations, a Conn.Close error was lost.

This switches the migration closure to a named return and assigns the finalizer error to it. The execution error keeps priority: when the body fails, its error is preserved and joined with any rollback/close error via errors.Join.

Tests (migrate/migration_finalizer_test.go) use a small database/sql driver whose transactions fail to commit (and optionally fail exec):

  • a Commit error is now returned from the migration (previously nil)
  • an execution error is still returned when the body fails

@Yusufihsangorgel
Yusufihsangorgel force-pushed the fix/migrate-finalizer-errors branch from a734787 to 7084b1f Compare July 14, 2026 11:37
@vmihailenco
vmihailenco merged commit 1a289f8 into uptrace:master Jul 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

migrate: SQL migration finalizer errors are lost due to unnamed return

2 participants