You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ra...@apache.org on 2015/02/27 02:03:46 UTC
thrift git commit: THRIFT-3008: Node.js server does not fully support
exceptions Client: Node.js Patch: Nobuaki Sukegawa
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 <ns...@gmail.com>
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 <ra...@apache.org>
Authored: Thu Feb 26 16:59:14 2015 -0800
Committer: Randy Abernethy <ra...@apache.org>
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) {