Migrator: migrateIndividual() error path falls through to transaction commit after rollback #2811

Summary

Migrator.migrateIndividual() in vendor/wheels/Migrator.cfc:225-248 issues transaction action=\"commit\" unconditionally after the try/catch — including on the error path, where the catch block has already issued transaction action=\"rollback\". Unlike the loops in migrateTo(), this function has no enclosing for to break out of, so control always falls through.

Repro

// vendor/wheels/Migrator.cfc:225-248 (current shape, post-#2810)
transaction {
    try {
        // ... up() runs ...
        local.migration.cfc.up();
        // ...
        $setVersionAsMigrated(...);
    } catch (any e) {
        local.rv = local.rv & \"Error migrating ...\";
        transaction action=\"rollback\";          // error path issues rollback
        StructDelete(request, \"\$wheelsTransactionWrapper\");
    }
    StructDelete(request, \"\$wheelsTransactionWrapper\");
    transaction action=\"commit\";                // ALWAYS runs — fires after rollback on error
}

In migrateTo() the same pattern is safe because the catch ends with break;, exiting the surrounding for loop and skipping the transaction action=\"commit\". migrateIndividual() has no surrounding loop, so the break is missing and the commit always runs.

Diagnosis

Surfaced during the round-1 Reviewer A review of #2810:

Unlike the migrateTo() loops, migrateIndividual() has no break in the catch, so control always falls through to transaction action=\"commit\" even on the error path. The new StructDelete placement is correct (flag cleared in both paths). The commit-after-rollback is pre-existing behaviour, but it's worth a follow-up issue since the fix passes through this function.

Reviewer B's round-1 confirmation:

Pre-existing commit-after-rollback in migrateIndividual() — confirmed. transaction action=\"commit\" at ~line 248 executes unconditionally in both the success and error paths; the error path already issued transaction action=\"rollback\". The new StructDelete lines are in the correct positions; the commit-after-rollback predates this PR.

Both reviewers explicitly recommended a dedicated follow-up issue rather than expanding #2810's scope.

Behavior across engines

Issuing transaction action=\"commit\" after transaction action=\"rollback\" in CFML is engine-defined. On Lucee the second action against a rolled-back transaction is a no-op (the transaction is already closed). On Adobe CF / BoxLang the behavior may be the same, or it may throw a "transaction not active" error depending on JDBC driver state. Worst case is a JDBC error masking the real migration failure with a transaction-state exception, making the error harder to diagnose.

Suggested fix

Add an explicit return / re-throw / status flag after the rollback so the subsequent commit doesn't run. The minimal patch mirrors migrateTo()'s pattern but uses an early return since there's no loop to break from:

transaction {
    try {
        // ... up() runs ...
    } catch (any e) {
        local.rv = local.rv & \"Error migrating ...\";
        transaction action=\"rollback\";
        StructDelete(request, \"\$wheelsTransactionWrapper\");
        return local.rv;                        // skip the commit below
    }
    StructDelete(request, \"\$wheelsTransactionWrapper\");
    transaction action=\"commit\";
}
return local.rv;

Or, equivalently, gate the commit on a success flag set in the try.

A regression spec under vendor/wheels/tests/specs/migrator/ should construct a synthetic migration whose up() throws, run migrateIndividual() against it, and assert (a) no exception escapes, (b) the returned rv string includes the error message, (c) wheels_migrator_versions does NOT have a row for that version (the rollback held).

Where found

Reviewer A and Reviewer B convergence on PR #2810. Both reviewers labelled this pre-existing and out of scope for #2810; capturing the diagnosis here for a follow-up PR.

Repo / version

• Wheels Core: develop (post-fix(model): bypass nested cftransaction when migrator owns the outer (#2789) #2810)
• File: vendor/wheels/Migrator.cfc:225-248 (migrateIndividual())
• Adapter: engine-agnostic at the CFML level; per-adapter JDBC behavior may vary