Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

feat: Sensible non-Error exception serializer #416

Merged
merged 4 commits into from
Jan 15, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var transports = require('./transports');
var nodeUtil = require('util'); // nodeUtil to avoid confusion with "utils"
var events = require('events');
var domain = require('domain');
var md5 = require('md5');

var instrumentor = require('./instrumentation/instrumentor');

Expand Down Expand Up @@ -355,20 +356,36 @@ extend(Raven.prototype, {
},

captureException: function captureException(err, kwargs, cb) {
if (!(err instanceof Error)) {
// This handles when someone does:
// throw "something awesome";
// We synthesize an Error here so we can extract a (rough) stack trace.
err = new Error(err);
}

if (!cb && typeof kwargs === 'function') {
cb = kwargs;
kwargs = {};
} else {
kwargs = kwargs || {};
}

if (!(err instanceof Error)) {
if (utils.isPlainObject(err)) {
// This will allow us to group events based on top-level keys
// which is much better than creating new group when any key/value change
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, that's a great idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, that's awesome kamil. do they need to be hashed? I guess it's a length thing, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without hashing, very large objects will create way too large fingerprints. This way it's always fixed length as you mentioned.

var hash = md5(Object.keys(err).sort());
var message = 'Sentry: non-Error exception captured [' + hash + ']';
Copy link
Contributor

@benvinegar benvinegar Jan 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that we need to prefix Sentry: here.

What about:

Non-Error exception captured with keys foo, bar, ...

^Something like that, only showing the first 2 or 3 keys. That'll be more grokkable than the fingerprint.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

var serializedException = utils.serializeException(err);

kwargs.message = message;
kwargs.fingerprint = [hash];
kwargs.extra = {
__serialized__: serializedException
};

err = new Error(message);
} else {
// This handles when someone does:
// throw "something awesome";
// We synthesize an Error here so we can extract a (rough) stack trace.
err = new Error(err);
}
}

var self = this;
var eventId = this.generateEventId();
parsers.parseError(err, kwargs, function(kw) {
Expand Down
74 changes: 74 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var transports = require('./transports');
var path = require('path');
var lsmod = require('lsmod');
var stacktrace = require('stack-trace');
var stringify = require('../vendor/json-stringify-safe');

var ravenVersion = require('../package.json').version;

Expand All @@ -16,6 +17,79 @@ var protocolMap = {

var consoleAlerts = {};

var MAX_SERIALIZE_EXCEPTION_DEPTH = 4;
var MAX_SERIALIZE_EXCEPTION_SIZE = 50 * 1024; // 50kB

function utf8Length(value) {
return ~-encodeURI(value).split(/%..|./).length;
}

function jsonSize(value) {
return utf8Length(JSON.stringify(value));
}

function isPlainObject(what) {
return Object.prototype.toString.call(what) === '[object Object]';
}

module.exports.isPlainObject = isPlainObject;

function serializeValue(value) {
var maxLength = 40;

if (typeof value === 'string') {
return value.length <= maxLength ? value : value.substr(0, maxLength) + '\u2026';
} else if (
typeof value === 'number' ||
typeof value === 'boolean' ||
typeof value === 'undefined'
) {
return value;
} else {
return Object.prototype.toString.call(value);
}
}

function serializeObject(value, depth) {
if (depth === 0) return serializeValue(value);

if (isPlainObject(value)) {
return Object.keys(value).reduce(function(acc, key) {
acc[key] = serializeObject(value[key], depth - 1);
return acc;
}, {});
} else if (Array.isArray(value)) {
return value.map(function(val) {
return serializeObject(val, depth - 1);
});
} else {
return serializeValue(value);
}
}

function serializeException(ex, depth, maxSize) {
if (!isPlainObject(ex)) return ex;

depth =
depth === undefined || typeof depth !== 'number'
? MAX_SERIALIZE_EXCEPTION_DEPTH
: depth;
maxSize =
maxSize === undefined || typeof depth !== 'number'
? MAX_SERIALIZE_EXCEPTION_SIZE
: maxSize;

var serialized = serializeObject(ex, depth);

if (jsonSize(stringify(serialized)) > maxSize) {
return serializeException(ex, depth - 1);
} else {
return serialized;
}
}

module.exports.serializeException = serializeException;

module.exports.disableConsoleAlerts = function disableConsoleAlerts() {
consoleAlerts = false;
};
Expand Down
172 changes: 172 additions & 0 deletions test/raven.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
'use strict';

var raven = require('../');
var stringify = require('../vendor/json-stringify-safe');

describe('raven.utils', function() {
describe('#parseDSN()', function() {
Expand Down Expand Up @@ -330,4 +331,175 @@ describe('raven.utils', function() {
raven.utils.getModule(filename).should.eql('lol');
});
});

describe.only('#serializeException()', function() {
it('return [object Object] when reached depth=0', function() {
var actual = raven.utils.serializeException(
{
a: 42,
b: 'asd',
c: true
},
0
);
var expected = stringify('[object Object]');

actual.should.eql(expected);
});

it('should serialize one level deep with depth=1', function() {
var actual = raven.utils.serializeException(
{
a: 42,
b: 'asd',
c: true,
d: undefined,
e:
'very long string that is definitely over 120 characters, which is default for now but can be changed anytime because why not?',
f: {foo: 42},
g: [1, 'a', true],
h: function() {}
},
1
);
var expected = stringify({
a: 42,
b: 'asd',
c: true,
d: undefined,
e: 'very long string that is definitely over\u2026',
f: '[object Object]',
g: '[object Array]',
h: '[object Function]'
});

actual.should.eql(expected);
});

it('should serialize arbitrary number of depths', function() {
var actual = raven.utils.serializeException(
{
a: 42,
b: 'asd',
c: true,
d: undefined,
e:
'very long string that is definitely over 40 characters, which is default for now but can be changed',
f: {
foo: 42,
bar: {
foo: 42,
bar: {
bar: {
bar: {
bar: 42
}
}
},
baz: ['hello']
},
baz: [1, 'a', true]
},
g: [1, 'a', true],
h: function() {}
},
5
);
var expected = stringify({
a: 42,
b: 'asd',
c: true,
d: undefined,
e: 'very long string that is definitely over\u2026',
f: {
foo: 42,
bar: {
foo: 42,
bar: {
bar: {
bar: '[object Object]'
}
},
baz: ['hello']
},
baz: [1, 'a', true]
},
g: [1, 'a', true],
h: '[object Function]'
});

actual.should.eql(expected);
});

it('should reduce depth if payload size was exceeded', function() {
var actual = raven.utils.serializeException(
{
a: {
a: '50kB worth of payload pickle rick',
b: '50kB worth of payload pickle rick'
},
b: '50kB worth of payload pickle rick'
},
2,
100
);
var expected = stringify({
a: '[object Object]',
b: '50kB worth of payload pickle rick'
});

actual.should.eql(expected);
});

it('should reduce depth only one level at the time', function() {
var actual = raven.utils.serializeException(
{
a: {
a: {
a: {
a: [
'50kB worth of payload pickle rick',
'50kB worth of payload pickle rick',
'50kB worth of payload pickle rick'
]
}
},
b: '50kB worth of payload pickle rick'
},
b: '50kB worth of payload pickle rick'
},
4,
200
);
var expected = stringify({
a: {
a: {
a: {
a: '[object Array]'
}
},
b: '50kB worth of payload pickle rick'
},
b: '50kB worth of payload pickle rick'
});

actual.should.eql(expected);
});

it('should fallback to [object Object] if cannot reduce payload size enough', function() {
var actual = raven.utils.serializeException(
{
a: '50kB worth of payload pickle rick',
b: '50kB worth of payload pickle rick',
c: '50kB worth of payload pickle rick',
d: '50kB worth of payload pickle rick'
},
1,
100
);
var expected = stringify('[object Object]');

actual.should.eql(expected);
});
});
});