tte commented on a change in pull request #2298:
URL: https://github.com/apache/thrift/pull/2298#discussion_r560251127
##########
File path: lib/go/thrift/exception.go
##########
@@ -26,19 +26,86 @@ import (
// Generic Thrift exception
type TException interface {
error
+
+ TExceptionType() TExceptionType
}
// Prepends additional information to an error without losing the Thrift
exception interface
func PrependError(prepend string, err error) error {
- if t, ok := err.(TTransportException); ok {
- return NewTTransportException(t.TypeId(), prepend+t.Error())
+ msg := prepend + err.Error()
+
+ if te, ok := err.(TException); ok {
+ switch te.TExceptionType() {
+ case TExceptionTypeTransport:
+ if t, ok := err.(TTransportException); ok {
+ return NewTTransportException(t.TypeId(), msg)
+ }
+ case TExceptionTypeProtocol:
+ if t, ok := err.(TProtocolException); ok {
+ return
NewTProtocolExceptionWithType(t.TypeId(), errors.New(msg))
+ }
+ case TExceptionTypeApplication:
+ if t, ok := err.(TApplicationException); ok {
+ return NewTApplicationException(t.TypeId(), msg)
+ }
+ }
+
+ return wrappedTException{
+ err: errors.New(msg),
Review comment:
@fishy Hi, thank you for expanding thrift errors and Unwrap support!
At this line we are losing here underline error and seems it's OK by design.
I'm just curious why we are setting `msg` instead of storing the original err +
msg at wrappedTException?
I'm working on error handler for thrift clients and the most common case
when `context.Canceled` error occurred. The problem is when we perform
read\write operations at thrift fields - we can get `context.Canceled` error,
but at the moment I can't unwrap thrift result error because of concatenation
of pretending msg.
Saving underlying error would be a simplified error handling process. For
example:
```golang
// updated wrappedTException
type wrappedTException struct {
err error
pretendMsg string
tExceptionType TExceptionType
}
func (w wrappedTException) Error() string {
return w.pretendMsg + w.err.Error()
}
// error occursed during thrift ReadField1 and return context.Canceled
func (p *ServiceResult) ReadField1(iprot thrift.TProtocol) error {
if err := p.Field.Read(iprot); err != nil {
return thrift.PrependError(fmt.Sprintf("%T error reading struct: ",
p.Field), err)
}
return nil
}
// app error helper fn
func isContextCancelledError(err error) bool {
if err == nil {
return false
}
switch e := err.(type) {
case thrift.TException:
return errors.Is(e, context.Canceled)
default:
return false
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
