Skip to content

Commit

Permalink
THRIFT-1267 Node.js can't throw exceptions
Browse files Browse the repository at this point in the history
Patch: Henrique Mendonca

git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1230797 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
bufferoverflow committed Jan 12, 2012
1 parent 0580d8d commit eaa61d8
Show file tree
Hide file tree
Showing 12 changed files with 499 additions and 35 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@
/test/hs/Makefile.in
/test/log/
/test/test.log
/test/nodejs/gen-nodejs/
/test/nodejs/Makefile
/test/nodejs/Makefile.in
/test/perl/gen-*
/test/perl/Makefile
/test/perl/Makefile.in
Expand Down
54 changes: 29 additions & 25 deletions compiler/cpp/src/generate/t_js_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -518,10 +518,10 @@ void t_js_generator::generate_js_struct_definition(ofstream& out,

if (gen_node_) {
if (is_exported) {
out << "var " << js_namespace(tstruct->get_program()) << tstruct->get_name() << " = " <<
"module.exports." << js_namespace(tstruct->get_program()) << tstruct->get_name() << " = function(args) {\n";
out << js_namespace(tstruct->get_program()) << tstruct->get_name() << " = " <<
"module.exports." << tstruct->get_name() << " = function(args) {\n";
} else {
out << "var " << js_namespace(tstruct->get_program()) << tstruct->get_name() << " = function(args) {\n";
out << js_namespace(tstruct->get_program()) << tstruct->get_name() << " = function(args) {\n";
}
} else {
out << js_namespace(tstruct->get_program()) << tstruct->get_name() <<" = function(args) {\n";
Expand Down Expand Up @@ -557,6 +557,17 @@ void t_js_generator::generate_js_struct_definition(ofstream& out,
}
}

// Early returns for exceptions
for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
t_type* t = get_true_type((*m_iter)->get_type());
if (t->is_xception()) {
out << indent() << "if (args instanceof " << js_type_namespace(t->get_program()) << t->get_name() << ") {" << endl
<< indent() << indent() << "this." << (*m_iter)->get_name() << " = args;" << endl
<< indent() << indent() << "return;" << endl
<< indent() << "}" << endl;
}
}

out << indent() << "if (args) {" << endl;

