THRIFT-1267 Node.js can't throw exceptions
Patch: Henrique Mendonca

git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1230797 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/.gitignore b/.gitignore
index cfe1185..24529c9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -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
diff --git a/compiler/cpp/src/generate/t_js_generator.cc b/compiler/cpp/src/generate/t_js_generator.cc
index 6a27886..5a5b59e 100644
--- a/compiler/cpp/src/generate/t_js_generator.cc
+++ b/compiler/cpp/src/generate/t_js_generator.cc
@@ -518,10 +518,10 @@
 
   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";
@@ -557,6 +557,17 @@
       }
     }
 
+    // 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) {
@@ -736,19 +747,18 @@
       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);
@@ -772,7 +782,7 @@
     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_);
@@ -836,12 +846,6 @@
         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();
@@ -871,11 +875,11 @@
     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 <<
@@ -959,7 +963,7 @@
 
   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_ <<
diff --git a/configure.ac b/configure.ac
index 0bbf4f2..07692da 100644
--- a/configure.ac
+++ b/configure.ac
@@ -492,6 +492,7 @@
   test/Makefile
   test/cpp/Makefile
   test/hs/Makefile
+  test/nodejs/Makefile
   test/perl/Makefile
   test/py/Makefile
   test/py.twisted/Makefile
diff --git a/lib/nodejs/examples/Makefile b/lib/nodejs/examples/Makefile
index 1930279..b4283dc 100644
--- a/lib/nodejs/examples/Makefile
+++ b/lib/nodejs/examples/Makefile
@@ -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
diff --git a/lib/nodejs/examples/server.js b/lib/nodejs/examples/server.js
index 3b8c046..1c482fe 100644
--- a/lib/nodejs/examples/server.js
+++ b/lib/nodejs/examples/server.js
@@ -24,15 +24,15 @@
 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]);
   },
 });
 
