Ethan-Xingyue opened a new issue, #1153:
URL: https://github.com/apache/incubator-seata-go/issues/1153
| Field | Value |
| --- | --- |
| Issue title | `[BUG] [TM][Getty/gRPC] Canceled context makes Commit return
nil and Rollback panic` |
| Labels (exist in repo) | `bug`, `module/tm`, `remoting` |
| Suggested priority | P0 (release blocker candidate) |
| Related | none known |
| Verification | Reproduced 2026-09-02 on master `3bf73586` with go1.24.3
darwin/arm64 |
---
## 🚀 Go Version
go1.24.3 darwin/arm64
## 📦 Seata-go Version
master, commit 3bf73586af81db1bd428982c93d82000d80cb1c8 (fetched 2026-09-02)
## 💾 Operating System
macOS
## 📝 Bug Description
In both TM implementations (Getty and gRPC), if the caller's
`context.Context` is already canceled when `Commit` or `Rollback` starts, no
request is sent to the TC, but:
- `Commit` returns `nil`, so the caller believes the global transaction was
committed.
- `Rollback` panics with a nil interface conversion.
The second phase runs inside the deferred function of `tm.WithGlobalTx`, so
the rollback panic propagates to the caller and, if not recovered, terminates
the client process.
**Root cause (from reading the code):**
1. When `ctx` is already canceled, `bf.Ongoing()` is false on the first
iteration, so the send loop never runs and `err` stays `nil`.
2. `Commit` uses `if err != nil || bf.Err() != nil { lastErr :=
errors.Wrap(err, bf.Err().Error()); ...; return lastErr }`. `errors.Wrap(nil,
...)` from `github.com/pkg/errors` returns `nil`, so the "request was never
sent" case is reported as success.
3. `Rollback` uses `if err != nil && bf.Err() != nil`, which skips the case
where only `bf.Err()` is set, then runs the type assertion
`res.(message.GlobalRollbackResponse)` (Getty) /
`res.(*pb.GlobalRollbackResponseProto)` (gRPC) on a `nil` `res` and panics.
Locations at 3bf73586:
- Getty Commit:
https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/tm/transaction/getty/getty_global_transaction.go#L94-L101
- Getty Rollback:
https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/tm/transaction/getty/getty_global_transaction.go#L136-L143
- gRPC Commit:
https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/tm/transaction/grpc/grpc_global_transaction.go#L105-L112
- gRPC Rollback:
https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/tm/transaction/grpc/grpc_global_transaction.go#L152-L159
Suggested priority: P0. A false "committed" result breaks the caller's view
of the final transaction state, and the rollback panic can crash the process.
## 🔄 Steps to Reproduce
1. Check out the commit and create a throwaway module that points at the
local checkout:
```bash
git clone https://github.com/apache/incubator-seata-go.git
cd incubator-seata-go
git checkout 3bf73586af81db1bd428982c93d82000d80cb1c8
repo_root="$(pwd)"
repro_dir="$(mktemp -d)"
cd "$repro_dir"
go mod init seata-repro
go mod edit -require=seata.apache.org/seata-go/[email protected]
go mod edit -replace=seata.apache.org/seata-go/v2="$repo_root"
```
2. Save the following as `repro_test.go` in `$repro_dir`:
```go
package repro
import (
"context"
"errors"
"testing"
"seata.apache.org/seata-go/v2/pkg/tm"
gettytm "seata.apache.org/seata-go/v2/pkg/tm/transaction/getty"
grpctm "seata.apache.org/seata-go/v2/pkg/tm/transaction/grpc"
)
func TestCanceledContextEndPhase(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
managers := []struct {
name string
m tm.GlobalTransactionManager
}{
{"getty", &gettytm.GettyGlobalTransactionManager{}},
{"grpc", &grpctm.GrpcGlobalTransactionManager{}},
}
for _, tc := range managers {
t.Run(tc.name+"_commit", func(t *testing.T) {
tx := &tm.GlobalTransaction{Xid: "xid-1", TxRole:
tm.Launcher}
if err := tc.m.Commit(ctx, tx); !errors.Is(err,
context.Canceled) {
t.Fatalf("Commit error=%v, want
context.Canceled", err)
}
})
t.Run(tc.name+"_rollback", func(t *testing.T) {
defer func() {
if r := recover(); r != nil {
t.Fatalf("Rollback panicked: %v", r)
}
}()
tx := &tm.GlobalTransaction{Xid: "xid-1", TxRole:
tm.Launcher}
if err := tc.m.Rollback(ctx, tx); !errors.Is(err,
context.Canceled) {
t.Fatalf("Rollback error=%v, want
context.Canceled", err)
}
})
}
}
```
3. Run:
```bash
go mod tidy
go test -run '^TestCanceledContextEndPhase$' -count=1 -v
```
## ✅ Expected Behavior
- Neither `Commit` nor `Rollback` sends an RPC, neither panics, and both
return a non-nil error for which `errors.Is(err, context.Canceled)` is true.
- Getty and gRPC behave identically (same error classification and retry
semantics).
## ❌ Actual Behavior
```text
=== RUN TestCanceledContextEndPhase
=== RUN TestCanceledContextEndPhase/getty_commit
repro_test.go:28: Commit error=<nil>, want context.Canceled
=== RUN TestCanceledContextEndPhase/getty_rollback
repro_test.go:34: Rollback panicked: interface conversion: interface {}
is nil, not message.GlobalRollbackResponse
=== RUN TestCanceledContextEndPhase/grpc_commit
repro_test.go:28: Commit error=<nil>, want context.Canceled
=== RUN TestCanceledContextEndPhase/grpc_rollback
repro_test.go:34: Rollback panicked: interface conversion: interface {}
is nil, not *pb.GlobalRollbackResponseProto
--- FAIL: TestCanceledContextEndPhase (0.00s)
--- FAIL: TestCanceledContextEndPhase/getty_commit (0.00s)
--- FAIL: TestCanceledContextEndPhase/getty_rollback (0.00s)
--- FAIL: TestCanceledContextEndPhase/grpc_commit (0.00s)
--- FAIL: TestCanceledContextEndPhase/grpc_rollback (0.00s)
FAIL
FAIL seata-audit-repros/tx001 1.427s
FAIL
```
## 💡 Possible Solution
- Extract one end-phase result normalization helper shared by Getty and gRPC
that handles, in order: `ctx.Err()`, the last RPC error, retries exhausted,
`nil` response, and unexpected response type.
- Do not only change the rollback condition from `&&` to `||`: `Commit`
would still return `nil` because of `errors.Wrap(nil, ...)`.
- Add table-driven conformance tests that run the same cases against both
transports: canceled before the call, canceled during retries, retries
exhausted, `nil` response, wrong response type, success.
Acceptance criteria:
- [ ] The test above fails before the fix and passes after it.
- [ ] Getty and gRPC share the same table-driven cases.
- [ ] Cancel-before-call, cancel-during-retry, retries-exhausted,
nil-response and wrong-response-type all have regression tests.
- [ ] None of these inputs panics, returns a false success, or leaves a
future behind.
--
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]
