[
https://issues.apache.org/jira/browse/THRIFT-2854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14225305#comment-14225305
]
Jens Geyer commented on THRIFT-2854:
------------------------------------
{quote}where are the flaws?{quote}
{code:title=protocol.go}
// note the error returns
type TProtocol interface {
WriteMessageBegin(name string, typeId TMessageType, seqid int32) error
WriteMessageEnd() error
// ... more code ...
ReadMessageBegin() (name string, typeId TMessageType, seqid int32, err
error)
ReadMessageEnd() error
// ... more code ...
Flush() (err error)
{code}
{code:title=some_generated_code.go}
func (p *foobarProcessorBaz_) Process(seqId int32, iprot, oprot
thrift.TProtocol) (success bool, err thrift.TException) {
args := NewBazArgs_{}
if err = args.Read(iprot); err != nil {
iprot.ReadMessageEnd() // <== no error check
x := thrift.NewTApplicationException(thrift.PROTOCOL_ERROR,
err.Error())
oprot.WriteMessageBegin("Quox", thrift.EXCEPTION, seqId) //
<== no error check
x.Write(oprot) // <== no error check
oprot.WriteMessageEnd() // <== no error check
oprot.Flush() // <== no error check
return false, err
}
iprot.ReadMessageEnd()
// ... more code ...
{code}
{quote}
Why does the Process() functions returns 2 variables (success bool, err
thrift.TException). Isn't just having err thrift.TException enough?
{quote}
That would break too many things. You know, people are actually using this
stuff and we already had some breaking changes in the Go library not so long
ago. I would like to avoid that, especially compared to the limited benefit we
would get from it. That is also a point to consider when providing patches in
general: First, we have a somwehat standardized layout of certain things across
all languages that we want to keep. Next, we want to be compatible.
Thanks to a number of significant contributions the Go part of Thrift has
reached quite a good state that we absolutely should strive to improve (there
is room for that), but not at all cost.
> Go Struct writer and reader looses important error information
> --------------------------------------------------------------
>
> Key: THRIFT-2854
> URL: https://issues.apache.org/jira/browse/THRIFT-2854
> Project: Thrift
> Issue Type: Bug
> Components: Go - Compiler
> Affects Versions: 0.9.2
> Reporter: Chi Vinh Le
> Fix For: 0.9.3
>
>
> The GO Compiler generates code for a struct so that the following occurs:
> When an error occurs while reading or writing, a new error is created with
> additional text information using {code}fmt.Errorf{code}.
> By doing this the original error is completely lost. This is a real problem
> because errors of type TTransportException and TProtocolExceptions which
> contain additional information are lost. This will cause bad error handling,
> as the server implementation is dependent on those information.
> In my personal fork, I have a quick'n dirty fix for this, but I'm looking for
> a better option.
> Maybe instead of {code}fmt.Errorf{code} we could use this:
> {code}
> // Prepends additional information to an error without losing the Thrift
> interface
> func PrependError(err error, prepend string) error {
> if t, ok := err.(TTransportException); ok {
> return NewTTransportException(t.TypeId(), prepend+t.Error())
> }
> if t, ok := err.(TProtocolException); ok {
> return NewTProtocolExceptionWithType(t.TypeId(),
> errors.New(prepend+err.Error()))
> }
> if t, ok := err.(TApplicationException); ok {
> return NewTApplicationException(t.TypeId(), prepend+t.Error())
> }
> return errors.New(prepend + err.Error())
> }
> {code}
> I want to discuss this first here, because making a patch is quiet some work
> and I don't want major changes after I create a fix.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
