Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade to Sentry 8 #25999

Merged
merged 11 commits into from
Jul 29, 2024
87 changes: 21 additions & 66 deletions app/scripts/lib/sentry-filter-events.ts
Original file line number Diff line number Diff line change
@@ -1,73 +1,28 @@
import {
Event as SentryEvent,
EventProcessor,
Hub,
Integration,
} from '@sentry/types';
import { Event as SentryEvent, Integration } from '@sentry/types';
import { logger } from '@sentry/utils';

const NAME = 'FilterEvents';

/**
* Filter events when MetaMetrics is disabled.
*
* @param options - Options bag.
* @param options.getMetaMetricsEnabled - Function that returns whether MetaMetrics is enabled.
*/
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: () => Promise<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: () => Promise<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(async (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 (!(await self.getMetaMetricsEnabled())) {
logger.warn(`Event dropped due to MetaMetrics setting.`);
return null;
}
export function filterEvents({
getMetaMetricsEnabled,
}: {
getMetaMetricsEnabled: () => Promise<boolean>;
}): Integration {
Comment on lines +12 to +16
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info: Refactored the FilterEvents class into a function that returns an integration object.

return {
name: NAME,
processEvent: async (event: SentryEvent) => {
if (!(await getMetaMetricsEnabled())) {
logger.warn(`Event dropped due to MetaMetrics setting.`);
return null;
Comment on lines +20 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info: Removed the use of the Sentry hub, simplifying the event processing logic.

}
return currentEvent;
});
}

return event;
},
};
}
39 changes: 23 additions & 16 deletions app/scripts/lib/setupSentry.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import * as Sentry from '@sentry/browser';
import { Dedupe, ExtraErrorData } from '@sentry/integrations';

import { AllProperties } from '../../../shared/modules/object.utils';
import { FilterEvents } from './sentry-filter-events';
import { filterEvents } from './sentry-filter-events';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info: Refactored FilterEvents integration to be an object returned by a function.

import extractEthjsErrorMessage from './extractEthjsErrorMessage';

/* eslint-disable prefer-destructuring */
Expand Down Expand Up @@ -592,6 +591,14 @@ export default function setupSentry({ release, getState }) {
return getMetaMetricsEnabled();
}

/**
* Sentry throws on initialization as it wants to avoid polluting the global namespace and
* potentially clashing with a website also using Sentry, but this could only happen in the content script.
* This emulates NW.js which disables these validations.
* https://docs.sentry.io/platforms/javascript/best-practices/shared-environments/
*/
globalThis.nw = {};

Sentry.init({
dsn: sentryTarget,
debug: METAMASK_DEBUG,
Expand Down Expand Up @@ -634,10 +641,10 @@ export default function setupSentry({ release, getState }) {
*
* @see https://github.com/MetaMask/metamask-extension/pull/15677
*/
new FilterEvents({ getMetaMetricsEnabled }),
new Dedupe(),
new ExtraErrorData(),
new Sentry.BrowserProfilingIntegration(),
filterEvents({ getMetaMetricsEnabled }),
Comment on lines 641 to +644
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Info: Replaced deprecated Sentry integrations with updated methods.

Sentry.dedupeIntegration(),
Sentry.extraErrorDataIntegration(),
Sentry.browserProfilingIntegration(),
],
release,
/**
Expand Down Expand Up @@ -666,11 +673,11 @@ export default function setupSentry({ release, getState }) {
* a new sentry session.
*/
const startSession = async () => {
const hub = Sentry.getCurrentHub?.();
const options = hub.getClient?.().getOptions?.() ?? {};
if (hub && (await getSentryEnabled()) === true) {
const client = Sentry.getClient();
const options = client?.getOptions?.() ?? {};
if (client && (await getSentryEnabled()) === true) {
options.autoSessionTracking = true;
hub.startSession();
Sentry.startSession();
}
};

Expand All @@ -680,11 +687,11 @@ export default function setupSentry({ release, getState }) {
* the current sentry session.
*/
const endSession = async () => {
const hub = Sentry.getCurrentHub?.();
const options = hub.getClient?.().getOptions?.() ?? {};
if (hub && (await getSentryEnabled()) === false) {
const client = Sentry.getClient();
const options = client?.getOptions?.() ?? {};
if (client && (await getSentryEnabled()) === false) {
options.autoSessionTracking = false;
hub.endSession();
Sentry.endSession();
}
};

Expand All @@ -694,8 +701,8 @@ export default function setupSentry({ release, getState }) {
* the Sentry client.
*/
const toggleSession = async () => {
const hub = Sentry.getCurrentHub?.();
const options = hub.getClient?.().getOptions?.() ?? {
const client = Sentry.getClient();
const options = client?.getOptions?.() ?? {
autoSessionTracking: false,
};
const isSentryEnabled = await getSentryEnabled();
Expand Down
2 changes: 2 additions & 0 deletions development/build/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const scuttlingConfigBase = {
encodeURIComponent: '',
console: '',
crypto: '',
Map: '',
// {clear/set}Timeout are "this sensitive"
clearTimeout: 'window',
setTimeout: 'window',
Expand All @@ -88,6 +89,7 @@ const scuttlingConfigBase = {
appState: '',
extra: '',
stateHooks: '',
nw: '',
},
};

Expand Down
Loading