[thrift] branch master updated: THRIFT-5459: Fix breaking issue when adding a new exception

[yuxuan]

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

yuxuan 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 cdbcff9  THRIFT-5459: Fix breaking issue when adding a new exception
cdbcff9 is described below

commit cdbcff99958c024d224de98a0dfc2e98d6d779b6
Author: Yuxuan 'fishy' Wang <yuxuan.w...@reddit.com>
AuthorDate: Wed Sep 1 14:17:31 2021 -0700

    THRIFT-5459: Fix breaking issue when adding a new exception
    
    Client: go
    
    Currently in the compiler generated go code, adding a new exception to
    an existing endpoint can cause unexpected behaviors when the client
    isn't updated. Fix the issue.
    
    Will be cherry-picked into 0.15.0 after merged.
---
 CHANGES.md                                         |  1 +
 compiler/cpp/src/thrift/generate/t_go_generator.cc | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/CHANGES.md b/CHANGES.md
index 5254682..adee1dd 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -16,6 +16,7 @@
 - [THRIFT-5389](https://issues.apache.org/jira/browse/THRIFT-5389) - The 
compiler now generates correct go code with thrift constant defined on optional 
enum/typedef fields
 - [THRIFT-4797](https://issues.apache.org/jira/browse/THRIFT-4797) - The 
compiler now correctly auto renames import thrift namespaces when they collide 
with system imports
 - [THRIFT-5453](https://issues.apache.org/jira/browse/THRIFT-5453) - Defer DNS 
lookups from NewTSocketConf (without any timeout check) to TSocket.Open 
(subject to ConnectTimeout set in TConfiguration)
+- [THRIFT-5459](https://issues.apache.org/jira/browse/THRIFT-5459) - Client 
calls will return TApplicationException with MISSING_RESULT when the result is 
a struct but is unset, and no other error is known.
 
 
 ## 0.14.2
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc 
b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index afed5ac..910eed3 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -2295,7 +2295,23 @@ void t_go_generator::generate_service_client(t_service* 
tservice) {
         f_types_ << indent() << "}" << endl << endl;
       }
 
-      if (!(*f_iter)->get_returntype()->is_void()) {
+      if ((*f_iter)->get_returntype()->is_struct()) {
+        // Check if the result is nil, which likely means we have a new
+        // exception added but unknown to the client yet
+        // (e.g. client hasn't updated the thrift file).
+        // Sadly this check can only be reliable done when the return type is a
+        // struct in go.
+        std::string retName = tmp("_ret");
+        f_types_ << indent() << "if " << retName << " := " << resultName
+                 << ".GetSuccess(); " << retName << " != nil {" << endl;
+        indent_up();
+        f_types_ << indent() << "return " << retName << ", nil" << endl;
+        indent_down();
+        f_types_ << indent() << "}" << endl;
+        f_types_ << indent() << "return nil, "
+                 << "thrift.NewTApplicationException(thrift.MISSING_RESULT, \""
+                 << method << " failed: unknown result\")" << endl;
+      } else if (!(*f_iter)->get_returntype()->is_void()) {
         f_types_ << indent() << "return " << resultName << ".GetSuccess(), 
nil" << endl;
       } else {
         f_types_ << indent() << "return nil" << endl;