diff --git a/lib/nodejs/examples/server_multitransport.js b/lib/nodejs/examples/server_multitransport.js
index e5d6d76..7f9b8fb 100644
--- a/lib/nodejs/examples/server_multitransport.js
+++ b/lib/nodejs/examples/server_multitransport.js
@@ -6,14 +6,14 @@
 
 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, {
diff --git a/test/Makefile.am b/test/Makefile.am
index 1dfff80..f45424a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -17,7 +17,7 @@
 # under the License.
 #
 
-SUBDIRS =
+SUBDIRS = nodejs
 
 if WITH_CPP
 SUBDIRS += cpp
@@ -45,6 +45,7 @@
 	csharp \
 	erl \
 	hs \
+	nodejs \
 	ocaml \
 	perl \
 	php \
diff --git a/test/nodejs/Makefile.am b/test/nodejs/Makefile.am
new file mode 100755
index 0000000..fc8e7d0
--- /dev/null
+++ b/test/nodejs/Makefile.am
@@ -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
diff --git a/test/nodejs/client.js b/test/nodejs/client.js
new file mode 100644
index 0000000..60d70bf
--- /dev/null
+++ b/test/nodejs/client.js
@@ -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);
diff --git a/test/nodejs/package.json b/test/nodejs/package.json
new file mode 100644
index 0000000..09c6b2c
--- /dev/null
+++ b/test/nodejs/package.json
@@ -0,0 +1,30 @@
+{
+  "name": "thrift-nodejs-test",
+  "version": "0.9.0-dev",
+  "description": "node.js test server and client for the Apache Thrift",
+  "homepage": "http://thrift.apache.org/",
+  "repository":
+    { "type" : "svn",
+      "url" : "http://svn.apache.org/repos/asf/thrift/trunk/"
+    },
+  "author":
+    { "name": "Apache Thrift Developers",
+      "email": "dev@thrift.apache.org",
+      "url": "http://thrift.apache.org"
+    },
+  "licenses":
+    [ { "type": "Apache-2.0",
+        "url": "http://www.apache.org/licenses/LICENSE-2.0"
+      }
+    ],
+  "bugs":
+    { "mail": "dev@thrift.apache.org",
+      "url": "https://issues.apache.org/jira/browse/THRIFT"
+    },
+  "directories" : { "lib" : "../lib/nodejs/lib/thrift" },
+  "main": "../lib/nodejs/lib/thrift",
+  "scripts": {
+    "start": "node ./http-server"
+  },
+  "engines": { "node": ">= 0.2.4" }
+}
diff --git a/test/nodejs/server.js b/test/nodejs/server.js
new file mode 100644
index 0000000..056209d
--- /dev/null
+++ b/test/nodejs/server.js
@@ -0,0 +1,220 @@
+/*
+ * 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 Thrift = thrift.Thrift;
+
+var ThriftTest = require('./gen-nodejs/ThriftTest'),
+    ttypes = require('./gen-nodejs/ThriftTest_types');
+
+
+var server = thrift.createServer(ThriftTest, {
+  testVoid: function(result) {
+    console.log('testVoid()');
+    result(null);
+  },
+
+  testString: function(thing, result) {
+    console.log('testString(\'' + thing + '\')');
+    result(null, thing);
+  },
+
+  testByte: function(thing, result) {
+    console.log('testByte(' + thing + ')');
+    result(null, thing);
+  },
+
+  testI32: function(thing, result) {
+    console.log('testI32(' + thing + ')');
+    result(null, thing);
+  },
+
+  testI64: function(thing, result) {
+    console.log('testI64(' + thing + ')');
+    result(null, thing);
+  },
+
+  testDouble: function(thing, result) {
+    console.log('testDouble(' + thing + ')');
+    result(null, thing);
+  },
+
+  testStruct: function(thing, result) {
+    console.log('testStruct(');
+    console.log(thing);
+    console.log(')');
+    result(null, thing);
+  },
+
+  testNest: function(nest, result) {
+    console.log('testNest(');
+    console.log(nest);
+    console.log(')');
+    result(null, nest);
+  },
+
+  testMap: function(thing, result) {
+    console.log('testMap(');
+    console.log(thing);
+    console.log(')');
+    result(null, thing);
+  },
+
+  testStringMap: function(thing, result) {
+    console.log('testStringMap(');
+    console.log(thing);
+    console.log(')');
+    result(null, thing);
+  },
+
+  testSet: function(thing, result) {
+    console.log('testSet(');
+    console.log(thing);
+    console.log(')');
+    result(null, thing);
+  },
+
+  testList: function(thing, result) {
+    console.log('testList(');
+    console.log(thing);
+    console.log(')');
+    result(null, thing);
+  },
+
+  testEnum: function(thing, result) {
+    console.log('testEnum(' + thing + ')');
+    result(null, thing);
+  },
+
+  testTypedef: function(thing, result) {
+    console.log('testTypedef(' + thing + ')');
+    result(null, thing);
+  },
+
+  testMapMap: function(hello, result) {
+    console.log('testMapMap(' + hello + ')');
+
+    var mapmap = [];
+    var pos = [];
+    var neg = [];
+    for (var i = 1; i < 5; i++) {
+      pos[i] = i;
+      neg[-i] = -i;
+    }
+    mapmap[4] = pos;
+    mapmap[-4] = neg;
+
+    result(null, mapmap);
+  },
+
+  testInsanity: function(argument, result) {
+    console.log('testInsanity()');
+
+    var hello = new ttypes.Xtruct();
+    hello.string_thing = 'Hello2';
+    hello.byte_thing = 2;
+    hello.i32_thing = 2;
+    hello.i64_thing = 2;
+
+    var goodbye = new ttypes.Xtruct();
+    goodbye.string_thing = 'Goodbye4';
+    goodbye.byte_thing = 4;
+    goodbye.i32_thing = 4;
+    goodbye.i64_thing = 4;
+
+    var crazy = new ttypes.Insanity();
+    crazy.userMap = [];
+    crazy.userMap[ttypes.Numberz.EIGHT] = 8;
+    crazy.userMap[ttypes.Numberz.FIVE] = 5;
+    crazy.xtructs = [];
+    crazy.xtructs.add(goodbye);
+    crazy.xtructs.add(hello);
+
+    var first_map = [];
+    var second_map = [];
+
+    first_map[ttypes.Numberz.TWO] = crazy;
+    first_map[ttypes.Numberz.THREE] = crazy;
+
+    var looney = new ttypes.Insanity();
+    second_map[ttypes.Numberz.SIX] = looney;
+
+    var insane = [];
+    insane[1] = first_map;
+    insane[2] = second_map;
+
+    result(null, insane);
+  },
+
+  testMulti: function(arg0, arg1, arg2, arg3, arg4, arg5, result) {
+    console.log('testMulti()');
+
+    var hello = new ttypes.Xtruct();;
+    hello.string_thing = 'Hello2';
+    hello.byte_thing = arg0;
+    hello.i32_thing = arg1;
+    hello.i64_thing = arg2;
+    result(null, hello);
+  },
+
+  testException: function(arg, result) {
+    console.log('testException('+arg+')');
+    if (arg === 'Xception') {
+      var x = new ttypes.Xception();
+      x.errorCode = 1001;
+      x.message = arg;
+      result(x);
+    } else if (arg === 'ApplicationException') {
+      result(new Thrift.TException(arg));
+    } else {
+      var res = new ttypes.Xtruct();
+      res.string_thing = arg;
+			result(null, res);
+    }
+  },
+
+  testMultiException: function(arg0, arg1, result) {
+    console.log('testMultiException(' + arg0 + ', ' + arg1 + ')');
+    if (arg0 === ('Xception')) {
+      var x = new Xception();
+      x.errorCode = 1001;
+      x.message = 'This is an Xception';
+      result(x);
+    } else if (arg0 === ('Xception2')) {
+      var x = new Xception2();
+      x.errorCode = 2002;
+      x.struct_thing = new Xtruct();
+      x.struct_thing.string_thing = 'This is an Xception2';
+      result(x);
+    }
+
+    var res = new Xtruct();
+    res.string_thing = arg1;
+    result(null, res);
+  },
+
+  testOneway: function(sleepFor, result) {
+    console.log('testOneway(' + sleepFor + ') => sleeping...');
+    setTimeout(function(){
+      console.log('Done sleeping!');
+    }, sleepFor);
+		result(null);
+  }
+});
+
+server.listen(9090);
diff --git a/test/test.sh b/test/test.sh
index ee74e75..071b5e5 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -76,6 +76,7 @@
 # we need a test certificate first
 #sockets="ip ip-ssl domain"
 
+
 for proto in $protocols; do
   for trans in $transports; do
     for sock in $sockets; do
@@ -117,3 +118,11 @@
         "perl -I perl/gen-perl/ -I../lib/perl/lib/ perl/TestClient.pl" \
         "cpp/TestServer" \
         "10"
+do_test "nodejs-nodejs" "binary" "framed-ip" \
+        "make -C nodejs/ client" \
+        "make -C nodejs/ server" \
+        "1"
+do_test "cpp-nodejs" "binary" "framed-ip" \
+        "cpp/TestClient --transport=framed" \
+        "make -C nodejs/ server" \
+        "1"