Skip to content

Commit

Permalink
Wrap errors from the conenction handling logic
Browse files Browse the repository at this point in the history
Summary:
I am not sure if we need similar wrapping on lower levels:
- compact_json_format.go
- simple_json_format.go
- binary_format.go
- ...

Reviewed By: awalterschulze

Differential Revision: D64966252

fbshipit-source-id: 7fd0a5fe5be064779766216395af82cdcaba5cec
  • Loading branch information
inesusvet authored and facebook-github-bot committed Oct 25, 2024
1 parent 7a98ea9 commit f51f0c9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 12 deletions.
12 changes: 6 additions & 6 deletions third-party/thrift/src/thrift/lib/go/thrift/clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ func (cc *ClientConn) SendMsg(ctx context.Context, method string, req types.IReq
cc.seqID++

if err := setRequestHeaders(ctx, cc.proto); err != nil {
return err
return fmt.Errorf("Failed to set request headers: %w", err)
}

if err := cc.proto.WriteMessageBegin(method, msgType, cc.seqID); err != nil {
return err
return fmt.Errorf("Failed to write message preamble: %w", err)
}

if err := req.Write(cc.proto); err != nil {
return err
return fmt.Errorf("Failed to write request body: %w", err)
}

if err := cc.proto.WriteMessageEnd(); err != nil {
return err
return fmt.Errorf("Failed to write message epilogue: %w", err)
}

return cc.proto.Flush()
Expand All @@ -69,7 +69,7 @@ func (cc *ClientConn) RecvMsg(ctx context.Context, method string, res types.IRes
recvMethod, mTypeID, seqID, err := cc.proto.ReadMessageBegin()

if err != nil {
return err
return fmt.Errorf("Failed to read message preamble: %w", err)
}

if method != recvMethod {
Expand All @@ -83,7 +83,7 @@ func (cc *ClientConn) RecvMsg(ctx context.Context, method string, res types.IRes
switch mTypeID {
case types.REPLY:
if err := res.Read(cc.proto); err != nil {
return err
return fmt.Errorf("Failed to read message body: %w", err)
}

return cc.proto.ReadMessageEnd()
Expand Down
13 changes: 7 additions & 6 deletions third-party/thrift/src/thrift/lib/go/thrift/clientconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package thrift
import (
"context"
"errors"
"fmt"
"testing"

"github.com/facebook/fbthrift/thrift/lib/go/thrift/types"
Expand Down Expand Up @@ -105,19 +106,19 @@ func TestSendMsgError(t *testing.T) {
// Bad WriteMessageBegin
{
proto: &fakeProto{errOnMessageBegin: true},
expected: errFakeProtoWriteMessageBegin,
expected: fmt.Errorf("Failed to write message preamble: %w", errFakeProtoWriteMessageBegin),
},
// Bad request.Write
{
proto: &fakeProto{errOnMessageBegin: true},
request: &fakeRequest{shouldReturnError: true},
expected: errFakeProtoWriteMessageBegin,
expected: fmt.Errorf("Failed to write message preamble: %w", errFakeProtoWriteMessageBegin),
},
// Bad WriteMessageEnd
{
proto: &fakeProto{errOnMessageEnd: true},
request: &fakeRequest{shouldReturnError: false},
expected: errFakeProtoWriteMessageEnd,
expected: fmt.Errorf("Failed to write message epilogue: %w", errFakeProtoWriteMessageEnd),
},
// Bad Flush
{
Expand Down Expand Up @@ -148,7 +149,7 @@ func TestRecvMsgError(t *testing.T) {
// Error reading message begin
{
proto: &fakeProto{shouldReturnError: true},
expected: errFakeProtoReadMessageBegin,
expected: fmt.Errorf("Failed to read message preamble: %w", errFakeProtoReadMessageBegin),
},

// Bad method name in response
Expand All @@ -173,7 +174,7 @@ func TestRecvMsgError(t *testing.T) {
{
proto: &fakeProto{method: "foobar", seqID: 0, typeID: types.REPLY},
response: &fakeResponse{shouldReturnError: true},
expected: errFakeResponseRead,
expected: fmt.Errorf("Failed to read message body: %w", errFakeResponseRead),
},
}

Expand All @@ -184,7 +185,7 @@ func TestRecvMsgError(t *testing.T) {
cc := ClientConn{proto: testCase.proto}

if err := cc.RecvMsg(ctx, "foobar", testCase.response); err.Error() != testCase.expected.Error() {
t.Errorf("#%d: expected call to RecvMsg to return \"%+v\"; got \"%+v\"", i, testCase.expected, err)
t.Errorf("#%d: expected call to RecvMsg to return \"%+v\"; got \"%+v\"", i, testCase.expected.Error(), err.Error())
}
}
}

0 comments on commit f51f0c9

Please sign in to comment.