Commit 5169e961 authored by Robert Knight's avatar Robert Knight

Add custom serialization for `Event` exception values in Sentry reporting

To help debug https://sentry.io/organizations/hypothesis/issues/1190575523
and similar issues, add logic to extract details of uncaught thrown
`Event`s and attach them to Sentry reports.
parent 4865ee82
...@@ -27,7 +27,7 @@ function init(config) { ...@@ -27,7 +27,7 @@ function init(config) {
environment: config.environment, environment: config.environment,
release: '__VERSION__', // replaced by versionify release: '__VERSION__', // replaced by versionify
beforeSend: event => { beforeSend: (event, hint) => {
if (eventsSent >= maxEventsToSendPerSession) { if (eventsSent >= maxEventsToSendPerSession) {
// Cap the number of events that any client instance will send, to // Cap the number of events that any client instance will send, to
// reduce the impact on our Sentry event quotas. // reduce the impact on our Sentry event quotas.
...@@ -39,8 +39,25 @@ function init(config) { ...@@ -39,8 +39,25 @@ function init(config) {
); );
return null; return null;
} }
++eventsSent; ++eventsSent;
// Add additional debugging information for non-Error exception types
// which Sentry can't serialize to a useful format automatically.
//
// See https://github.com/getsentry/sentry-javascript/issues/2210
try {
const originalErr = hint && hint.originalException;
if (originalErr instanceof Event) {
Object.assign(event.extra, {
type: originalErr.type,
detail: originalErr.detail,
isTrusted: originalErr.isTrusted,
});
}
} catch (e) {
// If something went wrong serializing the data, just ignore it.
}
return event; return event;
}, },
}); });
......
...@@ -30,6 +30,9 @@ describe('sidebar/util/sentry', () => { ...@@ -30,6 +30,9 @@ describe('sidebar/util/sentry', () => {
beforeEach(() => { beforeEach(() => {
fakeDocumentReferrer = sinon.stub(document, 'referrer'); fakeDocumentReferrer = sinon.stub(document, 'referrer');
fakeDocumentReferrer.get(() => 'https://example.com'); fakeDocumentReferrer.get(() => 'https://example.com');
// Reset rate limiting counters.
sentry.reset();
}); });
afterEach(() => { afterEach(() => {
...@@ -61,15 +64,19 @@ describe('sidebar/util/sentry', () => { ...@@ -61,15 +64,19 @@ describe('sidebar/util/sentry', () => {
); );
}); });
function getBeforeSendHook() {
return fakeSentry.init.getCall(0).args[0].beforeSend;
}
it('limits the number of events sent to Sentry per session', () => { it('limits the number of events sent to Sentry per session', () => {
sentry.init({ dsn: 'test-dsn' }); sentry.init({ dsn: 'test-dsn' });
assert.called(fakeSentry.init); assert.called(fakeSentry.init);
// The first `maxEvents` events should be sent to Sentry. // The first `maxEvents` events should be sent to Sentry.
const maxEvents = 5; const maxEvents = 5;
const beforeSend = fakeSentry.init.getCall(0).args[0].beforeSend; const beforeSend = getBeforeSendHook();
for (let i = 0; i < maxEvents; i++) { for (let i = 0; i < maxEvents; i++) {
const val = {}; const val = { extra: {} };
// These events should not be modified. // These events should not be modified.
assert.equal(beforeSend(val), val); assert.equal(beforeSend(val), val);
...@@ -81,6 +88,44 @@ describe('sidebar/util/sentry', () => { ...@@ -81,6 +88,44 @@ describe('sidebar/util/sentry', () => {
assert.equal(beforeSend({}), null); // Verify this works a second time. assert.equal(beforeSend({}), null); // Verify this works a second time.
assert.called(fakeWarnOnce); assert.called(fakeWarnOnce);
}); });
it('extracts metadata from thrown `Event`s', () => {
sentry.init({ dsn: 'test-dsn' });
const beforeSend = getBeforeSendHook();
const event = { extra: {} };
beforeSend(event, {
originalException: new CustomEvent('unexpectedevent', {
detail: 'Details of the unexpected event',
}),
});
assert.deepEqual(event, {
extra: {
type: 'unexpectedevent',
detail: 'Details of the unexpected event',
isTrusted: false,
},
});
});
it('ignores errors serializing non-Error exception values', () => {
sentry.init({ dsn: 'test-dsn' });
const beforeSend = getBeforeSendHook();
const event = { extra: {} };
const originalException = new CustomEvent('unexpectedevent');
Object.defineProperty(originalException, 'detail', {
get: () => {
throw new Error('Something went wrong');
},
});
beforeSend(event, { originalException });
// Serializing the custom event detail will fail, so that data will simply
// be omitted from the report.
assert.deepEqual(event.extra, {});
});
}); });
describe('setUserInfo', () => { describe('setUserInfo', () => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment