Skip to content

Commit 97a0073

Browse files
authored
chore: add sentry events for state splitting migration (#38978)
## **Description** Example: https://metamask.sentry.io/insights/frontend/summary/events/?project=273496&query=tags%5Btransaction%5D%3Atask%2FmigrateToSplitState&sort=-timestamp&statsPeriod=1h&transaction=task%2FmigrateToSplitState [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38978?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null <!-- ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds observability around split-state migration. > > - New `utils/run-tracked-task.ts` helper starts a Sentry span (`task/<name>`), sets status, tags errors, captures exceptions, and rethrows > - Wraps `PersistenceManager.migrateToSplitState` in `runTrackedTask('migrateToSplitState', ...)`; migration logic unchanged aside from tracing > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a1ac9bf. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f4eb205 commit 97a0073

File tree

2 files changed

+83
-20
lines changed

2 files changed

+83
-20
lines changed

app/scripts/lib/stores/persistence-manager.ts

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
BaseStore,
1212
MetaData,
1313
} from './base-store';
14+
import { runTrackedTask } from './utils/run-tracked-task';
1415

1516
export type StorageKind = 'data' | 'split';
1617

@@ -568,29 +569,33 @@ export class PersistenceManager {
568569
* occur.
569570
*/
570571
async migrateToSplitState(state: MetaMaskStateType) {
571-
if (this.storageKind === 'split') {
572-
log.debug('[Split State]: Storage is already split, skipping migration');
573-
return;
574-
}
572+
return runTrackedTask('migrateToSplitState', async () => {
573+
if (this.storageKind === 'split') {
574+
log.debug(
575+
'[Split State]: Storage is already split, skipping migration',
576+
);
577+
return;
578+
}
575579

576-
if (!this.#metadata) {
577-
throw new Error(
578-
'MetaMask - metadata must be set before calling "migrateToSplitState"',
579-
);
580-
}
580+
if (!this.#metadata) {
581+
throw new Error(
582+
'MetaMask - metadata must be set before calling "migrateToSplitState"',
583+
);
584+
}
581585

582-
this.storageKind = 'split';
583-
const metadata = structuredClone(this.#metadata);
584-
metadata.storageKind = 'split';
585-
this.setMetadata(metadata);
586-
for (const [key, value] of Object.entries(state)) {
587-
this.update(key, value);
588-
}
586+
this.storageKind = 'split';
587+
const metadata = structuredClone(this.#metadata);
588+
metadata.storageKind = 'split';
589+
this.setMetadata(metadata);
590+
for (const [key, value] of Object.entries(state)) {
591+
this.update(key, value);
592+
}
589593

590-
// mark data key for deletion
591-
this.update('data', undefined);
594+
// mark data key for deletion
595+
this.update('data', undefined);
592596

593-
log.debug('[Split State]: Migrating to split state storage');
594-
await this.persist();
597+
log.debug('[Split State]: Migrating to split state storage');
598+
await this.persist();
599+
});
595600
}
596601
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import type * as Sentry from '@sentry/browser';
2+
3+
const sentry = globalThis.sentry as typeof Sentry | undefined;
4+
5+
export async function runTrackedTask<Return>(
6+
taskName: string,
7+
fn: () => Promise<Return>,
8+
): Promise<Return> {
9+
if (!sentry) {
10+
return fn();
11+
}
12+
13+
const attributes = {
14+
'task.name': taskName,
15+
};
16+
17+
return sentry.startSpan(
18+
{
19+
name: `task/${taskName}`,
20+
op: 'task',
21+
forceTransaction: true,
22+
attributes,
23+
},
24+
async (span) => {
25+
try {
26+
const result = await fn();
27+
28+
span.setStatus({ code: 1 });
29+
30+
return result;
31+
} catch (err) {
32+
span.setStatus({
33+
code: 2, // 2 === SPAN_STATUS_ERROR
34+
message: 'internal_error',
35+
});
36+
span.setAttribute(
37+
'task.error_message',
38+
err instanceof Error ? err.message : String(err),
39+
);
40+
41+
// Creates an *error event* (Issue), separate from the span.
42+
sentry.captureException(err, {
43+
tags: { 'task.name': taskName, 'task.status': 'failed' },
44+
});
45+
46+
// rethrow the error after capturing it so the process crashes, as
47+
// we can't know how to recover from here. This will result in the
48+
// exception being captured by sentry _again_ by the global error
49+
// handler, and that's fine. We capture here because we want to add this
50+
// taskName tag to the error event so we can easily find and filter it
51+
// later. All this is sentry tracking code in here is temporary until we
52+
// are confident that the migration code is stable enough, and we can
53+
// roll it out to 100%.
54+
throw err;
55+
}
56+
},
57+
);
58+
}

0 commit comments

Comments
 (0)