Skip to content

fix: refresh access token on policy retry #8149

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.

Sign up for GitHub

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

Draft
mikesposito wants to merge 2 commits into
base: main
Choose a base branch
from
+8 −4
Draft

fix: refresh access token on policy retry #8149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9926729
fix: refresh access token on policy retry
mikesposito Mar 9, 2026
6232111
update changelog
mikesposito Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/profile-metrics-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- `ProfileMetricsService` recreates access token on each API call attempt ([#8149](https://github.com/MetaMask/core/pull/8149))
Copy link
Contributor

@NicolasMassart NicolasMassart Mar 10, 2026

Choose a reason for hiding this comment

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

suggestion (non-blocking): Changelog entry wording could be clearer

The changelog could be misread as a new behavior on every first call. It's specifically about retries within the service policy only.


### Changed

- Bump `@metamask/accounts-controller` from `^36.0.0` to `^36.0.1` ([#7996](https://github.com/MetaMask/core/pull/7996))
Expand Down
8 changes: 4 additions & 4 deletions packages/profile-metrics-controller/src/ProfileMetricsService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ export class ProfileMetricsService {
* @returns The response from the API.
*/
async submitMetrics(data: ProfileMetricsSubmitMetricsRequest): Promise<void> {
const authToken = await this.#messenger.call(
'AuthenticationController:getBearerToken',
data.entropySourceId ?? undefined,
);
await this.#policy.execute(async () => {
const authToken = await this.#messenger.call(
'AuthenticationController:getBearerToken',
Copy link
Contributor

@NicolasMassart NicolasMassart Mar 10, 2026

Choose a reason for hiding this comment

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

question (non-blocking): Is there a risk of getBearerToken itself triggering retries or delays inside the policy?

getBearerToken can itself perform a full login flow (nonce fetch, snap signing, OIDC exchange) if the session is expired. Now that this call is inside policy.execute, any latency or transient failure from the auth flow will count against the retry policy's timeout/backoff budget. If getBearerToken throws (e.g. network error during auth), the policy will retry the entire callback including auth. This is likely fine (and probably desirable), but worth confirming this is intentional.

data.entropySourceId ?? undefined,
);
Comment on lines +202 to +205
Copy link
Contributor

@NicolasMassart NicolasMassart Mar 10, 2026

Choose a reason for hiding this comment

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

issue (blocking): No test validates that the token is refreshed on retry

const url = new URL(`${this.#baseURL}/profile/accounts`);
const localResponse = await this.#fetch(url, {
method: 'PUT',
Expand Down
Loading