You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2017/11/30 17:37:33 UTC
[1/3] thrift git commit: THRIFT-3600 Make TTwisted server send
exception on unexpected handler error Client: py
Repository: thrift
Updated Branches:
refs/heads/master 1ce7a5b45 -> af5628637
THRIFT-3600 Make TTwisted server send exception on unexpected handler error
Client: py
This closes #838
This closes #1424
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/6f826403
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/6f826403
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/6f826403
Branch: refs/heads/master
Commit: 6f8264037c138b5042cbdbe8a444c2cff6065ee6
Parents: 1ce7a5b
Author: Nobuaki Sukegawa <ns...@apache.org>
Authored: Wed Feb 10 19:42:19 2016 +0900
Committer: James E. King, III <jk...@apache.org>
Committed: Thu Nov 30 12:36:11 2017 -0500
----------------------------------------------------------------------
.../cpp/src/thrift/generate/t_py_generator.cc | 112 +++++++++++--------
test/py.twisted/test_suite.py | 16 ++-
2 files changed, 75 insertions(+), 53 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/6f826403/compiler/cpp/src/thrift/generate/t_py_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_py_generator.cc b/compiler/cpp/src/thrift/generate/t_py_generator.cc
index 23d4b70..8851c31 100644
--- a/compiler/cpp/src/thrift/generate/t_py_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_py_generator.cc
@@ -1898,8 +1898,6 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
}
if (gen_twisted_) {
- // 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();
@@ -1918,65 +1916,83 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
}
f_service_ << ")" << endl;
- // Shortcut out here for oneway functions
if (tfunction->is_oneway()) {
- f_service_ << indent() << "return d" << endl;
- indent_down();
- f_service_ << endl;
- return;
- }
-
- f_service_ << indent() << "d.addCallback(self.write_results_success_" << tfunction->get_name()
- << ", result, seqid, oprot)" << endl;
-
- if (xceptions.size() > 0) {
- f_service_ << indent() << "d.addErrback(self.write_results_exception_"
+ f_service_ << indent() << "d.addErrback(self.handle_exception_" << tfunction->get_name()
+ << ", seqid)" << endl;
+ } else {
+ f_service_ << indent() << "d.addCallback(self.write_results_success_" << tfunction->get_name()
+ << ", result, seqid, oprot)" << endl
+ << indent() << "d.addErrback(self.write_results_exception_"
<< tfunction->get_name() << ", result, seqid, oprot)" << endl;
}
-
- f_service_ << indent() << "return d" << endl;
+ f_service_ << indent() << "return d" << endl << endl;
indent_down();
- f_service_ << endl;
- indent(f_service_) << "def write_results_success_" << tfunction->get_name()
- << "(self, success, result, seqid, oprot):" << endl;
- indent_up();
- f_service_ << indent() << "result.success = success" << endl << indent()
- << "oprot.writeMessageBegin(\"" << tfunction->get_name()
- << "\", TMessageType.REPLY, seqid)" << endl << indent() << "result.write(oprot)"
- << endl << indent() << "oprot.writeMessageEnd()" << endl << indent()
- << "oprot.trans.flush()" << endl;
- indent_down();
+ if (tfunction->is_oneway()) {
+ indent(f_service_) << "def handle_exception_" << tfunction->get_name()
+ << "(self, error, seqid):" << endl;
+ } else {
+ indent(f_service_) << "def write_results_success_" << tfunction->get_name()
+ << "(self, success, result, seqid, oprot):" << endl;
+ indent_up();
+ f_service_ << indent() << "result.success = success" << endl
+ << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name()
+ << "\", TMessageType.REPLY, seqid)" << endl
+ << indent() << "result.write(oprot)" << endl
+ << indent() << "oprot.writeMessageEnd()" << endl
+ << indent() << "oprot.trans.flush()" << endl
+ << endl;
+ indent_down();
- // Try block for a function with exceptions
- if (!tfunction->is_oneway() && xceptions.size() > 0) {
- f_service_ << endl;
indent(f_service_) << "def write_results_exception_" << tfunction->get_name()
<< "(self, error, result, seqid, oprot):" << endl;
- indent_up();
- f_service_ << indent() << "try:" << endl;
+ }
+ indent_up();
+ if (!tfunction->is_oneway()) {
+ f_service_ << indent() << "msg_type = TMessageType.REPLY" << endl;
+ }
+ f_service_ << indent() << "try:" << endl;
- // Kinda absurd
- f_service_ << indent() << indent_str() << "error.raiseException()" << endl;
+ // Kinda absurd
+ f_service_ << indent() << indent_str() << "error.raiseException()" << endl;
+ 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_up();
+ f_service_ << indent() << "result." << xname << " = " << xname << endl;
+ indent_down();
}
- 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;
- indent_down();
}
+ 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
+ << indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name()
+ << "\", msg_type, seqid)" << endl
+ << indent() << "result.write(oprot)" << endl
+ << indent() << "oprot.writeMessageEnd()" << endl
+ << indent() << "oprot.trans.flush()" << endl;
+ } else {
+ f_service_ << indent() << "except Exception:" << endl
+ << indent() << indent_str()
+ << "logging.exception('Exception in oneway handler')" << endl;
+ }
+ indent_down();
} else if (gen_tornado_) {
// TODO: Propagate arbitrary exception raised by handler to client as does plain "py"
http://git-wip-us.apache.org/repos/asf/thrift/blob/6f826403/test/py.twisted/test_suite.py
----------------------------------------------------------------------
diff --git a/test/py.twisted/test_suite.py b/test/py.twisted/test_suite.py
index 886de44..02eb7f1 100755
--- a/test/py.twisted/test_suite.py
+++ b/test/py.twisted/test_suite.py
@@ -19,14 +19,17 @@
# under the License.
#
-import sys
-import os
import glob
+import os
+import sys
import time
+
basepath = os.path.abspath(os.path.dirname(__file__))
sys.path.insert(0, os.path.join(basepath, 'gen-py.twisted'))
sys.path.insert(0, glob.glob(os.path.join(basepath, '../../lib/py/build/lib.*'))[0])
+from thrift.Thrift import TApplicationException
+
from ThriftTest import ThriftTest
from ThriftTest.ttypes import Xception, Xtruct
from thrift.transport import TTwisted
@@ -84,6 +87,7 @@ class TestHandler:
def fireOneway(t):
self.onewaysQueue.put((t, time.time(), seconds))
reactor.callLater(seconds, fireOneway, time.time())
+ raise Exception('')
def testNest(self, thing):
return thing
@@ -171,7 +175,6 @@ class ThriftTestCase(unittest.TestCase):
@defer.inlineCallbacks
def testException(self):
- yield self.client.testException('Safe')
try:
yield self.client.testException('Xception')
self.fail("should have gotten exception")
@@ -181,12 +184,15 @@ class ThriftTestCase(unittest.TestCase):
try:
yield self.client.testException("throw_undeclared")
- self.fail("should have thrown exception")
- except Exception: # type is undefined
+ self.fail("should have gotten exception")
+ except TApplicationException:
pass
+ yield self.client.testException('Safe')
+
@defer.inlineCallbacks
def testOneway(self):
yield self.client.testOneway(1)
start, end, seconds = yield self.handler.onewaysQueue.get()
self.assertAlmostEquals(seconds, (end - start), places=1)
+ self.assertEquals((yield self.client.testI32(-1)), -1)
[3/3] thrift git commit: THRIFT-3610 Streamline exception handling in
Python server handler Client: py
Posted by jk...@apache.org.
THRIFT-3610 Streamline exception handling in Python server handler
Client: py
This closes #842
This closes #1426
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/af562863
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/af562863
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/af562863
Branch: refs/heads/master
Commit: af562863781ee9a4e0f314470701f1ab2e3fbe33
Parents: 66c3dbf
Author: Nobuaki Sukegawa <ns...@apache.org>
Authored: Thu Feb 11 07:06:21 2016 +0900
Committer: James E. King, III <jk...@apache.org>
Committed: Thu Nov 30 12:36:52 2017 -0500
----------------------------------------------------------------------
.../cpp/src/thrift/generate/t_py_generator.cc | 50 +++++++++++---------
1 file changed, 27 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/af562863/compiler/cpp/src/thrift/generate/t_py_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_py_generator.cc b/compiler/cpp/src/thrift/generate/t_py_generator.cc
index 3b51714..9078da8 100644
--- a/compiler/cpp/src/thrift/generate/t_py_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_py_generator.cc
@@ -1513,12 +1513,13 @@ void t_py_generator::generate_service_client(t_service* tservice) {
const std::vector<t_field*>& xceptions = xs->get_members();
vector<t_field*>::const_iterator x_iter;
for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
- f_service_ << indent() << "if result." << (*x_iter)->get_name() << " is not None:" << endl;
+ const string& xname = (*x_iter)->get_name();
+ f_service_ << indent() << "if result." << xname << " is not None:" << endl;
if (gen_twisted_) {
- f_service_ << indent() << indent_str() << "return d.errback(result." << (*x_iter)->get_name() << ")"
+ f_service_ << indent() << indent_str() << "return d.errback(result." << xname << ")"
<< endl;
} else {
- f_service_ << indent() << indent_str() << "raise result." << (*x_iter)->get_name() << "" << endl;
+ f_service_ << indent() << indent_str() << "raise result." << xname << "" << endl;
}
}
@@ -2090,43 +2091,46 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
}
f_service_ << ")" << endl;
if (!tfunction->is_oneway()) {
- f_service_ << indent() << "msg_type = TMessageType.REPLY" << endl;
+ f_service_ << indent() << "msg_type = TMessageType.REPLY" << endl;
}
indent_down();
f_service_ << indent()
- << "except (TTransport.TTransportException, KeyboardInterrupt, SystemExit):" << endl
+ << "except TTransport.TTransportException:" << endl
<< indent() << indent_str() << "raise" << endl;
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() << "msg_type = TMessageType.REPLY" << endl;
- 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_up();
+ f_service_ << indent() << "msg_type = TMessageType.REPLY" << endl;
+ f_service_ << indent() << "result." << xname << " = " << xname << endl;
+ indent_down();
}
- f_service_ << indent() << "except Exception as ex:" << endl
+ 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() << "logging.exception(ex)" << endl
- << indent()
- << indent_str() << "result = TApplicationException(TApplicationException.INTERNAL_ERROR, "
- "'Internal error')" << endl
+ << indent() << indent_str()
+ << "result = TApplicationException(TApplicationException.INTERNAL_ERROR, "
+ "'Internal error')"
+ << endl
<< indent() << "oprot.writeMessageBegin(\"" << tfunction->get_name()
<< "\", msg_type, seqid)" << endl
<< indent() << "result.write(oprot)" << endl
<< indent() << "oprot.writeMessageEnd()" << endl
<< indent() << "oprot.trans.flush()" << endl;
} else {
- f_service_ << indent() << "except:" << endl
- << indent() << indent_str() << "pass" << endl;
+ f_service_ << indent() << "except Exception:" << endl
+ << indent() << indent_str() << "logging.exception('Exception in oneway handler')" << endl;
}
// Close function
[2/3] thrift git commit: THRIFT-3602 Make Tornado server send
exception on unexpected handler error Client: py
Posted by jk...@apache.org.
THRIFT-3602 Make Tornado server send exception on unexpected handler error
Client: py
This closes #839
This closes #1425
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/66c3dbf2
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/66c3dbf2
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/66c3dbf2
Branch: refs/heads/master
Commit: 66c3dbf2dec91718922e815c50a55900b78a58e1
Parents: 6f82640
Author: Nobuaki Sukegawa <ns...@apache.org>
Authored: Wed Feb 10 19:37:26 2016 +0900
Committer: James E. King, III <jk...@apache.org>
Committed: Thu Nov 30 12:36:33 2017 -0500
----------------------------------------------------------------------
.../cpp/src/thrift/generate/t_py_generator.cc | 55 +++++++++++++-------
test/py.tornado/test_suite.py | 22 ++++----
2 files changed, 48 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/66c3dbf2/compiler/cpp/src/thrift/generate/t_py_generator.cc
----------------------------------------------------------------------
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 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
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 @@ void t_py_generator::generate_process_function(t_service* tservice, t_function*
}
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
http://git-wip-us.apache.org/repos/asf/thrift/blob/66c3dbf2/test/py.tornado/test_suite.py
----------------------------------------------------------------------
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 import gen
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 @@ class TestHandler(object):
pass
def testString(self, s):
+ if s == 'unexpected_error':
+ raise Exception(s)
return s
def testByte(self, b):
@@ -85,7 +87,7 @@ class TestHandler(object):
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 @@ class TestHandler(object):
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 @@ class ThriftTestCase(AsyncTestCase):
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 @@ class ThriftTestCase(AsyncTestCase):
@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 @@ class ThriftTestCase(AsyncTestCase):
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()