Changing error handling to not catch errors generated from user code.

Author: scriby

lib/mongodb/connection.js

@@ -184,16 +184,13 @@ var setupConnectionPool = function(self, poolSize, reconnect) {
  // Ensure we clean up if needed
  try {
  self.emit("data", buffer); 
- } catch(err) {
- // debug("====================================================== 0")
- self.emit("error", {err:"socketHandler", trace:err, bin:buffer, parseState:conObj}); 
- }
- 
- // Reset the variables
- conObj.buffer = new Buffer(0); conObj.bytesRead = 0; conObj.sizeOfMessage = 0;
- // If message is longer than the current one, keep parsing
- if(remainingBytes < result.length) {
- receiveListener(result.slice(remainingBytes, result.length), fd);
+ } finally {
+ // Reset the variables
+ conObj.buffer = new Buffer(0); conObj.bytesRead = 0; conObj.sizeOfMessage = 0;
+ // If message is longer than the current one, keep parsing
+ if(remainingBytes < result.length) {
+ receiveListener(result.slice(remainingBytes, result.length), fd);
+ }
  }
  }
  } else if(conObj != null){
@@ -221,30 +218,17 @@ var setupConnectionPool = function(self, poolSize, reconnect) {
  conObj.bytesRead = result.length; conObj.sizeOfMessage = sizeOfMessage;
  } else if(sizeOfMessage == result.length) {
 
- // Catches the error, and ensures we do not get a crazy ripple killing the connection
- // Ensure we clean up if needed
- try {
- self.emit("data", result);
- } catch(err) {
- // debug("====================================================== 1")
- self.emit("error", {err:"socketHandler", trace:err, bin:result, parseState:conObj}); 
- }
-
- // self.emit("data", result);
+ //Try/finally not needed here as there is no more code to be executed afterwards
+ self.emit("data", result);
  } else if(sizeOfMessage < result.length) {
 
  // Catches the error, and ensures we do not get a crazy ripple killing the connection
  // Ensure we clean up if needed
  try {
  self.emit("data", result.slice(0, sizeOfMessage));
- } catch(err) {
- // debug("====================================================== 2")
- self.emit("error", {err:"socketHandler", trace:err, bin:result, parseState:conObj}); 
+ } finally {
+ receiveListener(result.slice(sizeOfMessage, result.length), fd);
  }
-
- // self.emit("data", result.slice(0, sizeOfMessage));
- // receiveListener(result.slice(sizeOfMessage, result.length), fd);
- receiveListener(result.slice(sizeOfMessage, result.length), fd);
  }
  } else {
  conObj.stubBuffer = result;

lib/mongodb/connection/connection.js

@@ -97,19 +97,13 @@ var dataHandler = function(self) {
  // Emit current complete message
  try {
  self.emit("message", self.buffer);
- } catch(err) {
- // We got a parse Error fire it off then keep going
- self.emit("parseError", {err:"socketHandler", trace:err, bin:buffer, parseState:{
-  sizeOfMessage:self.sizeOfMessage, 
-  bytesRead:self.bytesRead,
-  stubBuffer:self.stubBuffer}});
+ } finally {
+ // Reset state of buffer
+ self.buffer = null;
+ self.sizeOfMessage = 0;
+ self.bytesRead = 0;
+ self.stubBuffer = null;
  }
- 
- // Reset state of buffer
- self.buffer = null;
- self.sizeOfMessage = 0;
- self.bytesRead = 0;
- self.stubBuffer = null;
  }
  } else {
  // Stub buffer is kept in case we don't get enough bytes to determine the
@@ -171,18 +165,12 @@ var dataHandler = function(self) {
  } else if(sizeOfMessage == data.length) {
  try {
  emit("message", data);
+ } finally {
  // Reset state of buffer
  self.buffer = null;
  self.sizeOfMessage = 0;
  self.bytesRead = 0;
  self.stubBuffer = null;
- 
- } catch (err) {
- // We got a parse Error fire it off then keep going
- self.emit("parseError", {err:"socketHandler", trace:err, bin:buffer, parseState:{
- sizeOfMessage:self.sizeOfMessage, 
- bytesRead:self.bytesRead,
- stubBuffer:self.stubBuffer}}); 
  }
  }
 

lib/mongodb/connections/repl_set_servers.js

@@ -275,19 +275,21 @@ ReplSetServers.prototype.connect = function(parent, callback) {
  try {
  // Parse the data as a reply object
  reply = new MongoReply(parent, message); 
- // Emit error if there is one
- reply.responseHasError ? parent.emit(reply.responseTo.toString(), reply.documents[0], reply) : parent.emit(reply.responseTo.toString(), null, reply);
  } catch(err) {
  // Catch and emit
  var errObj = {err:"unparsable", bin:message, trace:err};
  server.logger.error("mongoreplyParserError", errObj);
  parent.emit("error", errObj);
  }
 
- // Remove the listener
- if(parent.notReplied[reply.responseTo.toString()]) {
- delete parent.notReplied[reply.responseTo.toString()];
- parent.removeListener(reply.responseTo.toString(), parent.listeners(reply.responseTo.toString())[0]);
+ try{
+ reply.responseHasError ? parent.emit(reply.responseTo.toString(), reply.documents[0], reply) : parent.emit(reply.responseTo.toString(), null, reply);
+ } finally {
+ // Remove the listener
+ if(parent.notReplied[reply.responseTo.toString()]) {
+ delete parent.notReplied[reply.responseTo.toString()];
+ parent.removeListener(reply.responseTo.toString(), parent.listeners(reply.responseTo.toString())[0]);
+ }
  }
  });
  });

