Merge pull request #37 from CUBRID/15.fix-connection-reset

Fix #15: When closing a query, if connection is reset, consider the request was successful.

.travis.yml

@@ -3,8 +3,6 @@ language: node_js
 node_js:
   # Quote the version number as Travis will interpret 0.10 as 0.1 unless quoted.
   - "0.10"
-  - "0.8"
-  - "0.6"
 
 env:
   - CUBRID_VERSION=9.1.0

CHANGELOG.md

@@ -1,5 +1,12 @@
 # node-cubrid change log
 
+## Version 2.2.2 (Feb 24, 2015)
+
+- Fix #15: When closing a query, if connection is reset, consider the request was successful.
+- Fix: mark the connection as closed only after closing the connection.
+- Doc: add Table of Contents to README documentation.
+- Travis: remove Node.js 0.6 and 0.8 support. Some of dependent modules for testing dropped the support of these versions.
+
 ## Version 2.2.1 (Feb 1, 2015)
 
 - Fix #15: When closing a query, if connection is closed, consider the request was successful.

README.md

@@ -3,6 +3,34 @@
 [![Build Status](https://travis-ci.org/CUBRID/node-cubrid.png)](https://travis-ci.org/CUBRID/node-cubrid)
 [![Coverage Status](https://coveralls.io/repos/CUBRID/node-cubrid/badge.png)](https://coveralls.io/r/CUBRID/node-cubrid)
 
+**Table of Contents**
+
+* [Introduction](#introduction)
+* [Key features](#key-features)
+* [CHANGELOG](#changelog)
+* [Installation](#installation)
+* [Request flow in node-cubrid](#request-flow-in-node-cubrid)
+* [API Documentation](#api-documentation)
+	* [Creating a CUBRID client](#creating-a-cubrid-client)
+	* [Establishing a connection](#establishing-a-connection)
+	* [Connection configuration](#connection-configuration)
+		* [Setting connection timeout](#setting-connection-timeout)
+		* [Setting CUBRID Server Parameters](#setting-cubrid-server-parameters)
+	* [Executing SQL queries](#executing-sql-queries)
+		* [READ queries](#read-queries)
+		* [Close Query](#close-query)
+		* [WRITE queries](#write-queries)
+	* [Queueing](#queueing)
+	* [Transactions](#transactions)
+	* [Closing a connection](#closing-a-connection)
+	* [Events by EventEmitter](#events-by-eventemitter)
+* [More examples](#more-examples)
+* [Running tests](#running-tests)
+	* [Running tests in a Docker container](#running-tests-in-a-docker-container)
+* [What's next](#whats-next)
+* [Authors and Contributors](#authors-and-contributors)
+* [Special thanks](#special-thanks)
+
 ## Introduction
 
 This is a Node.js driver for [CUBRID](http://www.cubrid.org) open-source relational database. **node-cubrid** is implemented in 100% JavaScript with no external dependency. Besides the database specific APIs, the module supplies several *helper* APIs which are useful to sanitize and validate user input values, format and parameterize SQL statements, etc.
@@ -559,7 +587,7 @@ Here is an example.
 
 Remember that the `callback` is optional in which case you should listen for events.
 
-Alternatively, for backward compatibility we still support `addQeury()` and `addNonQuery()` functions which we introduced in version 2.0.0.
+Alternatively, for backward compatibility we still support `addQuery()` and `addNonQuery()` functions which we introduced in version 2.0.0.
 
     var SQL_1 = "SELECT COUNT(*) FROM [code]";
     var SQL_2 = "SELECT * FROM [code] WHERE s_name = 'X'";
@@ -876,4 +904,4 @@ for the code we have (re)used and for doing such a great job for the open-source
 -	[https://github.com/felixge/node-mysql](https://github.com/felixge/node-mysql)
 -	[https://github.com/jeromeetienne/microcache.js](https://github.com/jeromeetienne/microcache.js)
 
-...Stay tuned for the next great driver releases! :)
+... Stay tuned for the next great driver releases! :)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "node-cubrid",
-  "version": "2.2.1",
+  "version": "2.2.2",
   "description": "This is a Node.js driver for CUBRID RDBMS. It is developed in 100% JavaScript and does not require specific platform compilation.",
   "author": "CUBRID <contact@cubrid.org> (http://www.cubrid.org/)",
   "repository": {

src/CUBRIDConnection.js

@@ -81,6 +81,7 @@ Util.inherits(CUBRIDConnection, EventEmitter);
  * @param password
  * @param database
  * @param cacheTimeout
+ * @param connectionTimeout
  * @constructor
  */
 function CUBRIDConnection(brokerServer, brokerPort, user, password, database, cacheTimeout, connectionTimeout) {
@@ -185,7 +186,6 @@ function CUBRIDConnection(brokerServer, brokerPort, user, password, database, ca
 
 /**
  * Get broker connection port
- * @param self
  * @param callback
  * @private
  */
@@ -336,7 +336,6 @@ CUBRIDConnection.prototype._doDatabaseLogin = function (callback) {
 
 /**
  * Get the server database engine version
- * @param self
  * @param callback
  */
 CUBRIDConnection.prototype._getEngineVersion = function (callback) {
@@ -1166,66 +1165,81 @@ CUBRIDConnection.prototype._closeQuery = function (queryHandle, callback) {
 
 	  Helpers._emitEvent(this, err, this.EVENT_ERROR, null);
   } else {
-    var closeQueryPacket = new CloseQueryPacket({
-		      casInfo    : this._CASInfo,
-		      reqHandle  : queryHandle,
-		      db_version : this._DB_ENGINE_VER
-		    }),
-		    packetWriter = new PacketWriter(closeQueryPacket.getBufferLength()),
-		    socket = this._socket;
-
-	  closeQueryPacket.write(packetWriter);
-
-	  this._socketCurrentEventCallback = callback;
-
-	  function onResponse(err) {
-		  // Remove the event we've previously bound to.
-		  socket.removeListener('end', onConnectionReset);
-
-		  if (!err) {
-			  // Remove the referenced query packet from the hash.
-			  self._queriesPacketList.splice(foundQueryHandle, 1);
+	  ActionQueue.enqueue([
+		  this._implyConnect.bind(this),
+		  function (cb) {
+			  var closeQueryPacket = new CloseQueryPacket({
+						  casInfo    : self._CASInfo,
+						  reqHandle  : queryHandle,
+						  db_version : self._DB_ENGINE_VER
+					  }),
+					  packetWriter = new PacketWriter(closeQueryPacket.getBufferLength()),
+					  socket = self._socket;
+
+			  closeQueryPacket.write(packetWriter);
+
+			  function onResponse(err) {
+				  // Remove the event we've previously bound to.
+				  socket.removeListener('end', onConnectionReset);
+
+				  if (!err) {
+					  // Remove the referenced query packet from the hash.
+					  self._queriesPacketList.splice(foundQueryHandle, 1);
+				  }
+
+				  cb(err);
+			  }
+
+			  function onConnectionReset() {
+				  // CUBRID Broker may occasionally reset the connection between the client and the 
+				  // CAS that the Broker has assigned to this client. Refer to 
+				  // https://github.com/CUBRID/node-cubrid/issues/15 for details. According to 
+				  // CUBRID CCI native driver implementation function qe_send_close_handle_msg(),
+				  // we can consider the connection closed operation as a successful request.
+				  // This is true because internally CUBRID Broker manages a pool of CAS 
+				  // (CUBRID Application Server) processes. When a client connects, the Broker 
+				  // assigns/connect it to one of the CAS. Then the client sends some query requests
+				  // to this CAS. After the client receives a response, it may decide to do some
+				  // other application logic before it closes the query handle. Once the client is
+				  // done with the response, it may try to close the query handle.
+				  // In between these receive response and close query, CUBRID Broker may reassign
+				  // the CAS to another client. Notice the client-Broker connection is not broken.
+				  // When the actual close query request arrives to the Broker, it finds out that 
+				  // the CAS referred by the client is reassigned, it sends CONNECTION RESET to the
+				  // client. node-cubrid should listen it and consider such event as if the close 
+				  // query request was successful.
+				  socket.removeAllListeners('data');
+				  // Execute `onResponse` without an error.
+				  onResponse();
+			  }
+
+			  function onError(err) {
+				  socket.removeAllListeners('data');
+
+				  // `ECONNRESET` should also be considered as a connection
+				  // reset.
+				  onResponse(err.code == 'ECONNRESET' ? null : err);
+			  }
+
+			  self._socketCurrentEventCallback = onError;
+
+			  socket.on('data', self._receiveBytes({
+				  queryHandle: queryHandle,
+				  parserFunction: self._parseCloseQueryBuffer,
+				  dataPacket: closeQueryPacket
+			  }, onResponse));
+
+			  socket.on('end', onConnectionReset);
+
+			  socket.write(packetWriter._buffer);
 		  }
-
+	  ], function (err) {
 		  if (typeof(callback) === 'function') {
 			  callback(err, queryHandle);
 		  }
-
-		  Helpers._emitEvent(self, err, self.EVENT_ERROR, self.EVENT_QUERY_CLOSED, queryHandle);
-	  }
 
-	  function onConnectionReset() {
-		  // CUBRID Broker may occasionally reset the connection between the client and the 
-		  // CAS that the Broker has assigned to this client. Refer to 
-		  // https://github.com/CUBRID/node-cubrid/issues/15 for details. According to 
-		  // CUBRID CCI native driver implementation function qe_send_close_handle_msg(),
-		  // we can consider the connection closed operation as a successful request.
-		  // This is true because internally CUBRID Broker manages a pool of CAS 
-		  // (CUBRID Application Server) processes. When a client connects, the Broker 
-		  // assigns/connect it to one of the CAS. Then the client sends some query requests
-		  // to this CAS. After the client receives a response, it may decide to do some
-		  // other application logic before it closes the query handle. Once the client is
-		  // done with the response, it may try to close the query handle.
-		  // In between these receive response and close query, CUBRID Broker may reassign
-		  // the CAS to another client. Notice the client-Broker connection is not broken.
-		  // When the actual close query request arrives to the Broker, it finds out that 
-		  // the CAS referred by the client is reassigned, it sends CONNECTION RESET to the
-		  // client. node-cubrid should listen it and consider such event as if the close 
-		  // query request was successful.
-		  socket.removeAllListeners('data');
-		  // Execute `onResponse` without an error.
-		  onResponse();
-	  }
-
-	  socket.on('data', this._receiveBytes({
-		  queryHandle: queryHandle,
-		  parserFunction: this._parseCloseQueryBuffer,
-		  dataPacket: closeQueryPacket
-	  }, onResponse));
-
-	  socket.on('end', onConnectionReset);
-
-	  socket.write(packetWriter._buffer);
+		  Helpers._emitEvent(self, err, self.EVENT_ERROR, self.EVENT_QUERY_CLOSED, queryHandle);
+	  });
   }
 };
 
@@ -1243,17 +1257,14 @@ function close(callback) {
 	if (!this.connectionOpened) {
 		// If the connection has already been closed, no need to emit
 		// the error. After all this is what the client wants - to
-		// close the connection
+		// close the connection.
 		if (typeof(callback) === 'function') {
 			callback();
 		}
 
 		return;
 	}
 
-	// Reset connection status
-	this.connectionPending = false;
-	this.connectionOpened = false;
 	// Remove all pending requests.
 	this._queue.empty();
 
@@ -1296,8 +1307,12 @@ function close(callback) {
 			socket.write(packetWriter._buffer);
 		}
 	], function (err) {
-		Helpers._emitEvent(self, err, self.EVENT_ERROR, self.EVENT_CONNECTION_CLOSED);
+		// Reset connection status
+		self.connectionPending = false;
+		self.connectionOpened = false;
 
+		Helpers._emitEvent(self, err, self.EVENT_ERROR, self.EVENT_CONNECTION_CLOSED);
+
 		if (typeof(callback) === 'function') {
 			callback(err);
 		}