From 1f36ba4b75dcfb3b40189a3d6366d7bc26e154e2 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 23 Aug 2022 14:44:14 -0400 Subject: [PATCH] Fix Sentry deduplication of events that were never sent (#15677) The Sentry `Dedupe` integration has been filtering out our events, even when they were never sent due to our `beforeSend` handler. It was wrongly identifying them as duplicates because it has no knowledge of `beforeSend` or whether they were actually sent or not. To resolve this, the filtering we were doing in `beforeSend` has been moved to a Sentry integration. This integration is installed ahead of the `Dedupe` integration, so `Dedupe` should never find out about any events that we filter out, and thus will never consider them as sent when they were not. --- app/scripts/lib/sentry-filter-events.ts | 73 +++++++++++++++++++++++++ app/scripts/lib/setupSentry.js | 38 +++++++++---- lavamoat/browserify/beta/policy.json | 54 +++++++++--------- lavamoat/browserify/flask/policy.json | 54 +++++++++--------- lavamoat/browserify/main/policy.json | 54 +++++++++--------- lavamoat/build-system/policy.json | 4 +- package.json | 4 +- yarn.lock | 4 +- 8 files changed, 187 insertions(+), 98 deletions(-) create mode 100644 app/scripts/lib/sentry-filter-events.ts diff --git a/app/scripts/lib/sentry-filter-events.ts b/app/scripts/lib/sentry-filter-events.ts new file mode 100644 index 000000000..050f0bcd2 --- /dev/null +++ b/app/scripts/lib/sentry-filter-events.ts @@ -0,0 +1,73 @@ +import { + Event as SentryEvent, + EventProcessor, + Hub, + Integration, +} from '@sentry/types'; +import { logger } from '@sentry/utils'; + +/** + * Filter events when MetaMetrics is disabled. + */ +export class FilterEvents implements Integration { + /** + * Property that holds the integration name. + */ + public static id = 'FilterEvents'; + + /** + * Another property that holds the integration name. + * + * I don't know why this exists, but the other Sentry integrations have it. + */ + public name: string = FilterEvents.id; + + /** + * A function that returns whether MetaMetrics is enabled. This should also + * return `false` if state has not yet been initialzed. + * + * @returns `true` if MetaMask's state has been initialized, and MetaMetrics + * is enabled, `false` otherwise. + */ + private getMetaMetricsEnabled: () => boolean; + + /** + * @param options - Constructor options. + * @param options.getMetaMetricsEnabled - A function that returns whether + * MetaMetrics is enabled. This should also return `false` if state has not + * yet been initialzed. + */ + constructor({ + getMetaMetricsEnabled, + }: { + getMetaMetricsEnabled: () => boolean; + }) { + this.getMetaMetricsEnabled = getMetaMetricsEnabled; + } + + /** + * Setup the integration. + * + * @param addGlobalEventProcessor - A function that allows adding a global + * event processor. + * @param getCurrentHub - A function that returns the current Sentry hub. + */ + public setupOnce( + addGlobalEventProcessor: (callback: EventProcessor) => void, + getCurrentHub: () => Hub, + ): void { + addGlobalEventProcessor((currentEvent: SentryEvent) => { + // Sentry integrations use the Sentry hub to get "this" references, for + // reasons I don't fully understand. + // eslint-disable-next-line consistent-this + const self = getCurrentHub().getIntegration(FilterEvents); + if (self) { + if (!self.getMetaMetricsEnabled()) { + logger.warn(`Event dropped due to MetaMetrics setting.`); + return null; + } + } + return currentEvent; + }); + } +} diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 21de22942..09d588454 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -2,6 +2,7 @@ import * as Sentry from '@sentry/browser'; import { Dedupe, ExtraErrorData } from '@sentry/integrations'; import { BuildType } from '../../../shared/constants/app'; +import { FilterEvents } from './sentry-filter-events'; import extractEthjsErrorMessage from './extractEthjsErrorMessage'; /* eslint-disable prefer-destructuring */ @@ -97,23 +98,36 @@ export default function setupSentry({ release, getState }) { sentryTarget = SENTRY_DSN_DEV; } + /** + * A function that returns whether MetaMetrics is enabled. This should also + * return `false` if state has not yet been initialzed. + * + * @returns `true` if MetaMask's state has been initialized, and MetaMetrics + * is enabled, `false` otherwise. + */ + function getMetaMetricsEnabled() { + if (getState) { + const appState = getState(); + if (!appState?.store?.metamask?.participateInMetaMetrics) { + return false; + } + } else { + return false; + } + return true; + } + Sentry.init({ dsn: sentryTarget, debug: METAMASK_DEBUG, environment, - integrations: [new Dedupe(), new ExtraErrorData()], + integrations: [ + new FilterEvents({ getMetaMetricsEnabled }), + new Dedupe(), + new ExtraErrorData(), + ], release, - beforeSend: (report) => { - if (getState) { - const appState = getState(); - if (!appState?.store?.metamask?.participateInMetaMetrics) { - return null; - } - } else { - return null; - } - return rewriteReport(report); - }, + beforeSend: (report) => rewriteReport(report), beforeBreadcrumb(breadcrumb) { if (getState) { const appState = getState(); diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 041b5d2f8..acd07769a 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -3260,9 +3260,9 @@ }, "packages": { "@sentry/browser>@sentry/core": true, - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core": { @@ -3273,9 +3273,9 @@ "packages": { "@sentry/browser>@sentry/core>@sentry/hub": true, "@sentry/browser>@sentry/core>@sentry/minimal": true, - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core>@sentry/hub": { @@ -3284,18 +3284,32 @@ "setInterval": true }, "packages": { - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core>@sentry/minimal": { "packages": { "@sentry/browser>@sentry/core>@sentry/hub": true, - "@sentry/browser>tslib": true + "@sentry/utils>tslib": true } }, - "@sentry/browser>@sentry/utils": { + "@sentry/integrations": { + "globals": { + "clearTimeout": true, + "console.error": true, + "console.log": true, + "setTimeout": true + }, + "packages": { + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true, + "localforage": true + } + }, + "@sentry/utils": { "globals": { "CustomEvent": true, "DOMError": true, @@ -3313,29 +3327,15 @@ "setTimeout": true }, "packages": { - "@sentry/browser>tslib": true, + "@sentry/utils>tslib": true, "browserify>process": true } }, - "@sentry/browser>tslib": { + "@sentry/utils>tslib": { "globals": { "define": true } }, - "@sentry/integrations": { - "globals": { - "clearTimeout": true, - "console.error": true, - "console.log": true, - "setTimeout": true - }, - "packages": { - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true, - "localforage": true - } - }, "@spruceid/siwe-parser": { "globals": { "console.error": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index dda355e88..b7ade56f4 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -3846,9 +3846,9 @@ }, "packages": { "@sentry/browser>@sentry/core": true, - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core": { @@ -3859,9 +3859,9 @@ "packages": { "@sentry/browser>@sentry/core>@sentry/hub": true, "@sentry/browser>@sentry/core>@sentry/minimal": true, - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core>@sentry/hub": { @@ -3870,18 +3870,32 @@ "setInterval": true }, "packages": { - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core>@sentry/minimal": { "packages": { "@sentry/browser>@sentry/core>@sentry/hub": true, - "@sentry/browser>tslib": true + "@sentry/utils>tslib": true } }, - "@sentry/browser>@sentry/utils": { + "@sentry/integrations": { + "globals": { + "clearTimeout": true, + "console.error": true, + "console.log": true, + "setTimeout": true + }, + "packages": { + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true, + "localforage": true + } + }, + "@sentry/utils": { "globals": { "CustomEvent": true, "DOMError": true, @@ -3899,29 +3913,15 @@ "setTimeout": true }, "packages": { - "@sentry/browser>tslib": true, + "@sentry/utils>tslib": true, "browserify>process": true } }, - "@sentry/browser>tslib": { + "@sentry/utils>tslib": { "globals": { "define": true } }, - "@sentry/integrations": { - "globals": { - "clearTimeout": true, - "console.error": true, - "console.log": true, - "setTimeout": true - }, - "packages": { - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true, - "localforage": true - } - }, "@spruceid/siwe-parser": { "globals": { "console.error": true, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 041b5d2f8..acd07769a 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -3260,9 +3260,9 @@ }, "packages": { "@sentry/browser>@sentry/core": true, - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core": { @@ -3273,9 +3273,9 @@ "packages": { "@sentry/browser>@sentry/core>@sentry/hub": true, "@sentry/browser>@sentry/core>@sentry/minimal": true, - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core>@sentry/hub": { @@ -3284,18 +3284,32 @@ "setInterval": true }, "packages": { - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true } }, "@sentry/browser>@sentry/core>@sentry/minimal": { "packages": { "@sentry/browser>@sentry/core>@sentry/hub": true, - "@sentry/browser>tslib": true + "@sentry/utils>tslib": true } }, - "@sentry/browser>@sentry/utils": { + "@sentry/integrations": { + "globals": { + "clearTimeout": true, + "console.error": true, + "console.log": true, + "setTimeout": true + }, + "packages": { + "@sentry/types": true, + "@sentry/utils": true, + "@sentry/utils>tslib": true, + "localforage": true + } + }, + "@sentry/utils": { "globals": { "CustomEvent": true, "DOMError": true, @@ -3313,29 +3327,15 @@ "setTimeout": true }, "packages": { - "@sentry/browser>tslib": true, + "@sentry/utils>tslib": true, "browserify>process": true } }, - "@sentry/browser>tslib": { + "@sentry/utils>tslib": { "globals": { "define": true } }, - "@sentry/integrations": { - "globals": { - "clearTimeout": true, - "console.error": true, - "console.log": true, - "setTimeout": true - }, - "packages": { - "@sentry/browser>@sentry/types": true, - "@sentry/browser>@sentry/utils": true, - "@sentry/browser>tslib": true, - "localforage": true - } - }, "@spruceid/siwe-parser": { "globals": { "console.error": true, diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index a9f2de281..ea09c605a 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -1086,7 +1086,7 @@ "@metamask/jazzicon>color>color-convert>color-name": true } }, - "@sentry/browser>tslib": { + "@sentry/utils>tslib": { "globals": { "define": true } @@ -1189,7 +1189,7 @@ }, "@typescript-eslint/eslint-plugin>tsutils": { "packages": { - "@sentry/browser>tslib": true, + "@sentry/utils>tslib": true, "typescript": true } }, diff --git a/package.json b/package.json index 40e9464e9..511d7a8dc 100644 --- a/package.json +++ b/package.json @@ -143,6 +143,8 @@ "@reduxjs/toolkit": "^1.6.2", "@sentry/browser": "^6.0.0", "@sentry/integrations": "^6.0.0", + "@sentry/types": "^6.0.1", + "@sentry/utils": "^6.0.1", "@spruceid/siwe-parser": "^1.1.3", "@truffle/codec": "^0.11.18", "@truffle/decoder": "^5.1.0", @@ -299,8 +301,8 @@ "browser-util-inspect": "^0.2.0", "browserify": "^16.5.1", "chalk": "^3.0.0", - "chromedriver": "^103.0.0", "chokidar": "^3.5.3", + "chromedriver": "^103.0.0", "concurrently": "^5.2.0", "copy-webpack-plugin": "^6.0.3", "cross-spawn": "^7.0.3", diff --git a/yarn.lock b/yarn.lock index e3e5293cf..a8849f7c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3495,12 +3495,12 @@ "@sentry/types" "6.13.3" tslib "^1.9.3" -"@sentry/types@6.13.3": +"@sentry/types@6.13.3", "@sentry/types@^6.0.1": version "6.13.3" resolved "https://registry.yarnpkg.com/@sentry/types/-/types-6.13.3.tgz#63ad5b6735b0dfd90b3a256a9f8e77b93f0f66b2" integrity sha512-Vrz5CdhaTRSvCQjSyIFIaV9PodjAVFkzJkTRxyY7P77RcegMsRSsG1yzlvCtA99zG9+e6MfoJOgbOCwuZids5A== -"@sentry/utils@6.13.3": +"@sentry/utils@6.13.3", "@sentry/utils@^6.0.1": version "6.13.3" resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-6.13.3.tgz#188754d40afe693c3fcae410f9322531588a9926" integrity sha512-zYFuFH3MaYtBZTeJ4Yajg7pDf0pM3MWs3+9k5my9Fd+eqNcl7dYQYJbT9gyC0HXK1QI4CAMNNlHNl4YXhF91ag==