Ethan-Xingyue opened a new issue, #1174: URL: https://github.com/apache/incubator-seata-go/issues/1174
# TCC Fence log cleaner ignores `db.Begin()` errors and panics on a nil transaction | Field | Value | | --- | --- | | Issue title | `[BUG] [TCC Fence] Log cleaner ignores db.Begin() errors and panics on a nil transaction, dropping the batch` | | Labels (exist in repo) | `bug`, `module/tcc` | | Suggested priority | P1 | | Related | none known | | Verification | Reproduced 2026-09-02 on master `3bf73586` with go1.24.3 darwin/arm64 (in-package test with go-sqlmock) | --- ## 🚀 Go Version go1.24.3 darwin/arm64 ## 📦 Seata-go Version master, commit 3bf73586af81db1bd428982c93d82000d80cb1c8 (fetched 2026-09-02) ## 💾 Operating System macOS ## 📝 Bug Description `tccFenceWrapperHandler.traversalCleanChannel` starts a database transaction with `tx, _ := db.Begin()` in two places and discards the error: https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/rm/tcc/fence/handler/tcc_fence_wrapper_handler.go#L319-L348 (L328 and L341) When the database is unavailable, `tx` is `nil`; the nil transaction is handed to the DAO and `tx.Commit()` (L333 / L346) is called on it, which panics with a nil pointer dereference and terminates the cleaner goroutine. When the delete or commit fails, the error is only logged and the current batch is dropped without rollback or retry. Other paths in the same file (`initLogCleanTask` L216, `drainCacheTask` L384) do check the `Begin` error, so the cleaner is the odd one out. Impact: a transient database outage can kill the background cleaner and silently lose pending fence-log identities, so fence logs accumulate. ## 🔄 Steps to Reproduce 1. Check out the commit: ```bash git clone https://github.com/apache/incubator-seata-go.git cd incubator-seata-go git checkout 3bf73586af81db1bd428982c93d82000d80cb1c8 ``` 2. Save the following as `pkg/rm/tcc/fence/handler/audit_fence_cleaner_repro_test.go` (it reuses `mockTCCFenceStore` from the existing package tests): ```go package handler import ( "errors" "testing" "github.com/DATA-DOG/go-sqlmock" "seata.apache.org/seata-go/v2/pkg/rm/tcc/fence/store/db/model" ) func TestAuditTraversalBeginErrorMustNotPanic(t *testing.T) { db, mock, err := sqlmock.New() if err != nil { t.Fatal(err) } defer db.Close() mock.ExpectBegin().WillReturnError(errors.New("database unavailable")) handler := &tccFenceWrapperHandler{ tccFenceDao: &mockTCCFenceStore{}, logQueue: make(chan *model.FenceLogIdentity, channelDelete), } for i := 0; i < channelDelete; i++ { handler.logQueue <- &model.FenceLogIdentity{Xid: "xid-1", BranchId: int64(i)} } close(handler.logQueue) defer func() { if r := recover(); r != nil { t.Fatalf("cleaner panicked after Begin failed: %v", r) } }() handler.traversalCleanChannel(db) } ``` 3. Run: ```bash go test -run '^TestAuditTraversalBeginErrorMustNotPanic$' -count=1 -v \ ./pkg/rm/tcc/fence/handler ``` Delete the temporary test file afterwards. ## ✅ Expected Behavior A `Begin` failure is logged and handled with a bounded retry or a clean exit. The cleaner does not panic, does not call the DAO or `Commit` on a nil transaction, and does not silently drop the batch. ## ❌ Actual Behavior ```text === RUN TestAuditTraversalBeginErrorMustNotPanic audit_fence_cleaner_repro_test.go:31: cleaner panicked after Begin failed: runtime error: invalid memory address or nil pointer dereference --- FAIL: TestAuditTraversalBeginErrorMustNotPanic (0.00s) FAIL FAIL seata.apache.org/seata-go/v2/pkg/rm/tcc/fence/handler 0.593s FAIL ``` ## 💡 Possible Solution - Reuse the retry/limit strategy already used by `drainCacheTask`; on Begin, delete or commit failure roll back (if begun) and keep or re-queue the batch. - Define the drain policy on shutdown so unprocessed identities are either drained or reported, never dropped silently. - Define ownership of the last partial batch when the channel is closed. Acceptance criteria: - [ ] The Begin-failure test above passes and asserts that the DAO is never called with a nil transaction. - [ ] Begin, delete, commit and rollback failures each have a test; no batch is lost silently. - [ ] Retries are bounded by count/time, with a metric and structured log when the bound is hit. - [ ] Normal shutdown drains as documented or reports the number of unprocessed identities. -- 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]
