fix(user-create): use throwing QueryUtils variants so rollback fires#37
Merged
Conversation
The transaction in `UserManager::create()` wrapped its inserts in `try/catch (\Throwable)` to roll back on failure, but routed every write through OpenEMR's `sqlInsert` / `sqlStatement` globals. Those helpers funnel SQL errors through `HelpfulDie()`, which calls `exit()` on most builds — so the catch never fires and the transaction is left in a half state, with a `users` row but no matching `users_secure` row (the bad state described in #8). Switch the writes inside the transaction to `QueryUtils::sqlInsert` / `QueryUtils::sqlStatementThrowException`, which raise `SqlQueryException` instead of exiting. The existing `catch (\Throwable)` then runs `sqlRollbackTrans()` and rethrows. `assignUuidIfPossible()` gets the same treatment so a SQL error during the optional UUID backfill doesn't bypass the outer rollback either — its existing `catch (\Throwable)` keeps that step non-fatal. Closes #8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UserManager::create()wrapped its inserts intry/catch (\Throwable)to roll back on failure, but called the globalsqlInsert/sqlStatementhelpers — which route SQL errors throughHelpfulDie()andexit()on most builds. The catch never fired and a failure mid-create could leave ausersrow without a matchingusers_securerow.QueryUtils::sqlInsert/QueryUtils::sqlStatementThrowException, which raiseSqlQueryExceptioninstead of exiting. The existing catch then runssqlRollbackTrans()and rethrows.SqlQueryExceptionand assertsbeginCount == 1,commitCount == 0,rollbackCount == 1.Closes #8.
Test plan
composer phpunit— 78 tests, 236 assertions, including the newcreateRollsBackWhenQueryUtilsThrowsSqlQueryExceptioncomposer phpcscomposer phpstan(level 9, clean)composer rector(dry run, clean)