for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
Expand Down Expand Up @@ -736,19 +747,18 @@ void t_js_generator::generate_service(t_service* tservice) {
render_includes() << endl;

if (gen_node_) {
if (tservice->get_extends() != NULL) {
f_service_ <<
"var " << tservice->get_extends()->get_name() <<
" = require('./" << tservice->get_extends()->get_name() << "')" << endl <<
"var " << tservice->get_extends()->get_name() << "Client = " <<
tservice->get_extends()->get_name() << ".Client" << endl <<
"var " << tservice->get_extends()->get_name() << "Processor = " <<
tservice->get_extends()->get_name() << ".Processor" << endl;

}

if (tservice->get_extends() != NULL) {
f_service_ <<
"var ttypes = require('./" + program_->get_name() + "_types');" << endl;
"var " << tservice->get_extends()->get_name() <<
" = require('./" << tservice->get_extends()->get_name() << "')" << endl <<
"var " << tservice->get_extends()->get_name() << "Client = " <<
tservice->get_extends()->get_name() << ".Client" << endl <<
"var " << tservice->get_extends()->get_name() << "Processor = " <<
tservice->get_extends()->get_name() << ".Processor" << endl;
}

f_service_ <<
"var ttypes = require('./" + program_->get_name() + "_types');" << endl;
}

generate_service_helpers(tservice);
Expand All @@ -772,7 +782,7 @@ void t_js_generator::generate_service_processor(t_service* tservice) {
vector<t_function*>::iterator f_iter;

f_service_ <<
"var " << js_namespace(tservice->get_program()) << service_name_ << "Processor = " <<
js_namespace(tservice->get_program()) << service_name_ << "Processor = " <<
"exports.Processor = function(handler) ";

scope_up(f_service_);
Expand Down Expand Up @@ -836,12 +846,6 @@ void t_js_generator::generate_process_function(t_service* tservice,
indent() << "args.read(input);" << endl <<
indent() << "input.readMessageEnd();" << endl;

// Declare result for non oneway function
if (!tfunction->is_oneway()) {
f_service_ <<
indent() << "var result = new " << resultname << "();" << endl;
}

// Generate the function call
t_struct* arg_struct = tfunction->get_arglist();
const std::vector<t_field*>& fields = arg_struct->get_members();
Expand Down Expand Up @@ -871,11 +875,11 @@ void t_js_generator::generate_process_function(t_service* tservice,
if (!first) {
f_service_ << ", ";
}
f_service_ << "function (success) {" << endl;
f_service_ << "function (err, result) {" << endl;
indent_up();

f_service_ <<
indent() << "result.success = success;" << endl <<
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 <<
Expand Down Expand Up @@ -959,7 +963,7 @@ void t_js_generator::generate_service_client(t_service* tservice) {

if (gen_node_) {
f_service_ <<
"var " << js_namespace(tservice->get_program()) << service_name_ << "Client = " <<
js_namespace(tservice->get_program()) << service_name_ << "Client = " <<
"exports.Client = function(output, pClass) {"<<endl;
} else {
f_service_ <<
Expand Down
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ AC_CONFIG_FILES([
test/Makefile
test/cpp/Makefile
test/hs/Makefile
test/nodejs/Makefile
test/perl/Makefile
test/py/Makefile
test/py.twisted/Makefile
Expand Down
8 changes: 7 additions & 1 deletion lib/nodejs/examples/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
ALL:
all:
../../../compiler/cpp/thrift --gen js:node user.thrift

server: all
NODE_PATH=../lib:../lib/thrift node server.js

client: all
NODE_PATH=../lib:../lib/thrift node client.js
8 changes: 4 additions & 4 deletions lib/nodejs/examples/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ var UserStorage = require('./gen-nodejs/UserStorage.js'),
var users = {};

var server = thrift.createServer(UserStorage, {
store: function(user, success) {
store: function(user, result) {
console.log("server stored:", user.uid);
users[user.uid] = user;
success();
result(null);
},

retrieve: function(uid, success) {
retrieve: function(uid, result) {
console.log("server retrieved:", uid);
success(users[uid]);
result(null, users[uid]);
},
});

Expand Down
8 changes: 4 additions & 4 deletions lib/nodejs/examples/server_multitransport.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ var UserStorage = require('./gen-nodejs/UserStorage'),

var users = {};

var store = function(user, success) {
var store = function(user, result) {
console.log("stored:", user.uid);
users[user.uid] = user;
success();
result(null);
};
var retrieve = function(uid, success) {
var retrieve = function(uid, result) {
console.log("retrieved:", uid);
success(users[uid]);
result(null, users[uid]);
};

var server_framed = thrift.createServer(UserStorage, {
Expand Down
3 changes: 2 additions & 1 deletion test/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# under the License.
#

SUBDIRS =
SUBDIRS = nodejs

if WITH_CPP
SUBDIRS += cpp
Expand Down Expand Up @@ -45,6 +45,7 @@ EXTRA_DIST = \
csharp \
erl \
hs \
nodejs \
ocaml \
perl \
php \
Expand Down
33 changes: 33 additions & 0 deletions test/nodejs/Makefile.am
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.


THRIFT = $(top_srcdir)/compiler/cpp/thrift

stubs: ../ThriftTest.thrift
$(THRIFT) --gen js:node ../ThriftTest.thrift

check: stubs

clean-local:
$(RM) -r gen-nodejs

server: stubs
NODE_PATH=../../lib/nodejs/lib:../../lib/nodejs/lib/thrift node server.js

client: stubs
NODE_PATH=../../lib/nodejs/lib:../../lib/nodejs/lib/thrift node client.js
157 changes: 157 additions & 0 deletions test/nodejs/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
var thrift = require('thrift');

var ThriftTest = require('./gen-nodejs/ThriftTest'),
ttypes = require('./gen-nodejs/ThriftTest_types');

var connection = thrift.createConnection('localhost', 9090),
client = thrift.createClient(ThriftTest, connection);

var tfailed = 0;
var tpassed = 0;

function failed(err) {
console.trace(err);
return tfailed++;
}
function passed() {
return tpassed++;
}

connection.on('error', function(err) {
failed(err);
});

console.time("Tests completed in");

client.testVoid(function(err, response) {
if (err) { return failed(err); }
console.log("testVoid() = ", response);
passed();
});

client.testString("Test", function(err, response) {
if (err) { return failed(err); }
console.log("testString('Test') = ", response);
passed();
});

client.testByte(1, function(err, response) {
if (err) { return failed(err); }
console.log("testByte(1) = ", response);
passed();
});

client.testI32(-1, function(err, response) {
if (err) { return failed(err); }
console.log("testI32(-1) = ", response);
passed();
});

client.testI64(5, function(err, response) {
if (err) { return failed(err); }
console.log("testI64(5) = ", response);
passed();
});

/*
* FATAL ERROR: CALL_AND_RETRY_2 Allocation failed - process out of memory
client.testI64(-5, function(err, response) {
if (err) { return failed(err); }
console.log("testI64(-5) = ", response);
passed();
});
*/

client.testI64(-34359738368, function(err, response) {
if (err) { return failed(err); }
console.log("testI64(-34359738368) = ", response);
passed();
});

client.testDouble(-5.2098523, function(err, response) {
if (err) { return failed(err); }
console.log("testDouble(-5.2098523) = ", response);
passed();
});

var out = new ttypes.Xtruct({
string_thing: 'Zero',
byte_thing: 1,
i32_thing: -3,
i64_thing: 1000000
});
client.testStruct(out, function(err, response) {
if (err) { return failed(err); }
console.log("testStruct(", out, ") = \n", response);
passed();
});

var out2 = new ttypes.Xtruct2();
out2.byte_thing = 1;
out2.struct_thing = out;
out2.i32_thing = 5;
client.testNest(out2, function(err, response) {
if (err) { return failed(err); }
console.log("testNest(", out2, ") = \n", response);
passed();
});

/*
* TypeError: Cannot read property 'length' of undefined
var mapout = {};
for (var i = 0; i < 5; ++i) {
mapout[i] = i-10;
}
client.testMap(mapout, function(err, response) {
if (err) { return failed(err); }
console.log("testMap(", mapout, ") = \n", response);
passed();
});
*/

/*
* TODO: testSet, testList, testEnum, testTypedef, testMapMap, testInsanity
*/


client.testException('ApplicationException', function(err, response) {
console.log("testException('ApplicationException') = ", err);
if (response) { return failed(response); }
passed();
});

client.testException('Xception', function(err, response) {
console.log("testException('Xception') = ", err);
if (response) { return failed(response); }
passed();
});

client.testException('success', function(err, response) {
if (err) { return failed(err); }
console.log("testException('success') = ", response);
passed();
});

setTimeout(function(){
console.timeEnd("Tests completed in");
console.log(tpassed + " passed, " + tfailed + " failed");
connection.end();
}, 200);
Loading

0 comments on commit eaa61d8

Please sign in to comment.