Skip to content

Commit

Permalink
THRIFT-5338: Raise minimal supported go version to 1.14.14
Browse files Browse the repository at this point in the history
Client: go

- Update Dockerfiles used by travis
- Add go.mod file
- Modify error handling code to take advantage of errors package updates
  in go1.13
  • Loading branch information
fishy committed Jan 20, 2021
1 parent 0e68e8c commit e27e82c
Show file tree
Hide file tree
Showing 17 changed files with 71 additions and 65 deletions.
2 changes: 1 addition & 1 deletion LANGUAGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ Thrift's core protocol is TBinary, supported by all languages except for JavaScr
<td align=left><a href="https://github.com/apache/thrift/blob/master/lib/go/README.md">Go</a></td>
<!-- Since -----------------><td>0.7.0</td>
<!-- Build Systems ---------><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cred.png" alt=""/></td>
<!-- Language Levels -------><td>1.10.8</td><td>1.13.1</td>
<!-- Language Levels -------><td>1.14.14</td><td>1.15.7</td>
<!-- Low-Level Transports --><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td>
<!-- Transport Wrappers ----><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cred.png" alt=""/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td>
<!-- Protocols -------------><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td><td><img src="doc/images/cgrn.png" alt="Yes"/></td>
Expand Down
2 changes: 1 addition & 1 deletion build/docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ Last updated: October 1, 2017
| dart | 2.0.0 | 2.4.0 | |
| delphi | | | Not in CI |
| erlang | 18.3 | 22.0 | |
| go | 1.10.8 | 1.12.6 | |
| go | 1.14.14 | 1.15.7 | |
| haskell | 7.10.3 | 8.0.2 | |
| haxe | 3.2.1 | 3.4.4 | THRIFT-4352: avoid 3.4.2 |
| java | 1.8.0_191 | 11.0.3 | |
Expand Down
4 changes: 2 additions & 2 deletions build/docker/ubuntu-bionic/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ RUN apt-get install -y --no-install-recommends \
libglib2.0-dev

# golang
ENV GOLANG_VERSION 1.13.1
ENV GOLANG_VERSION 1.15.7
ENV GOLANG_DOWNLOAD_URL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz
ENV GOLANG_DOWNLOAD_SHA256 94f874037b82ea5353f4061e543681a0e79657f787437974214629af8407d124
ENV GOLANG_DOWNLOAD_SHA256 0d142143794721bb63ce6c8a6180c4062bcf8ef4715e7d6d6609f3a8282629b3
RUN curl -fsSL "$GOLANG_DOWNLOAD_URL" -o golang.tar.gz && \
echo "$GOLANG_DOWNLOAD_SHA256 golang.tar.gz" | sha256sum -c - && \
tar -C /usr/local -xzf golang.tar.gz && \
Expand Down
4 changes: 2 additions & 2 deletions build/docker/ubuntu-disco/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ RUN apt-get install -y --no-install-recommends \
libglib2.0-dev

# golang
ENV GOLANG_VERSION 1.12.6
ENV GOLANG_VERSION 1.15.7
ENV GOLANG_DOWNLOAD_URL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz
ENV GOLANG_DOWNLOAD_SHA256 dbcf71a3c1ea53b8d54ef1b48c85a39a6c9a935d01fc8291ff2b92028e59913c
ENV GOLANG_DOWNLOAD_SHA256 0d142143794721bb63ce6c8a6180c4062bcf8ef4715e7d6d6609f3a8282629b3
RUN curl -fsSL "$GOLANG_DOWNLOAD_URL" -o golang.tar.gz && \
echo "$GOLANG_DOWNLOAD_SHA256 golang.tar.gz" | sha256sum -c - && \
tar -C /usr/local -xzf golang.tar.gz && \
Expand Down
4 changes: 2 additions & 2 deletions build/docker/ubuntu-xenial/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ RUN apt-get install -y --no-install-recommends \
libglib2.0-dev

