Repository: thrift Updated Branches: refs/heads/master 2ad6c307b -> bd60b92c6
THRIFT-3008: Node.js server does not fully support exceptions Client: Node.js Patch: Nobuaki Sukegawa Github Pull Request: This closes #382 commit 0c0d51ca1dafa5f8e0004563df780a92580590f3 Author: Nobuaki Sukegawa <[email protected]> Date: 2015-02-22T16:49:22Z THRIFT-3008 - Node.js server does not fully support exception Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/bd60b92c Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/bd60b92c Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/bd60b92c Branch: refs/heads/master Commit: bd60b92c6f31c871d5bd52debbe75394575cd786 Parents: 2ad6c30 Author: Randy Abernethy <[email protected]> Authored: Thu Feb 26 16:59:14 2015 -0800 Committer: Randy Abernethy <[email protected]> Committed: Thu Feb 26 16:59:14 2015 -0800 ---------------------------------------------------------------------- compiler/cpp/src/generate/t_js_generator.cc | 77 +++++++++++++++++++++--- lib/nodejs/test/test-cases.js | 6 -- lib/nodejs/test/test_driver.js | 22 +++++-- lib/nodejs/test/test_handler.js | 8 +-- 4 files changed, 90 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/bd60b92c/compiler/cpp/src/generate/t_js_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/generate/t_js_generator.cc b/compiler/cpp/src/generate/t_js_generator.cc index 755e7ae..970a179 100644 --- a/compiler/cpp/src/generate/t_js_generator.cc +++ b/compiler/cpp/src/generate/t_js_generator.cc @@ -1022,11 +1022,50 @@ void t_js_generator::generate_process_function(t_service* tservice, t_function* indent_down(); indent(f_service_) << "}, function (err) {" << endl; indent_up(); - f_service_ << indent() << "var result = new " << resultname << "(err);" << endl << indent() - << "output.writeMessageBegin(\"" << tfunction->get_name() - << "\", Thrift.MessageType.REPLY, seqid);" << endl << indent() - << "result.write(output);" << endl << indent() << "output.writeMessageEnd();" << endl - << indent() << "output.flush();" << endl; + + bool has_exception = false; + t_struct* exceptions = tfunction->get_xceptions(); + if (exceptions) { + const vector<t_field*>& members = exceptions->get_members(); + for (vector<t_field*>::const_iterator it = members.begin(); it != members.end(); ++it) { + t_type* t = get_true_type((*it)->get_type()); + if (t->is_xception()) { + if (!has_exception) { + has_exception = true; + indent(f_service_) << "if (err instanceof " << js_type_namespace(t->get_program()) + << t->get_name(); + } else { + f_service_ << " || err instanceof " << js_type_namespace(t->get_program()) + << t->get_name(); + } + } + } + } + + if (has_exception) { + f_service_ << ") {" << endl; + indent_up(); + f_service_ << indent() << "var result = new " << resultname << "(err);" << endl << indent() + << "output.writeMessageBegin(\"" << tfunction->get_name() + << "\", Thrift.MessageType.REPLY, seqid);" << endl; + + indent_down(); + indent(f_service_) << "} else {" << endl; + indent_up(); + } + + f_service_ << indent() << "var result = new " + "Thrift.TApplicationException(Thrift.TApplicationExceptionType.UNKNOWN," + " err.message);" << endl << indent() << "output.writeMessageBegin(\"" + << tfunction->get_name() << "\", Thrift.MessageType.EXCEPTION, seqid);" << endl; + + if (has_exception) { + indent_down(); + indent(f_service_) << "}" << endl; + } + + f_service_ << indent() << "result.write(output);" << endl << indent() + << "output.writeMessageEnd();" << endl << indent() << "output.flush();" << endl; indent_down(); indent(f_service_) << "});" << endl; indent_down(); @@ -1039,15 +1078,35 @@ void t_js_generator::generate_process_function(t_service* tservice, t_function* f_service_ << "args." << (*f_iter)->get_name() << ", "; } - f_service_ << " function (err, result) {" << endl; + f_service_ << "function (err, result) {" << endl; indent_up(); + indent(f_service_) << "if (err == null"; + if (has_exception) { + const vector<t_field*>& members = exceptions->get_members(); + for (vector<t_field*>::const_iterator it = members.begin(); it != members.end(); ++it) { + t_type* t = get_true_type((*it)->get_type()); + if (t->is_xception()) { + f_service_ << " || err instanceof " << js_type_namespace(t->get_program()) << t->get_name(); + } + } + } + f_service_ << ") {" << endl; + indent_up(); f_service_ << indent() << "var result = new " << resultname << "((err != null ? err : {success: result}));" << endl << indent() << "output.writeMessageBegin(\"" << tfunction->get_name() - << "\", Thrift.MessageType.REPLY, seqid);" << endl << indent() - << "result.write(output);" << endl << indent() << "output.writeMessageEnd();" << endl - << indent() << "output.flush();" << endl; + << "\", Thrift.MessageType.REPLY, seqid);" << endl; + indent_down(); + indent(f_service_) << "} else {" << endl; + indent_up(); + f_service_ << indent() << "var result = new " + "Thrift.TApplicationException(Thrift.TApplicationExceptionType.UNKNOWN," + " err.message);" << endl << indent() << "output.writeMessageBegin(\"" + << tfunction->get_name() << "\", Thrift.MessageType.EXCEPTION, seqid);" << endl; + indent_down(); + f_service_ << indent() << "}" << endl << indent() << "result.write(output);" << endl << indent() + << "output.writeMessageEnd();" << endl << indent() << "output.flush();" << endl; indent_down(); indent(f_service_) << "});" << endl; http://git-wip-us.apache.org/repos/asf/thrift/blob/bd60b92c/lib/nodejs/test/test-cases.js ---------------------------------------------------------------------- diff --git a/lib/nodejs/test/test-cases.js b/lib/nodejs/test/test-cases.js index 0d13cdd..c396ca9 100644 --- a/lib/nodejs/test/test-cases.js +++ b/lib/nodejs/test/test-cases.js @@ -73,16 +73,10 @@ for (var i = 0; i < 5; ++i) { mapout[i] = i-10; } -var mapMapTest = { - "4": {"1":1, "2":2, "3":3, "4":4}, - "-4": {"-4":-4, "-3":-3, "-2":-2, "-1":-1} -}; - var deep = [ ['testMap', mapout], ['testSet', [1,2,3]], ['testList', [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20]], - ['testMapMap', mapMapTest], ['testStringMap', mapTestInput] ]; http://git-wip-us.apache.org/repos/asf/thrift/blob/bd60b92c/lib/nodejs/test/test_driver.js ---------------------------------------------------------------------- diff --git a/lib/nodejs/test/test_driver.js b/lib/nodejs/test/test_driver.js index f79baa6..6e472ad 100644 --- a/lib/nodejs/test/test_driver.js +++ b/lib/nodejs/test/test_driver.js @@ -29,6 +29,7 @@ var test = require('tape'); //var assert = require('assert'); var ttypes = require('./gen-nodejs/ThriftTest_types'); +var TException = require('thrift').Thrift.TException; var Int64 = require('node-int64'); var testCases = require('./test-cases'); @@ -55,6 +56,15 @@ exports.ThriftTestDriver = function(client, callback) { })); testCases.deep.forEach(makeAsserter(assert.deepEqual)); + client.testMapMap(42, function(err, response) { + var expected = { + "4": {"1":1, "2":2, "3":3, "4":4}, + "-4": {"-4":-4, "-3":-3, "-2":-2, "-1":-1} + }; + assert.error(err, 'testMapMap: no callback error'); + assert.deepEqual(expected, response, 'testMapMap'); + }); + client.testStruct(testCases.out, function(err, response) { assert.error(err, 'testStruct: no callback error'); checkRecursively(testCases.out, response, 'testStruct'); @@ -71,11 +81,12 @@ exports.ThriftTestDriver = function(client, callback) { }); client.testException('TException', function(err, response) { - assert.error(err, 'testException: no callback error'); + assert.ok(err instanceof TException, 'testException: correct error type'); assert.ok(!response, 'testException: no response'); }); client.testException('Xception', function(err, response) { + assert.ok(err instanceof ttypes.Xception, 'testException: correct error type'); assert.ok(!response, 'testException: no response'); assert.equal(err.errorCode, 1001, 'testException: correct error code'); assert.equal('Xception', err.message, 'testException: correct error message'); @@ -152,15 +163,18 @@ exports.ThriftTestDriverPromise = function(client, callback) { client.testException('TException') .then(function(response) { - assert.ok(!response, 'testException: TException'); + fail('testException: TException'); }) - .fail(fail('testException: TException')); + .fail(function(err) { + assert.ok(err instanceof TException); + }); client.testException('Xception') .then(function(response) { - assert.ok(!response); + fail('testException: Xception'); }) .fail(function(err) { + assert.ok(err instanceof ttypes.Xception); assert.equal(err.errorCode, 1001); assert.equal('Xception', err.message); }); http://git-wip-us.apache.org/repos/asf/thrift/blob/bd60b92c/lib/nodejs/test/test_handler.js ---------------------------------------------------------------------- diff --git a/lib/nodejs/test/test_handler.js b/lib/nodejs/test/test_handler.js index de6f503..da32906 100644 --- a/lib/nodejs/test/test_handler.js +++ b/lib/nodejs/test/test_handler.js @@ -213,11 +213,11 @@ function testMultiExceptionAsync(arg0, arg1, result) { x2.struct_thing = new ttypes.Xtruct(); x2.struct_thing.string_thing = 'This is an Xception2'; result(x2); + } else { + var res = new ttypes.Xtruct(); + res.string_thing = arg1; + result(null, res); } - - var res = new ttypes.Xtruct(); - res.string_thing = arg1; - result(null, res); } function testOneway(sleepFor) {
