From 22ee7f402dfe9c78d556bc54128373d581b3b483 Mon Sep 17 00:00:00 2001
From: Abhinav Gupta <mail@abhinavg.net>
Date: Tue, 3 Feb 2015 12:52:18 -0800
Subject: [PATCH] [python] Graceful handling of server-side errors

Changes `process_*` functions to handle unexpected exceptions from
user-supplied handlers by throwing a `TApplicationException(INTERNAL_ERROR)`
to the client.
---
 compiler/cpp/src/generate/t_py_generator.cc | 60 +++++++++++++--------
 test/py/TestClient.py                       |  2 +-
 test/py/TestServer.py                       |  2 +
 3 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc
index 2228764b1d5..ac219be3222 100644
--- a/compiler/cpp/src/generate/t_py_generator.cc
+++ b/compiler/cpp/src/generate/t_py_generator.cc
@@ -1798,8 +1798,7 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
     f_service_ << endl;
 
   } else { // py
-    // Try block for a function with exceptions
-    if (xceptions.size() > 0) {
+    if (xceptions.size() > 0 || !tfunction->is_oneway()) {
       f_service_ << indent() << "try:" << endl;
       indent_up();
     }
@@ -1825,34 +1824,53 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
     }
     f_service_ << ")" << endl;
 
-    if (!tfunction->is_oneway() && xceptions.size() > 0) {
-      indent_down();
-      for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
-        f_service_ << indent() << "except " << type_name((*x_iter)->get_type()) << ", "
-                   << (*x_iter)->get_name() << ":" << endl;
-        if (!tfunction->is_oneway()) {
-          indent_up();
-          f_service_ << indent() << "result." << (*x_iter)->get_name() << " = "
-                     << (*x_iter)->get_name() << endl;
-          indent_down();
-        } else {
-          f_service_ << indent() << "pass" << endl;
-        }
+    if (xceptions.size() > 0 || !tfunction->is_oneway()) {
+        indent_down();
+    }
+    for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
+      f_service_ << indent() << "except " << type_name((*x_iter)->get_type()) << ", "
+                 << (*x_iter)->get_name() << ":" << endl;
+      if (!tfunction->is_oneway()) {
+        indent_up();
+        f_service_ << indent() << "result." << (*x_iter)->get_name() << " = "
+                   << (*x_iter)->get_name() << endl;
+        indent_down();
+      } else {
+        f_service_ << indent() << "pass" << endl;
       }
     }
 
-    // Shortcut out here for oneway functions
-    if (tfunction->is_oneway()) {
+    if (!tfunction->is_oneway()) {
+      f_service_ << indent() << "except Exception:" << endl;
+      indent_up();
+
+      f_service_
+        << indent() << "exc = TApplicationException(TApplicationException.INTERNAL_ERROR)" << endl
+        << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name()
+        << "\", TMessageType.EXCEPTION, seqid)" << endl << indent() << "exc.write(oprot)" << endl
+        << indent() << "result = None" << endl;
+
+      indent_down();
+
+      f_service_ << indent() << "if result is not None:" << endl;
+      indent_up();
+
+      f_service_ << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name()
+                 << "\", TMessageType.REPLY, seqid)" << endl << indent() << "result.write(oprot)"
+                 << endl;
+
+      indent_down();
+
+      f_service_ << indent() << "oprot.writeMessageEnd()" << endl
+                 << indent() << "oprot.trans.flush()" << endl;
+    } else {
+      // Shortcut out here for oneway functions
       f_service_ << indent() << "return" << endl;
       indent_down();
       f_service_ << endl;
       return;
     }
 
-    f_service_ << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name()
-               << "\", TMessageType.REPLY, seqid)" << endl << indent() << "result.write(oprot)"
-               << endl << indent() << "oprot.writeMessageEnd()" << endl << indent()
-               << "oprot.trans.flush()" << endl;
 
     // Close function
     indent_down();
diff --git a/test/py/TestClient.py b/test/py/TestClient.py
index a810b3fb8e6..6ea927a71e5 100755
--- a/test/py/TestClient.py
+++ b/test/py/TestClient.py
@@ -195,7 +195,7 @@ def testException(self):
     try:
       self.client.testException("throw_undeclared")
       self.fail("should have thrown exception")
-    except Exception: # type is undefined
+    except TApplicationException: # type is undefined
       pass
 
   def testOneway(self):
diff --git a/test/py/TestServer.py b/test/py/TestServer.py
index b29eabda9ba..86422e81921 100755
--- a/test/py/TestServer.py
+++ b/test/py/TestServer.py
@@ -118,6 +118,8 @@ def testException(self, arg):
       raise Xception(errorCode=1001, message=arg)
     elif arg == 'TException':
       raise TException(message='This is a TException')
+    elif arg == 'throw_undeclared':
+      raise ValueError('foo')
 
   def testMultiException(self, arg0, arg1):
     if options.verbose > 1: