Skip to content

Commit

Permalink
Fix edge cases constructing long stack traces
Browse files Browse the repository at this point in the history
fixes #1387
  • Loading branch information
dougwilson committed Jun 1, 2016
1 parent ef55076 commit 5013c07
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 14 deletions.
1 change: 1 addition & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 23 additions & 14 deletions lib/protocol/sequences/Sequence.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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');
};
22 changes: 22 additions & 0 deletions test/unit/connection/test-error-trace-bad-stack.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
20 changes: 20 additions & 0 deletions test/unit/connection/test-error-trace-no-stack.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
18 changes: 18 additions & 0 deletions test/unit/connection/test-error-trace.js
Original file line number Diff line number Diff line change
@@ -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();
});
});

0 comments on commit 5013c07

Please sign in to comment.