Copilot commented on code in PR #849:
URL:
https://github.com/apache/incubator-seata-go/pull/849#discussion_r2263331769
##########
pkg/tm/transaction_executor.go:
##########
@@ -73,8 +73,18 @@ func WithGlobalTx(ctx context.Context, gc *GtxConfig,
business CallbackWithCtx)
}
}
- if re != nil || err != nil {
- re = fmt.Errorf("first phase error: %v, second phase
error: %v", re, err)
+ if re == nil {
+ if deferErr != nil {
+ // if deferErr is not an error, then convert it
to error
+ // this is because panic may be caused by
non-error type e.g. panic("some string")
+ // so we need to convert it to error type
+ if _, ok := deferErr.(error); !ok {
+ deferErr = fmt.Errorf("%v", deferErr)
+ }
+ re = deferErr.(error)
Review Comment:
This type assertion will panic if deferErr is not an error type, even though
the code above checks for this case. The type assertion is unnecessary since we
already confirmed deferErr is an error type. Replace with: re =
deferErr.(error) or simply re = deferErr after the type conversion.
```suggestion
re = deferErr
```
##########
pkg/tm/transaction_executor.go:
##########
@@ -73,8 +73,18 @@ func WithGlobalTx(ctx context.Context, gc *GtxConfig,
business CallbackWithCtx)
}
}
- if re != nil || err != nil {
- re = fmt.Errorf("first phase error: %v, second phase
error: %v", re, err)
+ if re == nil {
Review Comment:
The logic structure has changed significantly but the original error
combining logic (first phase error + second phase error) appears to have been
removed entirely. Consider if this is intentional or if some form of error
aggregation should still occur when both re and err are present.
--
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: notifications-unsubscr...@seata.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For additional commands, e-mail: notifications-h...@seata.apache.org
