THRIFT-3602 Make Tornado server send exception on unexpected handler error
Client: py
This closes #839
This closes #1425
diff --git a/compiler/cpp/src/thrift/generate/t_py_generator.cc b/compiler/cpp/src/thrift/generate/t_py_generator.cc
index 8851c31..3b51714 100644
--- a/compiler/cpp/src/thrift/generate/t_py_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_py_generator.cc
@@ -1995,17 +1995,16 @@
indent_down();
} else if (gen_tornado_) {
- // TODO: Propagate arbitrary exception raised by handler to client as does plain "py"
-
// Generate the function call
t_struct* arg_struct = tfunction->get_arglist();
const std::vector<t_field*>& fields = arg_struct->get_members();
vector<t_field*>::const_iterator f_iter;
- if (xceptions.size() > 0) {
- f_service_ << indent() << "try:" << endl;
- indent_up();
+ if (!tfunction->is_oneway()) {
+ indent(f_service_) << "msg_type = TMessageType.REPLY" << endl;
}
+ f_service_ << indent() << "try:" << endl;
+ indent_up();
f_service_ << indent();
if (!tfunction->is_oneway() && !tfunction->get_returntype()->is_void()) {
f_service_ << "result.success = ";
@@ -2022,27 +2021,43 @@
}
f_service_ << "))" << endl;
- if (!tfunction->is_oneway() && xceptions.size() > 0) {
- indent_down();
+ indent_down();
+ if (!tfunction->is_oneway()) {
for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
- f_service_ << indent() << "except " << type_name((*x_iter)->get_type()) << " as "
- << (*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;
- }
+ const string& xname = (*x_iter)->get_name();
+ f_service_ << indent() << "except " << type_name((*x_iter)->get_type()) << " as " << xname
+ << ":" << endl
+ << indent() << indent_str() << "result." << xname << " = " << xname << endl;
}
}
+ f_service_ << indent() << "except TTransport.TTransportException:" << endl
+ << indent() << indent_str() << "raise" << endl;
+ if (!tfunction->is_oneway()) {
+ f_service_ << indent() << "except TApplicationException as ex:" << endl
+ << indent() << indent_str()
+ << "logging.exception('TApplication exception in handler')" << endl
+ << indent() << indent_str() << "msg_type = TMessageType.EXCEPTION" << endl
+ << indent() << indent_str() << "result = ex" << endl
+ << indent() << "except Exception:" << endl
+ << indent() << indent_str()
+ << "logging.exception('Unexpected exception in handler')" << endl
+ << indent() << indent_str() << "msg_type = TMessageType.EXCEPTION" << endl
+ << indent() << indent_str()
+ << "result = TApplicationException(TApplicationException.INTERNAL_ERROR, "
+ "'Internal error')"
+ << endl;
+ } else {
+ f_service_ << indent() << "except Exception:" << endl
+ << indent() << indent_str()
+ << "logging.exception('Exception in oneway handler')" << endl;
+ }
if (!tfunction->is_oneway()) {
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;
+ << "\", msg_type, seqid)" << endl
+ << indent() << "result.write(oprot)" << endl
+ << indent() << "oprot.writeMessageEnd()" << endl
+ << indent() << "oprot.trans.flush()" << endl;
}
// Close function
diff --git a/test/py.tornado/test_suite.py b/test/py.tornado/test_suite.py
index 32d1c2e..447fde6 100755
--- a/test/py.tornado/test_suite.py
+++ b/test/py.tornado/test_suite.py
@@ -21,8 +21,8 @@
import datetime
import glob
-import sys
import os
+import sys
import time
import unittest
@@ -40,8 +40,8 @@
from tornado.testing import AsyncTestCase, get_unused_port, gen_test
from thrift import TTornado
+from thrift.Thrift import TApplicationException
from thrift.protocol import TBinaryProtocol
-from thrift.transport.TTransport import TTransportException
from ThriftTest import ThriftTest
from ThriftTest.ttypes import Xception, Xtruct
@@ -55,6 +55,8 @@
pass
def testString(self, s):
+ if s == 'unexpected_error':
+ raise Exception(s)
return s
def testByte(self, b):
@@ -85,7 +87,7 @@
x.message = s
raise x
elif s == 'throw_undeclared':
- raise ValueError("foo")
+ raise ValueError('testing undeclared exception')
def testOneway(self, seconds):
start = time.time()
@@ -97,6 +99,7 @@
self.test_instance.io_loop.add_timeout(
datetime.timedelta(seconds=seconds),
fire_oneway)
+ raise Exception('testing exception in oneway method')
def testNest(self, thing):
return thing
@@ -187,10 +190,11 @@
self.assertEqual(y.i32_thing, -3)
self.assertEqual(y.i64_thing, -5)
+ @gen_test
def test_oneway(self):
- self.client.testOneway(0)
- start, end, seconds = self.wait(timeout=1)
- self.assertAlmostEqual(seconds, (end - start), places=3)
+ self.client.testOneway(1)
+ v = yield self.client.testI32(-1)
+ self.assertEqual(v, -1)
@gen_test
def test_map(self):
@@ -203,8 +207,6 @@
@gen_test
def test_exception(self):
- yield self.client.testException('Safe')
-
try:
yield self.client.testException('Xception')
except Xception as ex:
@@ -214,11 +216,13 @@
self.fail("should have gotten exception")
try:
yield self.client.testException('throw_undeclared')
- except TTransportException as ex:
+ except TApplicationException:
pass
else:
self.fail("should have gotten exception")
+ yield self.client.testException('Safe')
+
def suite():
suite = unittest.TestSuite()