Unverified Commit 952443c1 authored by Robert Knight's avatar Robert Knight Committed by GitHub

Merge pull request #1345 from hypothesis/add-sentry-url-whitelist

Whitelist Sentry error reports to only include errors from our code
parents 64b139cd 0b934afd
...@@ -13,6 +13,22 @@ const warnOnce = require('../../shared/warn-once'); ...@@ -13,6 +13,22 @@ const warnOnce = require('../../shared/warn-once');
let eventsSent = 0; let eventsSent = 0;
const maxEventsToSendPerSession = 5; const maxEventsToSendPerSession = 5;
/**
* Return the origin which the current script comes from.
*
* @return {string|null}
*/
function currentScriptOrigin() {
try {
// nb. IE 11 does not support `document.currentScript` and this property
// is only available while a `<script>` tag is initially being executed.
const scriptUrl = new URL(document.currentScript.src);
return scriptUrl.origin;
} catch (e) {
return null;
}
}
/** /**
* Initialize the Sentry integration. * Initialize the Sentry integration.
* *
...@@ -22,10 +38,23 @@ const maxEventsToSendPerSession = 5; ...@@ -22,10 +38,23 @@ const maxEventsToSendPerSession = 5;
* @param {SentryConfig} config * @param {SentryConfig} config
*/ */
function init(config) { function init(config) {
// Only send events for errors which can be attributed to our code. This
// reduces noise in Sentry caused by errors triggered by eg. script tags added
// by browser extensions. The downside is that this may cause us to miss errors
// which are caused by our code but, for any reason, cannot be attributed to
// it. This logic assumes that all of our script bundles are served from
// the same origin as the bundle which includes this module.
//
// If we can't determine the current script's origin, just disable the
// whitelist and report all errors.
const scriptOrigin = currentScriptOrigin();
const whitelistUrls = scriptOrigin ? [scriptOrigin] : null;
Sentry.init({ Sentry.init({
dsn: config.dsn, dsn: config.dsn,
environment: config.environment, environment: config.environment,
release: '__VERSION__', // replaced by versionify release: '__VERSION__', // replaced by versionify
whitelistUrls,
// See https://docs.sentry.io/error-reporting/configuration/filtering/?platform=javascript#before-send // See https://docs.sentry.io/error-reporting/configuration/filtering/?platform=javascript#before-send
beforeSend: (event, hint) => { beforeSend: (event, hint) => {
......
...@@ -4,6 +4,7 @@ const sentry = require('../sentry'); ...@@ -4,6 +4,7 @@ const sentry = require('../sentry');
describe('sidebar/util/sentry', () => { describe('sidebar/util/sentry', () => {
let fakeDocumentReferrer; let fakeDocumentReferrer;
let fakeDocumentCurrentScript;
let fakeSentry; let fakeSentry;
let fakeWarnOnce; let fakeWarnOnce;
...@@ -31,11 +32,18 @@ describe('sidebar/util/sentry', () => { ...@@ -31,11 +32,18 @@ describe('sidebar/util/sentry', () => {
fakeDocumentReferrer = sinon.stub(document, 'referrer'); fakeDocumentReferrer = sinon.stub(document, 'referrer');
fakeDocumentReferrer.get(() => 'https://example.com'); fakeDocumentReferrer.get(() => 'https://example.com');
fakeDocumentCurrentScript = sinon.stub(document, 'currentScript');
fakeDocumentCurrentScript.get(() => ({
src:
'https://cdn.hypothes.is/hypothesis/1.123.0/build/scripts/sidebar.bundle.js',
}));
// Reset rate limiting counters. // Reset rate limiting counters.
sentry.reset(); sentry.reset();
}); });
afterEach(() => { afterEach(() => {
fakeDocumentCurrentScript.restore();
fakeDocumentReferrer.restore(); fakeDocumentReferrer.restore();
}); });
...@@ -55,6 +63,36 @@ describe('sidebar/util/sentry', () => { ...@@ -55,6 +63,36 @@ describe('sidebar/util/sentry', () => {
); );
}); });
it('configures Sentry to only report errors that can be attributed to our code', () => {
sentry.init({
dsn: 'test-dsn',
environment: 'dev',
});
assert.calledWith(
fakeSentry.init,
sinon.match({
whitelistUrls: ['https://cdn.hypothes.is'],
})
);
});
it('disables the URL whitelist if `document.currentScript` is inaccessible', () => {
fakeDocumentCurrentScript.get(() => null);
sentry.init({
dsn: 'test-dsn',
environment: 'dev',
});
assert.calledWith(
fakeSentry.init,
sinon.match({
whitelistUrls: null,
})
);
});
it('adds extra context to reports', () => { it('adds extra context to reports', () => {
sentry.init({ dsn: 'test-dsn', environment: 'dev' }); sentry.init({ dsn: 'test-dsn', environment: 'dev' });
assert.calledWith( assert.calledWith(
......
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