Fix edge cases constructing long stack traces

fixes #1387

Author: dougwilson

Changes.md

@@ -8,6 +8,7 @@ you spot any mistakes.
 
 * Add `POOL_CLOSED` code to "Pool is closed." error
 * Add `POOL_CONNLIMIT` code to "No connections available." error #1332
+* Fix edge cases constructing long stack traces #1387
 * Fix Query stream to emit close after ending #1349 #1350
 * Fix type cast for BIGINT columns when number is negative #1376
 * Performance improvements for array/object escaping in SqlString #1331

lib/protocol/sequences/Sequence.js

@@ -6,6 +6,8 @@ var ErrorConstants = require('../constants/errors');
 var listenerCount = EventEmitter.listenerCount
   || function(emitter, type){ return emitter.listeners(type).length; };
 
+var LONG_STACK_DELIMITER = '\n    --------------------\n';
+
 module.exports = Sequence;
 Util.inherits(Sequence, EventEmitter);
 function Sequence(options, callback) {
@@ -54,20 +56,6 @@ Sequence.prototype._packetToError = function(packet) {
   return err;
 };
 
-Sequence.prototype._addLongStackTrace = function _addLongStackTrace(err) {
-  if (!this._callSite || !this._callSite.stack) {
-    return;
-  }
-
-  var delimiter = '\n    --------------------\n';
-
-  if (err.stack.indexOf(delimiter) > -1) {
-    return;
-  }
-
-  err.stack += delimiter + this._callSite.stack.replace(/.+\n/, '');
-};
-
 Sequence.prototype.end = function(err) {
   if (this._ended) {
     return;
@@ -113,6 +101,27 @@ Sequence.prototype['ErrorPacket'] = function(packet) {
 // Implemented by child classes
 Sequence.prototype.start = function() {};
 
+Sequence.prototype._addLongStackTrace = function _addLongStackTrace(err) {
+  var callSiteStack = this._callSite && this._callSite.stack;
+
+  if (!callSiteStack || typeof callSiteStack !== 'string') {
+    // No recorded call site
+    return;
+  }
+
+  if (err.stack.indexOf(LONG_STACK_DELIMITER) !== -1) {
+    // Error stack already looks long
+    return;
+  }
+
+  var index = callSiteStack.indexOf('\n');
+
+  if (index !== -1) {
+    // Append recorded call site
+    err.stack += LONG_STACK_DELIMITER + callSiteStack.substr(index + 1);
+  }
+};
+
 Sequence.prototype._onTimeout = function _onTimeout() {
   this.emit('timeout');
 };

test/unit/connection/test-error-trace-bad-stack.js

@@ -0,0 +1,22 @@
+var assert     = require('assert');
+var common     = require('../../common');
+var connection = common.createConnection({port: common.fakeServerPort});
+
+var server = common.createFakeServer();
+
+server.listen(common.fakeServerPort, function (err) {
+  assert.ifError(err);
+
+  // Common mistake to leave in code
+  Error.prepareStackTrace = function (err, stack) {
+    return stack;
+  };
+
+  connection.query('INVALID SQL', function (err) {
+    assert.ok(err, 'got error');
+    assert.ok(typeof err.stack !== 'string', 'error stack is not a string');
+
+    connection.destroy();
+    server.destroy();
+  });
+});

test/unit/connection/test-error-trace-no-stack.js

@@ -0,0 +1,20 @@
+var assert     = require('assert');
+var common     = require('../../common');
+var connection = common.createConnection({port: common.fakeServerPort});
+
+var server = common.createFakeServer();
+
+server.listen(common.fakeServerPort, function (err) {
+  assert.ifError(err);
+
+  Error.stackTraceLimit = 0;
+
+  connection.query('INVALID SQL', function (err) {
+    assert.ok(err, 'got error');
+    assert.ok(!err.stack || err.stack.indexOf('\n    --------------------\n') === -1,
+      'error stack does not have delimiter');
+
+    connection.destroy();
+    server.destroy();
+  });
+});

test/unit/connection/test-error-trace.js

@@ -0,0 +1,18 @@
+var assert     = require('assert');
+var common     = require('../../common');
+var connection = common.createConnection({port: common.fakeServerPort});
+
+var server = common.createFakeServer();
+
+server.listen(common.fakeServerPort, function (err) {
+  assert.ifError(err);
+
+  connection.query('INVALID SQL', function (err) {
+    assert.ok(err, 'got error');
+    assert.ok(typeof err.stack === 'string', 'error stack is string');
+    assert.ok(err.stack.indexOf('\n    --------------------\n') !== -1, 'error stack has delimiter');
+
+    connection.destroy();
+    server.destroy();
+  });
+});