[thrift] branch master updated: THRIFT-4984: Ignore EOF errors in TSimpleServer, take 2

[dc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dcelasun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new 4653009  THRIFT-4984: Ignore EOF errors in TSimpleServer, take 2
4653009 is described below

commit 4653009035db96228e33e8965e432752b41c8ed1
Author: Yuxuan 'fishy' Wang <yu...@reddit.com>
AuthorDate: Tue Nov 5 13:31:13 2019 -0800

    THRIFT-4984: Ignore EOF errors in TSimpleServer, take 2
    
    This is a different approach to take THRIFT-4984: Instead of checking
    EOF errors in place, handle them in a consolidated, deferred function.
    
    Also improve test error messages.
    
    Client: go
    
    This closes #1907.
---
 lib/go/test/tests/thrifttest_driver.go | 17 +++++++++--------
 lib/go/thrift/simple_server.go         | 27 ++++++++++++++++++++-------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/lib/go/test/tests/thrifttest_driver.go b/lib/go/test/tests/thrifttest_driver.go
index de54cbc..4fc5baa 100644
--- a/lib/go/test/tests/thrifttest_driver.go
+++ b/lib/go/test/tests/thrifttest_driver.go
@@ -213,24 +213,25 @@ func (p *ThriftTestDriver) Start() {
 		2: {thrifttest.Numberz_SIX: crazyEmpty},
 	}
 	if r, err := client.TestInsanity(defaultCtx, crazy); !reflect.DeepEqual(r, insanity) || err != nil {
-		t.Fatal("TestInsanity failed")
+		t.Fatal("TestInsanity failed:", err)
 	}
 
 	if err := client.TestException(defaultCtx, "TException"); err == nil {
-		t.Fatal("TestException TException failed")
+		t.Fatal("TestException TException failed:", err)
 	}
 
-	if err, ok := client.TestException(defaultCtx, "Xception").(*thrifttest.Xception); ok == false || err == nil {
-		t.Fatal("TestException Xception failed")
-	} else if err.ErrorCode != 1001 || err.Message != "Xception" {
-		t.Fatal("TestException Xception failed")
+	err := client.TestException(defaultCtx, "Xception")
+	if e, ok := err.(*thrifttest.Xception); ok == false || e == nil {
+		t.Fatal("TestException Xception failed:", err)
+	} else if e.ErrorCode != 1001 || e.Message != "Xception" {
+		t.Fatal("TestException Xception failed:", e)
 	}
 
 	if err := client.TestException(defaultCtx, "no Exception"); err != nil {
-		t.Fatal("TestException no Exception failed")
+		t.Fatal("TestException no Exception failed:", err)
 	}
 
 	if err := client.TestOneway(defaultCtx, 0); err != nil {
-		t.Fatal("TestOneway failed")
+		t.Fatal("TestOneway failed:", err)
 	}
 }
diff --git a/lib/go/thrift/simple_server.go b/lib/go/thrift/simple_server.go
index 51e3509..aa5a6a6 100644
--- a/lib/go/thrift/simple_server.go
+++ b/lib/go/thrift/simple_server.go
@@ -216,7 +216,25 @@ func (p *TSimpleServer) Stop() error {
 	return nil
 }
 
-func (p *TSimpleServer) processRequests(client TTransport) error {
+// If err is actually EOF, return nil, otherwise return err as-is.
+func treatEOFErrorsAsNil(err error) error {
+	if err == nil {
+		return nil
+	}
+	if err == io.EOF {
+		return nil
+	}
+	if err, ok := err.(TTransportException); ok && err.TypeId() == END_OF_FILE {
+		return nil
+	}
+	return err
+}
+
+func (p *TSimpleServer) processRequests(client TTransport) (err error) {
+	defer func() {
+		err = treatEOFErrorsAsNil(err)
+	}()
+
 	processor := p.processorFactory.GetProcessor(client)
 	inputTransport, err := p.inputTransportFactory.GetTransport(client)
 	if err != nil {
@@ -261,9 +279,6 @@ func (p *TSimpleServer) processRequests(client TTransport) error {
 			// won't break when it's called again later when we
 			// actually start to read the message.
 			if err := headerProtocol.ReadFrame(); err != nil {
-				if err == io.EOF {
-					return nil
-				}
 				return err
 			}
 			ctx = AddReadTHeaderToContext(ctx, headerProtocol.GetReadHeaders())
@@ -271,9 +286,7 @@ func (p *TSimpleServer) processRequests(client TTransport) error {
 		}
 
 		ok, err := processor.Process(ctx, inputProtocol, outputProtocol)
-		if err, ok := err.(TTransportException); ok && err.TypeId() == END_OF_FILE {
-			return nil
-		} else if err != nil {
+		if _, ok := err.(TTransportException); ok && err != nil {
 			return err
 		}
 		if err, ok := err.(TApplicationException); ok && err.TypeId() == UNKNOWN_METHOD {