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

Commit

Permalink
fix: Do not override context when capturing non-error exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
kamilogorek committed Apr 9, 2018
1 parent 530c258 commit b5e306c
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 14 deletions.
24 changes: 14 additions & 10 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ extend(Raven.prototype, {
cb = kwargs;
kwargs = {};
} else {
kwargs = kwargs || {};
// Do not use reference, as we'll modify this object later on
kwargs = kwargs ? JSON.parse(stringify(kwargs)) : {};
}

var eventId = this.generateEventId();
Expand Down Expand Up @@ -363,25 +364,28 @@ extend(Raven.prototype, {
cb = kwargs;
kwargs = {};
} else {
kwargs = kwargs || {};
var req = kwargs && kwargs.req;
// Do not use reference, as we'll modify this object later on
kwargs = kwargs ? JSON.parse(stringify(kwargs)) : {};
// Preserve `req` so we can use some framework's middleware methods
// `req` is the only thing that we pass through in `errorHandler`, so we need just that
if (req) kwargs.req = req;
}

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
var keys = Object.keys(err).sort();
var hash = md5(keys);
var message =
'Non-Error exception captured with keys: ' +
utils.serializeKeysForMessage(keys);
var serializedException = utils.serializeException(err);

kwargs.message = message;
kwargs.fingerprint = [hash];
kwargs.extra = {
__serialized__: serializedException
};
kwargs = extend(kwargs, {
message: message,
fingerprint: [md5(keys)],
extra: kwargs.extra || {}
});
kwargs.extra.__serialized__ = utils.serializeException(err);

err = new Error(message);
} else {
Expand Down
78 changes: 74 additions & 4 deletions test/raven.client.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,22 @@ describe('raven.Client', function() {
done();
});
});

it('should copy object with extra data instead of using its reference directly', function(done) {
var old = client.send;
var info = {
extra: {
hello: 'there'
}
};
client.send = function mockSend(kwargs) {
client.send = old;
kwargs.extra.should.have.property('hello', 'there');
kwargs.extra.should.not.equal(info);
done();
};
client.captureMessage('exception', info);
});
});

describe('#captureException()', function() {
Expand Down Expand Up @@ -259,13 +275,9 @@ describe('raven.Client', function() {
var old = client.send;
client.send = function mockSend(kwargs) {
client.send = old;

kwargs.message.should.equal(
'Non-Error exception captured with keys: aKeyOne, bKeyTwo, cKeyThree, dKeyFour\u2026'
);

// Remove superfluous node version data to simplify the test itself
delete kwargs.extra.node;
kwargs.extra.should.have.property('__serialized__', {
aKeyOne: 'a',
bKeyTwo: 42,
Expand Down Expand Up @@ -385,6 +397,64 @@ describe('raven.Client', function() {
done();
});
});

it('should use and merge provided extra data instead of overriding it', function(done) {
var old = client.send;
client.send = function mockSend(kwargs) {
client.send = old;
kwargs.extra.should.have.property('hello', 'there');
kwargs.tags.should.deepEqual({'0': 'whoop'});
done();
};
client.captureException(
{some: 'exception'},
{
extra: {
hello: 'there'
},
tags: ['whoop']
}
);
});

it('should copy object with extra data instead of using its reference directly', function(done) {
var old = client.send;
var info = {
extra: {
hello: 'there'
}
};
client.send = function mockSend(kwargs) {
client.send = old;
kwargs.extra.should.have.property('hello', 'there');
kwargs.extra.should.not.equal(info.extra);
done();
};
client.captureException({some: 'exception'}, info);
});

it('should preserve same reference to `req` attribute in kwargs', function(done) {
var old = client.process;
var info = {
extra: {
hello: 'there'
},
req: {
something: 'else'
}
};
// Use `process` instead of `send` as `req` is stripped from the final payload
client.process = function mockProcess(id, kwargs) {
client.process = old;
kwargs.extra.should.have.property('hello', 'there');
kwargs.extra.should.not.equal(info.extra);

kwargs.req.should.have.property('something', 'else');
kwargs.req.should.equal(info.req);
done();
};
client.captureException({some: 'exception'}, info);
});
});

describe('#install()', function() {
Expand Down

0 comments on commit b5e306c

Please sign in to comment.