knyk-dev opened a new issue, #1082:
URL: https://github.com/apache/incubator-seata-go/issues/1082

   ### ✅ Verification Checklist
   
   - [x] 🔍 I have searched the [existing 
issues](https://github.com/apache/incubator-seata-go/issues) and confirmed this 
is not a duplicate
   - [x] 🛠️ I am willing to try to fix this bug myself.
   
   ### 🚀 Go Version
   
   1.20.14
   
   ### 📦 Seata-go Version
   
   master
   
   ### 💾 Operating System
   
   🪟 Windows
   
   ### 📝 Bug Description
   
   In the AT auto-commit path, `createTxAndExecIfNeeded()` and 
`createTxAndQueryIfNeeded()` create a local transaction with `BeginTx()` when a 
global transaction is present.
   
   If `f()` returns an error, both helpers return immediately without calling 
`Rollback()`. They only roll back in the panic path.
   
   This leaves the physical connection with an unfinished local transaction. A 
later request may reuse that connection and inherit the dirty transaction 
state. The same bug already existed in the Exec path, and the new Query helper 
copied the same behavior into the query path.
   
   ### 🔄 Steps to Reproduce
   
   1. Start from `master`.
   2. Trigger the AT auto-commit path so that `BeginTx()` succeeds.
   3. Make the wrapped execution function return an error.
   4. Observe that the created local transaction is not rolled back before 
returning.
   
   A minimal unit test can reproduce it by asserting that `Rollback()` is 
called after `createTxAndExecIfNeeded()` or `createTxAndQueryIfNeeded()` 
returns an error.
   
   
   ### ✅ Expected Behavior
   
   If the helper created a local transaction and `f()` returns an error, the 
helper should roll back that transaction before returning the error.
   
   This should be true for both the Exec helper and the Query helper.
   
   ### ❌ Actual Behavior
   
   The helper returns the execution error directly and leaves the created 
transaction open.
   
   The regression test below fails on `master` with `go test -race 
./pkg/datasource/sql -run TestATConn_CreateTxHelpersRollbackOnError -count=1 
-v`:
   
   ```text
   === RUN   TestATConn_CreateTxHelpersRollbackOnError
   === RUN   
TestATConn_CreateTxHelpersRollbackOnError/createTxAndExecIfNeeded_rolls_back_created_tx_on_error
       conn_at_test.go:292:
                   Error Trace:    
F:/tmp/seata-at-auto-tx-rollback-repro/pkg/datasource/sql/conn_at_test.go:292
                   Error:          Not equal:
                                   expected: 1
                                   actual  : 0
                   Test:           
TestATConn_CreateTxHelpersRollbackOnError/createTxAndExecIfNeeded_rolls_back_created_tx_on_error
       controller.go:269: missing call(s) to *mock.MockTestDriverTx.Rollback() 
F:/tmp/seata-at-auto-tx-rollback-repro/pkg/datasource/sql/conn_at_test.go:256
       controller.go:269: aborting test due to missing call(s)
   === RUN   
TestATConn_CreateTxHelpersRollbackOnError/createTxAndQueryIfNeeded_rolls_back_created_tx_on_error
       conn_at_test.go:338:
                   Error Trace:    
F:/tmp/seata-at-auto-tx-rollback-repro/pkg/datasource/sql/conn_at_test.go:338
                   Error:          Not equal:
                                   expected: 1
                                   actual  : 0
                   Test:           
TestATConn_CreateTxHelpersRollbackOnError/createTxAndQueryIfNeeded_rolls_back_created_tx_on_error
       controller.go:269: missing call(s) to *mock.MockTestDriverTx.Rollback() 
F:/tmp/seata-at-auto-tx-rollback-repro/pkg/datasource/sql/conn_at_test.go:302
       controller.go:269: aborting test due to missing call(s)
   --- FAIL: TestATConn_CreateTxHelpersRollbackOnError (0.00s)
       --- FAIL: 
TestATConn_CreateTxHelpersRollbackOnError/createTxAndExecIfNeeded_rolls_back_created_tx_on_error
 (0.00s)
       --- FAIL: 
TestATConn_CreateTxHelpersRollbackOnError/createTxAndQueryIfNeeded_rolls_back_created_tx_on_error
 (0.00s)
   FAIL
   FAIL    seata.apache.org/seata-go/v2/pkg/datasource/sql    5.556s
   FAIL
   ```
   
   
   ### 💡 Possible Solution
   
   If a helper created the local transaction and `f()` returns an error, call 
`Rollback()` before returning the error.
   
   If `Rollback()` also fails, return both errors together.
   
   There should also be regression tests for both helper paths:
   - `createTxAndExecIfNeeded()` should roll back the created transaction on 
error
   - `createTxAndQueryIfNeeded()` should roll back the created transaction on 
error
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to