Skip to content

Commit 0f3d948

Browse files
authored
fix: handle polling subscriptions after subscription with card cp-13.13.0 (#38610)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Previously after subscription with card we navigate to shield settings screen with wait for active subscriptions params but in background we only fetch subscription once so if stripe event handle in backend is slower than client fetch, it would timeout in shield settings screen. This PR add subscriptions polling after subscription with card stripe checkout succeeded to ensure we fetch for latest subscription created. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38610?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: fix shield subscription subscribe with card race condition ## **Related issues** Fixes: #38615 ## **Manual testing steps** 1. Go to shield plan 2. Subscribe with card 3. After succeed should navigate to shield settings screen and wait for subscription creation ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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 - [x] 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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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 polling after card checkout to wait for active/paused Shield subscription with configurable interval/timeout, and centralizes Shield error constants; updates tests and UI usage. > > - **Subscription Service** (`app/scripts/services/subscription/subscription-service.ts`): > - Add polling (`#pollForSubscription`) using `getIsShieldSubscriptionActive/Paused`, with defaults `SUBSCRIPTION_POLL_INTERVAL` and `SUBSCRIPTION_POLL_TIMEOUT`; throws `SHIELD_ERROR.subscriptionPollingTimedOut`. > - `startSubscriptionWithCard` now polls until subscription exists; accepts optional `subscriptionPollInterval`/`subscriptionPollTimeout`; on "already exists" error, refreshes `getSubscriptions`. > - Use `SHIELD_ERROR.tabActionFailed` when checkout tab closes without success; open settings with `waitForSubscriptionCreation=true`. > - Minor: import time constants (`SECOND`) and Shield helpers. > - **Shared** (`shared/modules/shield/constants.ts`): > - Introduce `SHIELD_ERROR` with `tabActionFailed` and `subscriptionPollingTimedOut`. > - **UI Hook** (`ui/hooks/subscription/useSubscription.ts`): > - Switch to `SHIELD_ERROR.tabActionFailed` for non-API tab failure handling. > - **Tests** (`app/scripts/services/subscription/subscription-service.test.ts`): > - Add mocks and cases for polling behavior (paused, multi-attempts, timeout) and environment setup; assert results/errors accordingly. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3dfb3b6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0680b94 commit 0f3d948

File tree

4 files changed

+144
-9
lines changed

4 files changed

+144
-9
lines changed

app/scripts/services/subscription/subscription-service.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import {
99
PAYMENT_TYPES,
1010
PRODUCT_TYPES,
1111
RECURRING_INTERVALS,
12+
Subscription,
13+
SUBSCRIPTION_STATUSES,
14+
SubscriptionPaymentMethod,
1215
} from '@metamask/subscription-controller';
1316
import browser from 'webextension-polyfill';
1417
import { TransactionType } from '@metamask/transaction-controller';
@@ -17,6 +20,8 @@ import { ENVIRONMENT } from '../../../../development/build/constants';
1720
import { WebAuthenticator } from '../oauth/types';
1821
import { createSwapsMockStore } from '../../../../test/jest';
1922
import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout';
23+
import { DAY } from '../../../../shared/constants/time';
24+
import { SHIELD_ERROR } from '../../../../shared/modules/shield';
2025
import { SubscriptionService } from './subscription-service';
2126
import { SubscriptionServiceMessenger } from './types';
2227

@@ -50,6 +55,24 @@ const MAINNET_BASE = {
5055

5156
const MOCK_REDIRECT_URI = 'https://mocked-redirect-uri';
5257

58+
const MOCK_ACTIVE_SHIELD_SUBSCRIPTION: Subscription = {
59+
id: 'sub_123',
60+
status: SUBSCRIPTION_STATUSES.active,
61+
products: [
62+
{
63+
name: PRODUCT_TYPES.SHIELD,
64+
currency: 'usd',
65+
unitAmount: 100,
66+
unitDecimals: 2,
67+
},
68+
],
69+
paymentMethod: { type: PAYMENT_TYPES.byCard } as SubscriptionPaymentMethod,
70+
interval: RECURRING_INTERVALS.month,
71+
currentPeriodStart: new Date().toISOString(),
72+
currentPeriodEnd: new Date(Date.now() + 30 * DAY).toISOString(),
73+
isEligibleForSupport: true,
74+
};
75+
5376
const getRedirectUrlSpy = jest.fn().mockReturnValue(MOCK_REDIRECT_URI);
5477
const mockSubmitSponsorshipIntents = jest.fn();
5578
const mockGetTransactions = jest.fn();
@@ -194,11 +217,14 @@ describe('SubscriptionService - startSubscriptionWithCard', () => {
194217

195218
beforeEach(() => {
196219
process.env = { ...originalEnv };
220+
process.env.METAMASK_SHIELD_ENABLED = 'true';
197221
delete process.env.IN_TEST; // unset IN_TEST environment variable
198222

199223
mockStartShieldSubscriptionWithCard.mockResolvedValue({
200224
checkoutSessionUrl: mockCheckoutSessionUrl,
201225
});
226+
// Mock getSubscriptions to return active subscription for polling to complete
227+
mockGetSubscriptions.mockResolvedValue([MOCK_ACTIVE_SHIELD_SUBSCRIPTION]);
202228
mockGetAppStateControllerState.mockReturnValue({
203229
defaultSubscriptionPaymentOptions: {
204230
defaultBillingInterval: RECURRING_INTERVALS.month,
@@ -345,6 +371,64 @@ describe('SubscriptionService - startSubscriptionWithCard', () => {
345371
expect(mockGetRewardSeasonMetadata).toHaveBeenCalledWith('current');
346372
expect(mockGetHasAccountOptedIn).not.toHaveBeenCalled();
347373
});
374+
375+
it('should poll until paused subscription is found', async () => {
376+
mockGetSubscriptions.mockRestore();
377+
const pausedSubscription = {
378+
...MOCK_ACTIVE_SHIELD_SUBSCRIPTION,
379+
status: SUBSCRIPTION_STATUSES.paused,
380+
};
381+
mockGetSubscriptions.mockResolvedValue([pausedSubscription]);
382+
383+
const result = await subscriptionService.startSubscriptionWithCard({
384+
products: [PRODUCT_TYPES.SHIELD],
385+
isTrialRequested: false,
386+
recurringInterval: RECURRING_INTERVALS.month,
387+
});
388+
389+
expect(mockGetSubscriptions).toHaveBeenCalled();
390+
expect(result).toEqual([pausedSubscription]);
391+
});
392+
393+
it('should poll multiple times until active subscription is found', async () => {
394+
mockGetSubscriptions.mockRestore();
395+
// First call returns no subscription, second call returns active subscription
396+
mockGetSubscriptions
397+
.mockResolvedValueOnce([])
398+
.mockResolvedValueOnce([MOCK_ACTIVE_SHIELD_SUBSCRIPTION]);
399+
400+
const result = await subscriptionService.startSubscriptionWithCard(
401+
{
402+
products: [PRODUCT_TYPES.SHIELD],
403+
isTrialRequested: false,
404+
recurringInterval: RECURRING_INTERVALS.month,
405+
},
406+
undefined,
407+
100,
408+
);
409+
410+
expect(mockGetSubscriptions).toHaveBeenCalledTimes(2);
411+
expect(result).toEqual([MOCK_ACTIVE_SHIELD_SUBSCRIPTION]);
412+
});
413+
414+
it('should throw timeout error when subscription is not activated within timeout period', async () => {
415+
mockGetSubscriptions.mockRestore();
416+
// Always return empty subscriptions to simulate no active subscription
417+
mockGetSubscriptions.mockResolvedValue([]);
418+
419+
await expect(() =>
420+
subscriptionService.startSubscriptionWithCard(
421+
{
422+
products: [PRODUCT_TYPES.SHIELD],
423+
isTrialRequested: false,
424+
recurringInterval: RECURRING_INTERVALS.month,
425+
},
426+
undefined,
427+
100,
428+
1000,
429+
),
430+
).rejects.toThrow(SHIELD_ERROR.subscriptionPollingTimedOut);
431+
});
348432
});
349433

350434
describe('SubscriptionService - handlePostTransaction', () => {

app/scripts/services/subscription/subscription-service.ts

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
PAYMENT_TYPES,
55
PRODUCT_TYPES,
66
StartSubscriptionRequest,
7+
Subscription,
78
UpdatePaymentMethodOpts,
89
} from '@metamask/subscription-controller';
910
import {
@@ -36,6 +37,12 @@ import {
3637
MetaMetricsEventCategory,
3738
MetaMetricsEventName,
3839
} from '../../../../shared/constants/metametrics';
40+
import { SECOND } from '../../../../shared/constants/time';
41+
import {
42+
getIsShieldSubscriptionActive,
43+
getIsShieldSubscriptionPaused,
44+
} from '../../../../shared/lib/shield';
45+
import { SHIELD_ERROR } from '../../../../shared/modules/shield';
3946
import {
4047
SubscriptionServiceAction,
4148
SubscriptionServiceEvent,
@@ -44,6 +51,9 @@ import {
4451
ServiceName,
4552
} from './types';
4653

54+
const SUBSCRIPTION_POLL_INTERVAL = 5 * SECOND;
55+
const SUBSCRIPTION_POLL_TIMEOUT = 60 * SECOND;
56+
4757
export class SubscriptionService {
4858
// Required for modular initialisation.
4959
name: ServiceName = SERVICE_NAME;
@@ -137,6 +147,8 @@ export class SubscriptionService {
137147
async startSubscriptionWithCard(
138148
params: StartSubscriptionRequest,
139149
currentTabId?: number,
150+
subscriptionPollInterval?: number,
151+
subscriptionPollTimeout?: number,
140152
) {
141153
try {
142154
const redirectUrl = this.#webAuthenticator.getRedirectURL();
@@ -169,8 +181,10 @@ export class SubscriptionService {
169181
}
170182
}
171183

172-
const subscriptions = await this.#messenger.call(
173-
'SubscriptionController:getSubscriptions',
184+
// Poll subscriptions until active or paused subscription is created (stripe webhook may be delayed)
185+
const subscriptions = await this.#pollForSubscription(
186+
subscriptionPollInterval,
187+
subscriptionPollTimeout,
174188
);
175189
this.trackSubscriptionRequestEvent('completed');
176190

@@ -187,6 +201,11 @@ export class SubscriptionService {
187201
// eslint-disable-next-line @typescript-eslint/naming-convention
188202
error_message: errorMessage,
189203
});
204+
205+
// fetch latest subscriptions to update the state in case subscription already created error (not when polling timed out)
206+
if (errorMessage.toLocaleLowerCase().includes('already exists')) {
207+
await this.#messenger.call('SubscriptionController:getSubscriptions');
208+
}
190209
throw error;
191210
}
192211
}
@@ -365,14 +384,46 @@ export class SubscriptionService {
365384
if (succeeded) {
366385
resolve();
367386
} else {
368-
reject(new Error('Tab action failed'));
387+
reject(new Error(SHIELD_ERROR.tabActionFailed));
369388
}
370389
}
371390
};
372391
this.#platform.addTabRemovedListener(onTabRemovedListener);
373392
});
374393
}
375394

395+
/**
396+
* Poll for active subscription until one is found or timeout.
397+
* This is needed because Stripe webhook may be delayed after card payment.
398+
*
399+
* @param interval
400+
* @param timeout
401+
* @returns Promise<Subscription[]> - The subscriptions when an active one is found.
402+
*/
403+
async #pollForSubscription(
404+
interval: number = SUBSCRIPTION_POLL_INTERVAL,
405+
timeout: number = SUBSCRIPTION_POLL_TIMEOUT,
406+
): Promise<Subscription[]> {
407+
const startTime = Date.now();
408+
409+
while (Date.now() - startTime < timeout) {
410+
const subscriptions = await this.#messenger.call(
411+
'SubscriptionController:getSubscriptions',
412+
);
413+
414+
if (
415+
getIsShieldSubscriptionActive(subscriptions) ||
416+
getIsShieldSubscriptionPaused(subscriptions)
417+
) {
418+
return subscriptions;
419+
}
420+
421+
await new Promise((resolve) => setTimeout(resolve, interval));
422+
}
423+
424+
throw new Error(SHIELD_ERROR.subscriptionPollingTimedOut);
425+
}
426+
376427
async #handleShieldSubscriptionApproveTransaction(txMeta: TransactionMeta) {
377428
const { isGasFeeSponsored, chainId } = txMeta;
378429
const bundlerSupported = await isSendBundleSupported(chainId);

shared/modules/shield/constants.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1-
export const SHIELD_SUBSCRIPTION_CARD_TAB_ACTION_ERROR_MESSAGE =
2-
'tab action failed';
1+
export const SHIELD_ERROR = {
2+
tabActionFailed: 'tab action failed',
3+
subscriptionPollingTimedOut: 'subscription polling timed out',
4+
};

ui/hooks/subscription/useSubscription.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ import {
6666
getLatestSubscriptionStatus,
6767
getShieldMarketingUtmParamsForMetrics,
6868
getUserBalanceCategory,
69-
SHIELD_SUBSCRIPTION_CARD_TAB_ACTION_ERROR_MESSAGE,
69+
SHIELD_ERROR,
7070
} from '../../../shared/modules/shield';
7171
import { openWindow } from '../../helpers/utils/window';
7272
import { SUPPORT_LINK } from '../../../shared/lib/ui-utils';
@@ -596,9 +596,7 @@ export const useHandleSubscription = ({
596596
e instanceof Error &&
597597
e.message
598598
.toLowerCase()
599-
.includes(
600-
SHIELD_SUBSCRIPTION_CARD_TAB_ACTION_ERROR_MESSAGE.toLowerCase(),
601-
)
599+
.includes(SHIELD_ERROR.tabActionFailed.toLowerCase())
602600
) {
603601
// tab action failed is not api error, only log it here
604602
console.error('[useHandleSubscription error]:', e);

0 commit comments

Comments
 (0)