THRIFT-2854 Go Struct writer and reader looses important error information
Client: Go
Patch: Chi Vinh Le <cvl@chinet.info>
This closes #291
Fixes error reporting in go generator
diff --git a/lib/go/test/ErrorTest.thrift b/lib/go/test/ErrorTest.thrift
new file mode 100644
index 0000000..eeaeb95
--- /dev/null
+++ b/lib/go/test/ErrorTest.thrift
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ * Contains some contributions under the Thrift Software License.
+ * Please see doc/old-thrift-license.txt in the Thrift distribution for
+ * details.
+ */
+struct TestStruct
+{
+ 1: map<string, string> m,
+ 2: list<string> l,
+ 3: set<string> s,
+ 4: i32 i
+}
+
+service ErrorTest
+{
+ TestStruct testStruct(1: TestStruct thing)
+}
diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am
index c13ba74..a93318a 100644
--- a/lib/go/test/Makefile.am
+++ b/lib/go/test/Makefile.am
@@ -30,7 +30,8 @@
ServicesTest.thrift \
GoTagTest.thrift \
TypedefFieldTest.thrift \
- RefAnnotationFieldsTest.thrift
+ RefAnnotationFieldsTest.thrift \
+ ErrorTest.thrift
mkdir -p gopath/src
grep -v list.*map.*list.*map $(THRIFTTEST) | grep -v 'set<Insanity>' > ThriftTest.thrift
$(THRIFT) -r IncludesTest.thrift
@@ -42,6 +43,7 @@
$(THRIFT) GoTagTest.thrift
$(THRIFT) TypedefFieldTest.thrift
$(THRIFT) RefAnnotationFieldsTest.thrift
+ $(THRIFT) ErrorTest.thrift
GOPATH=`pwd`/gopath $(GO) get code.google.com/p/gomock/gomock
ln -nfs ../../../thrift gopath/src/thrift
ln -nfs ../../tests gopath/src/tests
@@ -53,7 +55,8 @@
binarykeytest \
servicestest \
typedeffieldtest \
- refannotationfieldstest
+ refannotationfieldstest \
+ errortest
GOPATH=`pwd`/gopath $(GO) test thrift tests
clean-local:
@@ -73,4 +76,5 @@
OptionalFieldsTest.thrift \
RefAnnotationFieldsTest.thrift \
ServicesTest.thrift \
- TypedefFieldTest.thrift
+ TypedefFieldTest.thrift \
+ ErrorTest.thrift
diff --git a/lib/go/test/tests/client_error_test.go b/lib/go/test/tests/client_error_test.go
new file mode 100644
index 0000000..11ea1c7
--- /dev/null
+++ b/lib/go/test/tests/client_error_test.go
@@ -0,0 +1,455 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package tests
+
+import (
+ "code.google.com/p/gomock/gomock"
+ "errors"
+ "errortest"
+ "testing"
+ "thrift"
+)
+
+// Setup mock to fail at a certain position. Return true if position exists otherwise false.
+func prepareClientProtocolFailure(protocol *MockTProtocol, failAt int, failWith error) bool {
+ var err error = nil
+
+ if failAt == 0 {
+ err = failWith
+ }
+ last := protocol.EXPECT().WriteMessageBegin("testStruct", thrift.TMessageType(1), int32(1)).Return(err)
+ if failAt == 0 {
+ return true
+ }
+ if failAt == 1 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteStructBegin("testStruct_args").Return(err).After(last)
+ if failAt == 1 {
+ return true
+ }
+ if failAt == 2 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldBegin("thing", thrift.TType(thrift.STRUCT), int16(1)).Return(err).After(last)
+ if failAt == 2 {
+ return true
+ }
+ if failAt == 3 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteStructBegin("TestStruct").Return(err).After(last)
+ if failAt == 3 {
+ return true
+ }
+ if failAt == 4 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldBegin("m", thrift.TType(thrift.MAP), int16(1)).Return(err).After(last)
+ if failAt == 4 {
+ return true
+ }
+ if failAt == 5 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteMapBegin(thrift.TType(thrift.STRING), thrift.TType(thrift.STRING), 0).Return(err).After(last)
+ if failAt == 5 {
+ return true
+ }
+ if failAt == 6 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteMapEnd().Return(err).After(last)
+ if failAt == 6 {
+ return true
+ }
+ if failAt == 7 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldEnd().Return(err).After(last)
+ if failAt == 7 {
+ return true
+ }
+ if failAt == 8 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldBegin("l", thrift.TType(thrift.LIST), int16(2)).Return(err).After(last)
+ if failAt == 8 {
+ return true
+ }
+ if failAt == 9 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteListBegin(thrift.TType(thrift.STRING), 0).Return(err).After(last)
+ if failAt == 9 {
+ return true
+ }
+ if failAt == 10 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteListEnd().Return(err).After(last)
+ if failAt == 10 {
+ return true
+ }
+ if failAt == 11 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldEnd().Return(err).After(last)
+ if failAt == 11 {
+ return true
+ }
+ if failAt == 12 {
+ err = failWith
+ }
+
+ last = protocol.EXPECT().WriteFieldBegin("s", thrift.TType(thrift.SET), int16(3)).Return(err).After(last)
+ if failAt == 12 {
+ return true
+ }
+ if failAt == 13 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteSetBegin(thrift.TType(thrift.STRING), 0).Return(err).After(last)
+ if failAt == 13 {
+ return true
+ }
+ if failAt == 14 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteSetEnd().Return(err).After(last)
+ if failAt == 14 {
+ return true
+ }
+ if failAt == 15 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldEnd().Return(err).After(last)
+ if failAt == 15 {
+ return true
+ }
+ if failAt == 16 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldBegin("i", thrift.TType(thrift.I32), int16(4)).Return(err).After(last)
+ if failAt == 16 {
+ return true
+ }
+ if failAt == 17 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteI32(int32(3)).Return(err).After(last)
+ if failAt == 17 {
+ return true
+ }
+ if failAt == 18 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldEnd().Return(err).After(last)
+ if failAt == 18 {
+ return true
+ }
+ if failAt == 19 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldStop().Return(err).After(last)
+ if failAt == 19 {
+ return true
+ }
+ if failAt == 20 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteStructEnd().Return(err).After(last)
+ if failAt == 20 {
+ return true
+ }
+ if failAt == 21 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldEnd().Return(err).After(last)
+ if failAt == 21 {
+ return true
+ }
+ if failAt == 22 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteFieldStop().Return(err).After(last)
+ if failAt == 22 {
+ return true
+ }
+ if failAt == 23 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteStructEnd().Return(err).After(last)
+ if failAt == 23 {
+ return true
+ }
+ if failAt == 24 {
+ err = failWith
+ }
+ last = protocol.EXPECT().WriteMessageEnd().Return(err).After(last)
+ if failAt == 24 {
+ return true
+ }
+ if failAt == 25 {
+ err = failWith
+ }
+ last = protocol.EXPECT().Flush().Return(err).After(last)
+ if failAt == 25 {
+ return true
+ }
+ if failAt == 26 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadMessageBegin().Return("testStruct", thrift.TMessageType(1), int32(1), err).After(last)
+ if failAt == 26 {
+ return true
+ }
+ if failAt == 27 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadStructBegin().Return("testStruct_args", err).After(last)
+ if failAt == 27 {
+ return true
+ }
+ if failAt == 28 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("_", thrift.TType(thrift.STRUCT), int16(0), err).After(last)
+ if failAt == 28 {
+ return true
+ }
+ if failAt == 29 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadStructBegin().Return("TestStruct", err).After(last)
+ if failAt == 29 {
+ return true
+ }
+ if failAt == 30 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("m", thrift.TType(thrift.MAP), int16(1), err).After(last)
+ if failAt == 30 {
+ return true
+ }
+ if failAt == 31 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadMapBegin().Return(thrift.TType(thrift.STRING), thrift.TType(thrift.STRING), 0, err).After(last)
+ if failAt == 31 {
+ return true
+ }
+ if failAt == 32 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadMapEnd().Return(err).After(last)
+ if failAt == 32 {
+ return true
+ }
+ if failAt == 33 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldEnd().Return(err).After(last)
+ if failAt == 33 {
+ return true
+ }
+ if failAt == 34 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("l", thrift.TType(thrift.LIST), int16(2), err).After(last)
+ if failAt == 34 {
+ return true
+ }
+ if failAt == 35 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadListBegin().Return(thrift.TType(thrift.STRING), 0, err).After(last)
+ if failAt == 35 {
+ return true
+ }
+ if failAt == 36 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadListEnd().Return(err).After(last)
+ if failAt == 36 {
+ return true
+ }
+ if failAt == 37 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldEnd().Return(err).After(last)
+ if failAt == 37 {
+ return true
+ }
+ if failAt == 38 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("s", thrift.TType(thrift.SET), int16(3), err).After(last)
+ if failAt == 38 {
+ return true
+ }
+ if failAt == 39 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadSetBegin().Return(thrift.TType(thrift.STRING), 0, err).After(last)
+ if failAt == 39 {
+ return true
+ }
+ if failAt == 40 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadSetEnd().Return(err).After(last)
+ if failAt == 40 {
+ return true
+ }
+ if failAt == 41 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldEnd().Return(err).After(last)
+ if failAt == 41 {
+ return true
+ }
+ if failAt == 42 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("i", thrift.TType(thrift.I32), int16(4), err).After(last)
+ if failAt == 42 {
+ return true
+ }
+ if failAt == 43 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadI32().Return(int32(3), err).After(last)
+ if failAt == 43 {
+ return true
+ }
+ if failAt == 44 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldEnd().Return(err).After(last)
+ if failAt == 44 {
+ return true
+ }
+ if failAt == 45 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("_", thrift.TType(thrift.STOP), int16(5), err).After(last)
+ if failAt == 45 {
+ return true
+ }
+ if failAt == 46 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadStructEnd().Return(err).After(last)
+ if failAt == 46 {
+ return true
+ }
+ if failAt == 47 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldEnd().Return(err).After(last)
+ if failAt == 47 {
+ return true
+ }
+ if failAt == 48 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadFieldBegin().Return("_", thrift.TType(thrift.STOP), int16(1), err).After(last)
+ if failAt == 48 {
+ return true
+ }
+ if failAt == 49 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadStructEnd().Return(err).After(last)
+ if failAt == 49 {
+ return true
+ }
+ if failAt == 50 {
+ err = failWith
+ }
+ last = protocol.EXPECT().ReadMessageEnd().Return(err).After(last)
+ if failAt == 50 {
+ return true
+ }
+ return false
+}
+
+func TestClientReportTTransportErrors(t *testing.T) {
+ mockCtrl := gomock.NewController(t)
+ transport := thrift.NewTMemoryBuffer()
+
+ thing := errortest.NewTestStruct()
+ thing.M = make(map[string]string)
+ thing.L = make([]string, 0)
+ thing.S = make(map[string]bool)
+ thing.I = 3
+
+ err := thrift.NewTTransportException(thrift.TIMED_OUT, "test")
+ for i := 0; ; i++ {
+ protocol := NewMockTProtocol(mockCtrl)
+ if !prepareClientProtocolFailure(protocol, i, err) {
+ return
+ }
+ client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
+ _, retErr := client.TestStruct(thing)
+ err2, ok := retErr.(thrift.TTransportException)
+ if !ok {
+ t.Fatal("Expected a TTrasportException")
+ }
+
+ if err2.TypeId() != err.TypeId() {
+ t.Fatal("Expected a same error type id")
+ }
+
+ mockCtrl.Finish()
+ }
+}
+
+func TestClientReportTProtocolErrors(t *testing.T) {
+ mockCtrl := gomock.NewController(t)
+ transport := thrift.NewTMemoryBuffer()
+
+ thing := errortest.NewTestStruct()
+ thing.M = make(map[string]string)
+ thing.L = make([]string, 0)
+ thing.S = make(map[string]bool)
+ thing.I = 3
+
+ err := thrift.NewTProtocolExceptionWithType(thrift.INVALID_DATA, errors.New("test"))
+ for i := 0; ; i++ {
+ protocol := NewMockTProtocol(mockCtrl)
+ if !prepareClientProtocolFailure(protocol, i, err) {
+ return
+ }
+ client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
+ _, retErr := client.TestStruct(thing)
+ err2, ok := retErr.(thrift.TProtocolException)
+ if !ok {
+ t.Fatal("Expected a TProtocolException")
+ }
+
+ if err2.TypeId() != err.TypeId() {
+ t.Fatal("Expected a same error type id")
+ }
+
+ mockCtrl.Finish()
+ }
+}
diff --git a/lib/go/thrift/exception.go b/lib/go/thrift/exception.go
index e08ffc0..4afcefd 100644
--- a/lib/go/thrift/exception.go
+++ b/lib/go/thrift/exception.go
@@ -19,7 +19,26 @@
package thrift
+import (
+ "errors"
+)
+
// Generic Thrift exception
type TException interface {
error
}
+
+// Prepends additional information to an error without loosing the Thrift exception interface
+func PrependError(prepend string, err error) 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())
+}
diff --git a/lib/go/thrift/exception_test.go b/lib/go/thrift/exception_test.go
new file mode 100644
index 0000000..71f5e2c
--- /dev/null
+++ b/lib/go/thrift/exception_test.go
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package thrift
+
+import (
+ "errors"
+ "testing"
+)
+
+func TestPrependError(t *testing.T) {
+ err := NewTApplicationException(INTERNAL_ERROR, "original error")
+ err2, ok := PrependError("Prepend: ", err).(TApplicationException)
+ if !ok {
+ t.Fatal("Couldn't cast error TApplicationException")
+ }
+ if err2.Error() != "Prepend: original error" {
+ t.Fatal("Unexpected error string")
+ }
+ if err2.TypeId() != INTERNAL_ERROR {
+ t.Fatal("Unexpected type error")
+ }
+
+ err3 := NewTProtocolExceptionWithType(INVALID_DATA, errors.New("original error"))
+ err4, ok := PrependError("Prepend: ", err3).(TProtocolException)
+ if !ok {
+ t.Fatal("Couldn't cast error TProtocolException")
+ }
+ if err4.Error() != "Prepend: original error" {
+ t.Fatal("Unexpected error string")
+ }
+ if err4.TypeId() != INVALID_DATA {
+ t.Fatal("Unexpected type error")
+ }
+
+ err5 := NewTTransportException(TIMED_OUT, "original error")
+ err6, ok := PrependError("Prepend: ", err5).(TTransportException)
+ if !ok {
+ t.Fatal("Couldn't cast error TTransportException")
+ }
+ if err6.Error() != "Prepend: original error" {
+ t.Fatal("Unexpected error string")
+ }
+ if err6.TypeId() != TIMED_OUT {
+ t.Fatal("Unexpected type error")
+ }
+
+ err7 := errors.New("original error")
+ err8 := PrependError("Prepend: ", err7)
+ if err8.Error() != "Prepend: original error" {
+ t.Fatal("Unexpected error string")
+ }
+}