From 7e056e7572c642bd7f5d5677261763f05a17e1dd Mon Sep 17 00:00:00 2001 From: Roger Meier Date: Sun, 17 Jul 2011 07:28:28 +0000 Subject: [PATCH] THRIFT-1222 Unhandled exception for TEvhttpServer request Patch: Alexandre Parenteau git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1147542 13f79535-47bb-0310-9956-ffa450edef68 --- compiler/cpp/src/generate/t_cpp_generator.cc | 14 +- lib/cpp/Makefile.am | 6 +- lib/cpp/src/async/TEvhttpClientChannel.cpp | 1 + lib/cpp/src/async/TEvhttpServer.cpp | 10 +- test/cpp/Makefile.am | 3 +- test/cpp/src/TestClient.cpp | 55 ++++++- test/cpp/src/TestServer.cpp | 150 ++++++++++++++++++- 7 files changed, 226 insertions(+), 13 deletions(-) mode change 100644 => 100755 compiler/cpp/src/generate/t_cpp_generator.cc mode change 100644 => 100755 test/cpp/src/TestServer.cpp diff --git a/compiler/cpp/src/generate/t_cpp_generator.cc b/compiler/cpp/src/generate/t_cpp_generator.cc old mode 100644 new mode 100755 index e8feb424e..389bbf900 --- a/compiler/cpp/src/generate/t_cpp_generator.cc +++ b/compiler/cpp/src/generate/t_cpp_generator.cc @@ -231,28 +231,28 @@ class t_cpp_generator : public t_oop_generator { std::string get_include_prefix(const t_program& program) const; /** - * True iff we should generate pure enums for Thrift enums, instead of wrapper classes. + * True if we should generate pure enums for Thrift enums, instead of wrapper classes. */ bool gen_pure_enums_; /** - * True iff we should generate local reflection metadata for TDenseProtocol. + * True if we should generate local reflection metadata for TDenseProtocol. */ bool gen_dense_; /** - * True iff we should generate templatized reader/writer methods. + * True if we should generate templatized reader/writer methods. */ bool gen_templates_; /** - * True iff we should use a path prefix in our #include statements for other + * True if we should use a path prefix in our #include statements for other * thrift-generated header files. */ bool use_include_prefix_; /** - * True iff we should generate "Continuation OBject"-style classes as well. + * True if we should generate "Continuation OBject"-style classes as well. */ bool gen_cob_style_; @@ -4142,6 +4142,10 @@ string t_cpp_generator::get_include_prefix(const t_program& program) const { THRIFT_REGISTER_GENERATOR(cpp, "C++", +" cob_style: Generate \"Continuation OBject\"-style classes.\n" +" no_client_completion:\n" +" Omit calls to completion__() in CobClient class.\n" +" templates: Generate templatized reader/writer methods.\n" " pure_enums: Generate pure enums instead of wrapper classes.\n" " dense: Generate type specifications for the dense protocol.\n" " include_prefix: Use full include paths in generated files.\n" diff --git a/lib/cpp/Makefile.am b/lib/cpp/Makefile.am index 253b68e8c..35bc8a892 100644 --- a/lib/cpp/Makefile.am +++ b/lib/cpp/Makefile.am @@ -70,11 +70,13 @@ libthrift_la_SOURCES = src/Thrift.cpp \ src/server/TSimpleServer.cpp \ src/server/TThreadPoolServer.cpp \ src/server/TThreadedServer.cpp \ - src/async/TAsyncChannel.cpp \ + src/async/TAsyncChannel.cpp \ src/processor/PeekProcessor.cpp libthriftnb_la_SOURCES = src/server/TNonblockingServer.cpp \ - src/async/TAsyncProtocolProcessor.cpp + src/async/TAsyncProtocolProcessor.cpp \ + src/async/TEvhttpServer.cpp \ + src/async/TEvhttpClientChannel.cpp libthriftz_la_SOURCES = src/transport/TZlibTransport.cpp diff --git a/lib/cpp/src/async/TEvhttpClientChannel.cpp b/lib/cpp/src/async/TEvhttpClientChannel.cpp index 54676a118..8b4717369 100644 --- a/lib/cpp/src/async/TEvhttpClientChannel.cpp +++ b/lib/cpp/src/async/TEvhttpClientChannel.cpp @@ -19,6 +19,7 @@ #include "TEvhttpClientChannel.h" #include +#include "transport/TBufferTransports.h" namespace apache { namespace thrift { namespace async { diff --git a/lib/cpp/src/async/TEvhttpServer.cpp b/lib/cpp/src/async/TEvhttpServer.cpp index 299759713..30e33e52c 100644 --- a/lib/cpp/src/async/TEvhttpServer.cpp +++ b/lib/cpp/src/async/TEvhttpServer.cpp @@ -22,6 +22,10 @@ #include "transport/TBufferTransports.h" #include +#ifndef HTTP_INTERNAL // libevent < 2 +#define HTTP_INTERNAL 500 +#endif + using apache::thrift::transport::TMemoryBuffer; namespace apache { namespace thrift { namespace async { @@ -98,7 +102,11 @@ TEvhttpServer::RequestContext::RequestContext(struct evhttp_request* req) : req( void TEvhttpServer::request(struct evhttp_request* req, void* self) { - static_cast(self)->process(req); + try { + static_cast(self)->process(req); + } catch(std::exception& e) { + evhttp_send_reply(req, HTTP_INTERNAL, e.what(), 0); + } } diff --git a/test/cpp/Makefile.am b/test/cpp/Makefile.am index a828e5797..6c62cfbcf 100755 --- a/test/cpp/Makefile.am +++ b/test/cpp/Makefile.am @@ -87,7 +87,7 @@ StressTestNonBlocking_LDADD = \ THRIFT = $(top_builddir)/compiler/cpp/thrift gen-cpp/ThriftTest.cpp gen-cpp/ThriftTest_types.cpp gen-cpp/ThriftTest_constants.cpp: $(top_srcdir)/test/ThriftTest.thrift - $(THRIFT) --gen cpp:templates $< + $(THRIFT) --gen cpp:templates,cob_style -r $< gen-cpp/ThriftTest.cpp gen-cpp/StressTest_types.cpp gen-cpp/StressTest_constants.cpp: $(top_srcdir)/test/StressTest.thrift $(THRIFT) --gen cpp $< @@ -97,6 +97,7 @@ INCLUDES = \ AM_CPPFLAGS = $(BOOST_CPPFLAGS) AM_CXXFLAGS = -Wall +AM_LDFLAGS = $(BOOST_LDFLAGS) clean-local: $(RM) -r gen-cpp diff --git a/test/cpp/src/TestClient.cpp b/test/cpp/src/TestClient.cpp index ceea8389d..3f22ad225 100644 --- a/test/cpp/src/TestClient.cpp +++ b/test/cpp/src/TestClient.cpp @@ -29,9 +29,12 @@ #include #include #include +#include +#include // #include #include +#include #include "ThriftTest.h" @@ -41,6 +44,9 @@ using namespace apache::thrift; using namespace apache::thrift::protocol; using namespace apache::thrift::transport; using namespace thrift::test; +using namespace apache::thrift::async; + +using std::tr1::placeholders::_1; //extern uint32_t g_socket_syscalls; @@ -56,6 +62,33 @@ uint64_t now() return ret; } +static void testString_clientReturn(const char* host, int port, event_base *base, TProtocolFactory* protocolFactory, ThriftTestCobClient* client) { + try { + string s; + client->recv_testString(s); + cout << "testString: " << s << endl; + } catch (TException& exn) { + cout << "Error: " << exn.what() << endl; + } + + event_base_loopbreak(base); // end test +} + +static void testVoid_clientReturn(const char* host, int port, event_base *base, TProtocolFactory* protocolFactory, ThriftTestCobClient* client) { + try { + client->recv_testVoid(); + cout << "testVoid" << endl; + + // next test + delete client; + shared_ptr channel(new TEvhttpClientChannel(host, "/", host, port, base)); + client = new ThriftTestCobClient(channel, protocolFactory); + client->testString(tr1::bind(testString_clientReturn, host, port, base, protocolFactory, _1), "Test"); + } catch (TException& exn) { + cout << "Error: " << exn.what() << endl; + } +} + int main(int argc, char** argv) { string host = "localhost"; int port = 9090; @@ -71,7 +104,7 @@ int main(int argc, char** argv) { ("host", program_options::value(&host)->default_value(host), "Host to connect") ("port", program_options::value(&port)->default_value(port), "Port number to connect") ("domain-socket", program_options::value(&domain_socket)->default_value(domain_socket), "Domain Socket (e.g. /tmp/ThriftTest.thrift), instead of host and port") - ("transport", program_options::value(&transport_type)->default_value(transport_type), "Transport: buffered, framed, http") + ("transport", program_options::value(&transport_type)->default_value(transport_type), "Transport: buffered, framed, http, evhttp") ("protocol", program_options::value(&protocol_type)->default_value(protocol_type), "Protocol: binary, json") ("ssl", "Encrypted Transport using SSL") ("testloops,n", program_options::value(&numTests)->default_value(numTests), "Number of Tests") @@ -99,6 +132,7 @@ int main(int argc, char** argv) { if (transport_type == "buffered") { } else if (transport_type == "framed") { } else if (transport_type == "http") { + } else if (transport_type == "evhttp") { } else { throw invalid_argument("Unknown transport type "+transport_type); } @@ -162,6 +196,25 @@ int main(int argc, char** argv) { } cout << endl; + if (transport_type.compare("evhttp") == 0) { + event_base *base = event_base_new(); + cout << "Libevent Version: " << event_get_version() << endl; + cout << "Libevent Method: " << event_base_get_method(base) << endl; +#if LIBEVENT_VERSION_NUMBER >= 0x02000000 + cout << "Libevent Features: 0x" << hex << event_base_get_features(base) << endl; +#endif + + shared_ptr protocolFactory(new TBinaryProtocolFactory()); + + shared_ptr channel(new TEvhttpClientChannel(host.c_str(), "/", host.c_str(), port, base)); + ThriftTestCobClient* client = new ThriftTestCobClient(channel, protocolFactory.get()); + client->testVoid(tr1::bind(testVoid_clientReturn, host.c_str(), port, base, protocolFactory.get(), _1)); + + event_base_loop(base, 0); + return 0; + } + + ThriftTestClient testClient(protocol); uint64_t time_min = 0; diff --git a/test/cpp/src/TestServer.cpp b/test/cpp/src/TestServer.cpp old mode 100644 new mode 100755 index 9ba05098a..2c9d43a48 --- a/test/cpp/src/TestServer.cpp +++ b/test/cpp/src/TestServer.cpp @@ -27,6 +27,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -52,6 +55,7 @@ using namespace apache::thrift::concurrency; using namespace apache::thrift::protocol; using namespace apache::thrift::transport; using namespace apache::thrift::server; +using namespace apache::thrift::async; using namespace thrift::test; @@ -345,6 +349,137 @@ class TestProcessorEventHandler : public TProcessorEventHandler { }; +class TestHandlerAsync : public ThriftTestCobSvIf { +public: + TestHandlerAsync(shared_ptr& handler) : _delegate(handler) {} + virtual ~TestHandlerAsync() {} + + virtual void testVoid(std::tr1::function cob) { + _delegate->testVoid(); + cob(); + } + + virtual void testString(std::tr1::function cob, const std::string& thing) { + std::string res; + _delegate->testString(res, thing); + cob(res); + } + + virtual void testByte(std::tr1::function cob, const int8_t thing) { + int8_t res = _delegate->testByte(thing); + cob(res); + } + + virtual void testI32(std::tr1::function cob, const int32_t thing) { + int32_t res = _delegate->testI32(thing); + cob(res); + } + + virtual void testI64(std::tr1::function cob, const int64_t thing) { + int64_t res = _delegate->testI64(thing); + cob(res); + } + + virtual void testDouble(std::tr1::function cob, const double thing) { + double res = _delegate->testDouble(thing); + cob(res); + } + + virtual void testStruct(std::tr1::function cob, const Xtruct& thing) { + Xtruct res; + _delegate->testStruct(res, thing); + cob(res); + } + + virtual void testNest(std::tr1::function cob, const Xtruct2& thing) { + Xtruct2 res; + _delegate->testNest(res, thing); + cob(res); + } + + virtual void testMap(std::tr1::function const& _return)> cob, const std::map & thing) { + std::map res; + _delegate->testMap(res, thing); + cob(res); + } + + virtual void testStringMap(std::tr1::function const& _return)> cob, const std::map & thing) { + std::map res; + _delegate->testStringMap(res, thing); + cob(res); + } + + virtual void testSet(std::tr1::function const& _return)> cob, const std::set & thing) { + std::set res; + _delegate->testSet(res, thing); + cob(res); + } + + virtual void testList(std::tr1::function const& _return)> cob, const std::vector & thing) { + std::vector res; + _delegate->testList(res, thing); + cob(res); + } + + virtual void testEnum(std::tr1::function cob, const Numberz::type thing) { + Numberz::type res = _delegate->testEnum(thing); + cob(res); + } + + virtual void testTypedef(std::tr1::function cob, const UserId thing) { + UserId res = _delegate->testTypedef(thing); + cob(res); + } + + virtual void testMapMap(std::tr1::function > const& _return)> cob, const int32_t hello) { + std::map > res; + _delegate->testMapMap(res, hello); + cob(res); + } + + virtual void testInsanity(std::tr1::function > const& _return)> cob, const Insanity& argument) { + std::map > res; + _delegate->testInsanity(res, argument); + cob(res); + } + + virtual void testMulti(std::tr1::function cob, const int8_t arg0, const int32_t arg1, const int64_t arg2, const std::map & arg3, const Numberz::type arg4, const UserId arg5) { + Xtruct res; + _delegate->testMulti(res, arg0, arg1, arg2, arg3, arg4, arg5); + cob(res); + } + + virtual void testException(std::tr1::function cob, std::tr1::function exn_cob, const std::string& arg) { + try { + _delegate->testException(arg); + } catch(const apache::thrift::TException& e) { + exn_cob(apache::thrift::TDelayedException::delayException(e)); + return; + } + cob(); + } + + virtual void testMultiException(std::tr1::function cob, std::tr1::function exn_cob, const std::string& arg0, const std::string& arg1) { + Xtruct res; + try { + _delegate->testMultiException(res, arg0, arg1); + } catch(const apache::thrift::TException& e) { + exn_cob(apache::thrift::TDelayedException::delayException(e)); + return; + } + cob(res); + } + + virtual void testOneway(std::tr1::function cob, const int32_t secondsToSleep) { + _delegate->testOneway(secondsToSleep); + cob(); + } + +protected: + shared_ptr _delegate; +}; + + int main(int argc, char **argv) { int port = 9090; bool ssl = false; @@ -464,7 +599,7 @@ int main(int argc, char **argv) { // Factory shared_ptr transportFactory; - if (transport_type == "http") { + if (transport_type == "http" && server_type != "nonblocking") { shared_ptr httpTransportFactory(new THttpServerTransportFactory()); transportFactory = httpTransportFactory; } else if (transport_type == "framed") { @@ -522,8 +657,17 @@ int main(int argc, char **argv) { threadedServer.serve(); } else if (server_type == "nonblocking") { - TNonblockingServer nonblockingServer(testProcessor, port); - nonblockingServer.serve(); + if(transport_type == "http") { + shared_ptr testHandlerAsync(new TestHandlerAsync(testHandler)); + shared_ptr testProcessorAsync(new ThriftTestAsyncProcessor(testHandlerAsync)); + shared_ptr testBufferProcessor(new TAsyncProtocolProcessor(testProcessorAsync, protocolFactory)); + + TEvhttpServer nonblockingServer(testBufferProcessor, port); + nonblockingServer.serve(); +} else { + TNonblockingServer nonblockingServer(testProcessor, port); + nonblockingServer.serve(); + } } cout << "done." << endl;