# golang
ENV GOLANG_VERSION 1.10.8
ENV GOLANG_VERSION 1.14.14
ENV GOLANG_DOWNLOAD_URL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz
ENV GOLANG_DOWNLOAD_SHA256 d8626fb6f9a3ab397d88c483b576be41fa81eefcec2fd18562c87626dbb3c39e
ENV GOLANG_DOWNLOAD_SHA256 6f1354c9040d65d1622b451f43c324c1e5197aa9242d00c5a117d0e2625f3e0d
RUN curl -fsSL "$GOLANG_DOWNLOAD_URL" -o golang.tar.gz && \
echo "$GOLANG_DOWNLOAD_SHA256 golang.tar.gz" | sha256sum -c - && \
tar -C /usr/local -xzf golang.tar.gz && \
Expand Down
17 changes: 17 additions & 0 deletions lib/go/test/tests/client_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,23 @@ import (
// TestCase: Comprehensive call and reply workflow in the client.
// Setup mock to fail at a certain position. Return true if position exists otherwise false.
func prepareClientCallReply(protocol *MockTProtocol, failAt int, failWith error) bool {
// NOTE: here the number 50 is the same as the last number at the end of
// this function. If more function calls are added in the future, this
// number needs to be increased accordingly.
//
// It's needed for go 1.14+. Before go 1.14 we only call gomock
// controller's Finish function when this function returns true.
// Starting from go 1.14 the gomock will take advantage of
// testing.T.Cleanup interface, which means the Finish function will
// always be called by t.Cleanup, even if we return false here.
// As a result, in the case we need to return false by this function,
// we must return before calling any of the
// protocol.EXPECT().* functions.
const lastFailAt = 50
if failAt > lastFailAt {
return false
}

var err error = nil

if failAt == 0 {
Expand Down
6 changes: 4 additions & 2 deletions lib/go/thrift/exception.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ type TException interface {
func PrependError(prepend string, err error) error {
msg := prepend + err.Error()

if te, ok := err.(TException); ok {
var te TException
if errors.As(err, &te) {
switch te.TExceptionType() {
case TExceptionTypeTransport:
if t, ok := err.(TTransportException); ok {
Expand All @@ -45,7 +46,8 @@ func PrependError(prepend string, err error) error {
return prependTProtocolException(prepend, t)
}
case TExceptionTypeApplication:
if t, ok := err.(TApplicationException); ok {
var t TApplicationException
if errors.As(err, &t) {
return NewTApplicationException(t.TypeId(), msg)
}
}
Expand Down
3 changes: 3 additions & 0 deletions lib/go/thrift/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module github.com/apache/thrift/lib/go/thrift

go 1.14
5 changes: 3 additions & 2 deletions lib/go/thrift/header_protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package thrift

import (
"context"
"errors"
)

// THeaderProtocol is a thrift protocol that implements THeader:
Expand Down Expand Up @@ -233,8 +234,8 @@ func (p *THeaderProtocol) ReadMessageBegin(ctx context.Context) (name string, ty
var newProto TProtocol
newProto, err = p.transport.Protocol().GetProtocol(p.transport)
if err != nil {
tAppExc, ok := err.(TApplicationException)
if !ok {
var tAppExc TApplicationException
if !errors.As(err, &tAppExc) {
return
}
if e := p.protocol.WriteMessageBegin(ctx, "", EXCEPTION, seqID); e != nil {
Expand Down
2 changes: 1 addition & 1 deletion lib/go/thrift/header_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ func (t *THeaderTransport) parseHeaders(ctx context.Context, frameSize uint32) e
headers := make(THeaderMap)
for {
infoType, err := hp.readVarint32()
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
}
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion lib/go/thrift/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package thrift
import (
"bytes"
"context"
"errors"
"io"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -159,7 +160,7 @@ func (p *THttpClient) Read(buf []byte) (int, error) {
return 0, NewTTransportException(NOT_OPEN, "Response buffer is empty, no request.")
}
n, err := p.response.Body.Read(buf)
if n > 0 && (err == nil || err == io.EOF) {
if n > 0 && (err == nil || errors.Is(err, io.EOF)) {
return n, nil
}
return n, NewTTransportExceptionFromError(err)
Expand Down
3 changes: 2 additions & 1 deletion lib/go/thrift/protocol_exception.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package thrift

import (
"encoding/base64"
"errors"
)

// Thrift Protocol exception
Expand Down Expand Up @@ -76,7 +77,7 @@ func NewTProtocolException(err error) TProtocolException {
return e
}

if _, ok := err.(base64.CorruptInputError); ok {
if errors.As(err, new(base64.CorruptInputError)) {
return NewTProtocolExceptionWithType(INVALID_DATA, err)
}

Expand Down
7 changes: 5 additions & 2 deletions lib/go/thrift/rich_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@

package thrift

import "io"
import (
"errors"
"io"
)

type RichTransport struct {
TTransport
Expand Down Expand Up @@ -49,7 +52,7 @@ func (r *RichTransport) RemainingBytes() (num_bytes uint64) {
func readByte(r io.Reader) (c byte, err error) {
v := [1]byte{0}
n, err := r.Read(v[0:1])
if n > 0 && (err == nil || err == io.EOF) {
if n > 0 && (err == nil || errors.Is(err, io.EOF)) {
return v[0], nil
}
if n > 0 && err != nil {
Expand Down
30 changes: 7 additions & 23 deletions lib/go/thrift/simple_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,11 @@ func treatEOFErrorsAsNil(err error) error {
if err == nil {
return nil
}
// err could be io.EOF wrapped with TProtocolException,
// so that err == io.EOF doesn't necessarily work in some cases.
if err.Error() == io.EOF.Error() {
if errors.Is(err, io.EOF) {
return nil
}
if err, ok := err.(TTransportException); ok && err.TypeId() == END_OF_FILE {
var te TTransportException
if errors.As(err, &te) && te.TypeId() == END_OF_FILE {
return nil
}
return err
Expand Down Expand Up @@ -315,15 +314,14 @@ func (p *TSimpleServer) processRequests(client TTransport) (err error) {
}

ok, err := processor.Process(ctx, inputProtocol, outputProtocol)
// Once we dropped support for pre-go1.13 this can be replaced by:
// errors.Is(err, ErrAbandonRequest)
if unwrapError(err) == ErrAbandonRequest {
if errors.Is(err, ErrAbandonRequest) {
return client.Close()
}
if _, ok := err.(TTransportException); ok && err != nil {
if errors.As(err, new(TTransportException)) && err != nil {
return err
}
if err, ok := err.(TApplicationException); ok && err.TypeId() == UNKNOWN_METHOD {
var tae TApplicationException
if errors.As(err, &tae) && tae.TypeId() == UNKNOWN_METHOD {
continue
}
if !ok {
Expand All @@ -332,17 +330,3 @@ func (p *TSimpleServer) processRequests(client TTransport) (err error) {
}
return nil
}

type unwrapper interface {
Unwrap() error
}

func unwrapError(err error) error {
for {
if u, ok := err.(unwrapper); ok {
err = u.Unwrap()
} else {
return err
}
}
}
3 changes: 2 additions & 1 deletion lib/go/thrift/socket_unix_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
package thrift

import (
"errors"
"io"
"syscall"
"time"
Expand Down Expand Up @@ -67,7 +68,7 @@ func (sc *socketConn) checkConn() error {
return nil
}

if err == syscall.EAGAIN || err == syscall.EWOULDBLOCK {
if errors.Is(err, syscall.EAGAIN) || errors.Is(err, syscall.EWOULDBLOCK) {
// This means the connection is still open but we don't have
// anything to read right now.
return nil
Expand Down
25 changes: 13 additions & 12 deletions lib/go/thrift/transport_exception.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,23 +92,23 @@ func NewTTransportExceptionFromError(e error) TTransportException {
return t
}

newTTransportException := func(typeID int, err error, prefix string) TTransportException {
return &tTransportException{
typeId: typeID,
err: err,
msg: prefix + err.Error(),
}
te := &tTransportException{
typeId: UNKNOWN_TRANSPORT_EXCEPTION,
err: e,
msg: e.Error(),
}

if isTimeoutError(e) {
return newTTransportException(TIMED_OUT, e, "")
te.typeId = TIMED_OUT
return te
}

if e == io.EOF {
return newTTransportException(END_OF_FILE, e, "")
if errors.Is(e, io.EOF) {
te.typeId = END_OF_FILE
return te
}

return newTTransportException(UNKNOWN_TRANSPORT_EXCEPTION, e, "")
return te
}

func prependTTransportException(prepend string, e TTransportException) TTransportException {
Expand All @@ -119,11 +119,12 @@ func prependTTransportException(prepend string, e TTransportException) TTranspor
}
}

// isTimeoutError returns true when err is a timeout error.
// isTimeoutError returns true when err is an error caused by timeout.
//
// Note that this also includes TTransportException wrapped timeout errors.
func isTimeoutError(err error) bool {
if t, ok := err.(timeoutable); ok {
var t timeoutable
if errors.As(err, &t) {
return t.Timeout()
}
return false
Expand Down
16 changes: 4 additions & 12 deletions lib/go/thrift/transport_exception_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,8 @@ func TestTExceptionTimeout(t *testing.T) {
t.Errorf("TypeId was not TIMED_OUT: expected %v, got %v", TIMED_OUT, exception.TypeId())
}

// NOTE: this can also be replaced by errors.Unwrap, but that requires
// go 1.13+.
if e, ok := exception.(unwrapper); !ok {
t.Error("Expected exception to be unwrappable, it is not.")
} else if e.Unwrap() != timeout {
t.Errorf("Unwrapped exception did not match: expected %v, got %v", timeout, e.Unwrap())
if unwrapped := errors.Unwrap(exception); unwrapped != timeout {
t.Errorf("Unwrapped exception did not match: expected %v, got %v", timeout, unwrapped)
}
}

Expand All @@ -66,12 +62,8 @@ func TestTExceptionEOF(t *testing.T) {
t.Errorf("TypeId was not END_OF_FILE: expected %v, got %v", END_OF_FILE, exception.TypeId())
}

// NOTE: this can also be replaced by errors.Unwrap, but that requires
// go 1.13+.
if e, ok := exception.(unwrapper); !ok {
t.Error("Expected exception to be unwrappable, it is not.")
} else if e.Unwrap() != io.EOF {
t.Errorf("Unwrapped exception did not match: expected %v, got %v", io.EOF, e.Unwrap())
if unwrapped := errors.Unwrap(exception); unwrapped != io.EOF {
t.Errorf("Unwrapped exception did not match: expected %v, got %v", io.EOF, unwrapped)
}
}

Expand Down

0 comments on commit e27e82c

Please sign in to comment.