lib/mongodb/connections/server.js

@@ -90,18 +90,22 @@ Server.prototype.connect = function(parent, callback) {
  // Parse the data as a reply object
  reply = new MongoReply(parent, message); 
  // Emit message
- parent.emit(reply.responseTo.toString(), null, reply);
+
  } catch(err) {
  // Catch and emit
  var errObj = {err:"unparsable", bin:message, trace:err};
  server.logger.error("mongoreplyParserError", errObj);
  parent.emit("error", errObj);
  } 
 
- // Remove the listener
- if(reply != null && parent.notReplied[reply.responseTo.toString()]) {
- delete parent.notReplied[reply.responseTo.toString()];
- parent.removeListener(reply.responseTo.toString(), parent.listeners(reply.responseTo.toString())[0]);
+ try {
+ parent.emit(reply.responseTo.toString(), null, reply);
+ } finally {
+ // Remove the listener
+ if(reply != null && parent.notReplied[reply.responseTo.toString()]) {
+ delete parent.notReplied[reply.responseTo.toString()];
+ parent.removeListener(reply.responseTo.toString(), parent.listeners(reply.responseTo.toString())[0]);
+ }
  }
  });
 

test/exception_handling_test.js

@@ -47,7 +47,8 @@ var tests = testCase({
  client.createCollection('shouldCorrectlyHandleThrownError', function(err, r) {
  try {
  client.collection('shouldCorrectlyHandleThrownError', function(err, collection) {
- debug(someUndefinedVariable);
+ test.done();
+ //debug(someUndefinedVariable);
  }); 
  } catch (err) {
  test.ok(err != null);
@@ -66,7 +67,8 @@ var tests = testCase({
  client.createCollection('shouldCorrectlyHandleThrownErrorInRename', function(err, r) { 
  client.collection('shouldCorrectlyHandleThrownError', function(err, collection) {
  collection.rename("shouldCorrectlyHandleThrownErrorInRename2", function(err, result) {
- debug(someUndefinedVariable); 
+ test.done();
+ //debug(someUndefinedVariable);
  })
  }); 
  });

test/find_test.js

@@ -48,19 +48,14 @@ var tests = testCase({
  'Error thrown in handler test': function(test){
  // Should not be called
  var exceptionHandler = function(exception) {
- test.ok(false);
- console.log('Exception caught: ' + exception.message);
- console.log(inspect(exception.stack.toString()))
+ numberOfFailsCounter++;
+ //console.log('Exception caught: ' + exception.message);
+ //console.log(inspect(exception.stack.toString()))
  };
 
  // Number of times we should fail
  var numberOfFailsCounter = 0;
 
- // Error handler
- client.on("error", function(err) {
- numberOfFailsCounter = numberOfFailsCounter + 1;
- });
- 
  process.on('uncaughtException', exceptionHandler)
 
  client.createCollection('error_test', function(err, collection) {
@@ -78,8 +73,7 @@ var tests = testCase({
  var findOne = function(){
  collection.findOne({name: 'test1'}, function(err, doc) {
  counter++;
- process.nextTick(findOne);
- 
+
  if(counter > POOL_SIZE){
  process.removeListener('uncaughtException', exceptionHandler);
 
@@ -89,6 +83,7 @@ var tests = testCase({
  test.done();
  }); 
  } else {
+ process.nextTick(findOne);
  throw new Error('Some error');
  }
  });