Commit fd022199 authored by Robert Knight's avatar Robert Knight

Implement client-side rate limiting per session

Previous experience with Sentry has found that there are various
scenarios that can cause a client to spam the server with a large number
of reports. Although Sentry has its own server-side quotas and rate
limiting, I think it will be useful to do client-side per-session rate
limiting as well. This also provides a place where we can hook in other
client-side filtering in future.
parent 542b96f0
...@@ -2,11 +2,16 @@ ...@@ -2,11 +2,16 @@
const Sentry = require('@sentry/browser'); const Sentry = require('@sentry/browser');
const warnOnce = require('../../shared/warn-once');
/** /**
* @typedef SentryConfig * @typedef SentryConfig
* @prop {string} dsn * @prop {string} dsn
*/ */
let eventsSent = 0;
const maxEventsToSendPerSession = 5;
/** /**
* Initialize the Sentry integration. * Initialize the Sentry integration.
* *
...@@ -19,6 +24,23 @@ function init(config) { ...@@ -19,6 +24,23 @@ function init(config) {
Sentry.init({ Sentry.init({
dsn: config.dsn, dsn: config.dsn,
release: '__VERSION__', // replaced by versionify release: '__VERSION__', // replaced by versionify
beforeSend: event => {
if (eventsSent >= maxEventsToSendPerSession) {
// Cap the number of events that any client instance will send, to
// reduce the impact on our Sentry event quotas.
//
// Sentry implements its own server-side rate limiting in addition.
// See https://docs.sentry.io/accounts/quotas/.
warnOnce(
'Client-side Sentry quota reached. No further Sentry events will be sent'
);
return null;
}
++eventsSent;
return event;
},
}); });
} }
...@@ -33,7 +55,17 @@ function setUserInfo(user) { ...@@ -33,7 +55,17 @@ function setUserInfo(user) {
Sentry.setUser(user); Sentry.setUser(user);
} }
/**
* Reset metrics used for client-side event filtering.
*/
function reset() {
eventsSent = 0;
}
module.exports = { module.exports = {
init, init,
setUserInfo, setUserInfo,
// Test helpers.
reset,
}; };
'use strict';
const sentry = require('../sentry');
describe('sidebar/util/sentry', () => {
let fakeSentry;
let fakeWarnOnce;
beforeEach(() => {
fakeSentry = {
init: sinon.stub(),
setUser: sinon.stub(),
};
fakeWarnOnce = sinon.stub();
sentry.$imports.$mock({
'@sentry/browser': fakeSentry,
'../../shared/warn-once': fakeWarnOnce,
});
});
afterEach(() => {
sentry.$imports.$restore();
});
it('limits the number of events sent to Sentry per session', () => {
sentry.init({ dsn: 'test-dsn' });
assert.called(fakeSentry.init);
// The first `maxEvents` events should be sent to Sentry.
const maxEvents = 5;
const beforeSend = fakeSentry.init.getCall(0).args[0].beforeSend;
for (let i = 0; i < maxEvents; i++) {
const val = {};
// These events should not be modified.
assert.equal(beforeSend(val), val);
}
assert.notCalled(fakeWarnOnce);
// Subsequent events should not be sent and a warning should be logged.
assert.equal(beforeSend({}), null);
assert.equal(beforeSend({}), null); // Verify this works a second time.
assert.called(fakeWarnOnce);
});
describe('setUserInfo', () => {
it('sets the Sentry user', () => {
sentry.setUserInfo({ id: 'acct:jimsmith@hypothes.is' });
// `setUserInfo` is currently a trivial wrapper.
assert.calledWith(fakeSentry.setUser, {
id: 'acct:jimsmith@hypothes.is',
});
});
});
});
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