From 55f4d942197b10a4b1df1c47374bb659cd283a78 Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 4 Feb 2026 20:02:56 +0800 Subject: [PATCH 01/12] chore: improve error handling in SubscriptionService --- .../src/SubscriptionController.ts | 15 ++- .../src/SubscriptionService.ts | 93 ++++++++++++++++--- .../subscription-controller/src/errors.ts | 22 ++++- 3 files changed, 113 insertions(+), 17 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index 26a9393dc4f..233604c8e56 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -410,6 +410,7 @@ export class SubscriptionController extends StaticIntervalPollingController()< currentSubscriptions, newSubscriptions, ); + console.log('areSubscriptionsEqual', areSubscriptionsEqual); // check if the new trialed products are different from the current trialed products const areTrialedProductsEqual = this.#areTrialedProductsEqual( currentTrialedProducts, @@ -440,8 +441,11 @@ export class SubscriptionController extends StaticIntervalPollingController()< state.lastSubscription = newLastSubscription; state.rewardAccountId = newRewardAccountId; }); + console.log('update state'); + console.log('trigger access token refresh'); // trigger access token refresh to ensure the user has the latest access token if subscription state change this.triggerAccessTokenRefresh(); + console.log('trigger access token refresh done'); } return newSubscriptions; @@ -960,7 +964,16 @@ export class SubscriptionController extends StaticIntervalPollingController()< // We perform a sign out to clear the access token from the authentication // controller. Next time the access token is requested, a new access token // will be fetched. - this.messenger.call('AuthenticationController:performSignOut'); + try { + this.messenger.call('AuthenticationController:performSignOut'); + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : JSON.stringify(error); + + throw new Error( + `Failed to trigger access token refresh. ${errorMessage}`, + ); + } } #getProductPriceByProductAndPlan( diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index 59b7806d151..b6338a1b56e 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -1,4 +1,4 @@ -import { handleFetch } from '@metamask/controller-utils'; +import { fetchWithErrorHandling, HttpError } from '@metamask/controller-utils'; import { getEnvUrls, SubscriptionControllerErrorMessage } from './constants'; import type { Env } from './constants'; @@ -203,40 +203,103 @@ export class SubscriptionService implements ISubscriptionService { body?: Record, queryParams?: Record, ): Promise { - try { - const headers = await this.#getAuthorizationHeader(); - const url = new URL(SUBSCRIPTION_URL(this.#env, path)); + const headers = await this.#getAuthorizationHeader(); + const url = new URL(SUBSCRIPTION_URL(this.#env, path)); + try { if (queryParams) { Object.entries(queryParams).forEach(([key, value]) => { url.searchParams.append(key, value); }); } - const response = await handleFetch(url.toString(), { - method, - headers: { - 'Content-Type': 'application/json', - ...headers, + const response = await fetchWithErrorHandling({ + url: url.toString(), + options: { + method, + headers: { + 'Content-Type': 'application/json', + ...headers, + }, + body: body ? JSON.stringify(body) : undefined, }, - body: body ? JSON.stringify(body) : undefined, }); return response; } catch (error) { - const errorMessage = - error instanceof Error ? error.message : JSON.stringify(error); + const errorContext = this.#buildErrorContext(error); + const errorMessage = errorContext.message; throw new SubscriptionServiceError( - `failed to make request. ${errorMessage}`, + `Failed to make request. ${errorMessage}`, + { + cause: error instanceof Error ? error : undefined, + details: { + request: { + method, + url: url.toString(), + }, + error: errorContext.details, + }, + }, ); } } + /** + * Builds the error context for the SubscriptionService error. + * + * @param error - The error to build the context for. + * @returns The SubscriptionService error context. + */ + #buildErrorContext(error: unknown): { + message: string; + details: Record; + } { + if (error instanceof HttpError) { + return { + message: `${error.message} (status=${error.httpStatus})`, + details: { + name: 'HttpError', + message: error.message, + stack: error.stack, + httpStatus: error.httpStatus, + }, + }; + } + + if (error instanceof Error) { + return { + message: error.message, + details: { + name: error.name, + message: error.message, + stack: error.stack, + }, + }; + } + + return { + message: `Non-Error thrown: ${JSON.stringify(error)}`, + details: { + nonErrorThrown: JSON.stringify(error), + }, + }; + } + // eslint-disable-next-line @typescript-eslint/naming-convention async #getAuthorizationHeader(): Promise<{ Authorization: string }> { - const accessToken = await this.authUtils.getAccessToken(); - return { Authorization: `Bearer ${accessToken}` }; + try { + const accessToken = await this.authUtils.getAccessToken(); + return { Authorization: `Bearer ${accessToken}` }; + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : JSON.stringify(error); + + throw new SubscriptionServiceError( + `Failed to get authorization header. ${errorMessage}`, + ); + } } async getPricing(): Promise { diff --git a/packages/subscription-controller/src/errors.ts b/packages/subscription-controller/src/errors.ts index 0e4efbcbbb9..50c9c540a04 100644 --- a/packages/subscription-controller/src/errors.ts +++ b/packages/subscription-controller/src/errors.ts @@ -1,6 +1,26 @@ +export type SubscriptionServiceErrorDetails = Record; + export class SubscriptionServiceError extends Error { - constructor(message: string) { + /** + * The underlying error that caused this error. + */ + cause?: Error; + + /** + * Additional details about the error. + */ + details?: SubscriptionServiceErrorDetails; + + constructor( + message: string, + options?: { + cause?: Error; + details?: SubscriptionServiceErrorDetails; + }, + ) { super(message); this.name = 'SubscriptionServiceError'; + this.details = options?.details; + this.cause = options?.cause; } } From d4095cfaf93c10945c15376e9a9859711283cc9e Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 4 Feb 2026 20:03:08 +0800 Subject: [PATCH 02/12] test: updated test --- .../src/SubscriptionController.test.ts | 33 +++ .../src/SubscriptionService.test.ts | 204 ++++++++++++------ 2 files changed, 169 insertions(+), 68 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionController.test.ts b/packages/subscription-controller/src/SubscriptionController.test.ts index dfa036df6f7..37b33f23fce 100644 --- a/packages/subscription-controller/src/SubscriptionController.test.ts +++ b/packages/subscription-controller/src/SubscriptionController.test.ts @@ -436,6 +436,39 @@ describe('SubscriptionController', () => { }); }); + it('should surface triggerAccessTokenRefresh errors', async () => { + await withController( + async ({ controller, mockService, mockPerformSignOut }) => { + mockService.getSubscriptions.mockResolvedValue( + MOCK_GET_SUBSCRIPTIONS_RESPONSE, + ); + mockPerformSignOut.mockImplementation(() => { + throw new Error('Wallet is locked'); + }); + + await expect(controller.getSubscriptions()).rejects.toThrow( + 'Failed to trigger access token refresh. Wallet is locked', + ); + }, + ); + + await withController( + async ({ controller, mockService, mockPerformSignOut }) => { + mockService.getSubscriptions.mockResolvedValue( + MOCK_GET_SUBSCRIPTIONS_RESPONSE, + ); + mockPerformSignOut.mockImplementation(() => { + // eslint-disable-next-line @typescript-eslint/only-throw-error + throw 'string error'; + }); + + await expect(controller.getSubscriptions()).rejects.toThrow( + `Failed to trigger access token refresh. ${JSON.stringify('string error')}`, + ); + }, + ); + }); + it('should update state when subscription is fetched', async () => { const initialSubscription = { ...MOCK_SUBSCRIPTION, id: 'sub_old' }; const newSubscription = { ...MOCK_SUBSCRIPTION, id: 'sub_new' }; diff --git a/packages/subscription-controller/src/SubscriptionService.test.ts b/packages/subscription-controller/src/SubscriptionService.test.ts index 849cd02283a..181645b60c5 100644 --- a/packages/subscription-controller/src/SubscriptionService.test.ts +++ b/packages/subscription-controller/src/SubscriptionService.test.ts @@ -1,4 +1,4 @@ -import { handleFetch } from '@metamask/controller-utils'; +import { fetchWithErrorHandling, HttpError } from '@metamask/controller-utils'; import { Env, @@ -31,10 +31,11 @@ import { // Mock the handleFetch function jest.mock('@metamask/controller-utils', () => ({ - handleFetch: jest.fn(), + ...jest.requireActual('@metamask/controller-utils'), + fetchWithErrorHandling: jest.fn(), })); -const handleFetchMock = handleFetch as jest.Mock; +const fetchWithErrorHandlingMock = fetchWithErrorHandling as jest.Mock; // Mock data const MOCK_SUBSCRIPTION: Subscription = { @@ -189,7 +190,7 @@ describe('SubscriptionService', () => { describe('getSubscriptions', () => { it('should fetch subscriptions successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({ + fetchWithErrorHandlingMock.mockResolvedValue({ customerId: 'cus_1', subscriptions: [MOCK_SUBSCRIPTION], trialedProducts: [], @@ -206,9 +207,31 @@ describe('SubscriptionService', () => { }); }); - it('should throw SubscriptionServiceError for error responses', async () => { + it('should throw SubscriptionServiceError for network errors', async () => { + await withMockSubscriptionService(async ({ service }) => { + fetchWithErrorHandlingMock.mockRejectedValue( + new Error('Network error'), + ); + + const error = await service.getSubscriptions().then( + () => { + throw new Error('Expected getSubscriptions to throw'); + }, + (rejection) => rejection, + ); + + expect(error).toBeInstanceOf(SubscriptionServiceError); + const serviceError = error as SubscriptionServiceError; + expect(serviceError.details).toMatchObject({ + request: { + method: 'GET', + url: `${getTestUrl(Env.DEV)}/v1/subscriptions`, + }, + }); + }); + await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + fetchWithErrorHandlingMock.mockRejectedValue('string error'); await expect(service.getSubscriptions()).rejects.toThrow( SubscriptionServiceError, @@ -216,13 +239,33 @@ describe('SubscriptionService', () => { }); }); - it('should throw SubscriptionServiceError for network errors', async () => { + it('should include httpStatus details for HttpError responses', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + fetchWithErrorHandlingMock.mockRejectedValue( + new HttpError(503, 'Service unavailable'), + ); - await expect(service.getSubscriptions()).rejects.toThrow( - SubscriptionServiceError, + const error = await service.getSubscriptions().then( + () => { + throw new Error('Expected getSubscriptions to throw'); + }, + (rejection) => rejection, ); + + expect(error).toBeInstanceOf(SubscriptionServiceError); + const serviceError = error as SubscriptionServiceError; + expect(serviceError.message).toContain('status=503'); + expect(serviceError.details).toMatchObject({ + request: { + method: 'GET', + url: `${getTestUrl(Env.DEV)}/v1/subscriptions`, + }, + error: { + httpStatus: 503, + message: 'Service unavailable', + name: 'HttpError', + }, + }); }); }); @@ -237,13 +280,24 @@ describe('SubscriptionService', () => { SubscriptionServiceError, ); }); + + await withMockSubscriptionService(async ({ service, config }) => { + // Simulate a non-Error thrown from the auth.getAccessToken mock + (config.auth.getAccessToken as jest.Mock).mockRejectedValue( + new Error('Wallet is locked'), + ); + + await expect(service.getSubscriptions()).rejects.toThrow( + SubscriptionServiceError, + ); + }); }); }); describe('cancelSubscription', () => { it('should cancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.cancelSubscription({ subscriptionId: 'sub_123456789' }); @@ -253,7 +307,9 @@ describe('SubscriptionService', () => { it('should throw SubscriptionServiceError for network errors', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + fetchWithErrorHandlingMock.mockRejectedValue( + new Error('Network error'), + ); await expect( service.cancelSubscription({ subscriptionId: 'sub_123456789' }), @@ -265,7 +321,7 @@ describe('SubscriptionService', () => { describe('uncancelSubscription', () => { it('should cancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.unCancelSubscription({ subscriptionId: 'sub_123456789' }); @@ -275,7 +331,9 @@ describe('SubscriptionService', () => { it('should throw SubscriptionServiceError for network errors', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + fetchWithErrorHandlingMock.mockRejectedValue( + new Error('Network error'), + ); await expect( service.unCancelSubscription({ subscriptionId: 'sub_123456789' }), @@ -287,7 +345,9 @@ describe('SubscriptionService', () => { describe('startSubscription', () => { it('should start subscription successfully', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE); + fetchWithErrorHandlingMock.mockResolvedValue( + MOCK_START_SUBSCRIPTION_RESPONSE, + ); const result = await service.startSubscriptionWithCard( MOCK_START_SUBSCRIPTION_REQUEST, @@ -306,7 +366,9 @@ describe('SubscriptionService', () => { recurringInterval: RECURRING_INTERVALS.month, }; - handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE); + fetchWithErrorHandlingMock.mockResolvedValue( + MOCK_START_SUBSCRIPTION_RESPONSE, + ); const result = await service.startSubscriptionWithCard(request); @@ -347,7 +409,7 @@ describe('SubscriptionService', () => { status: SUBSCRIPTION_STATUSES.active, }; - handleFetchMock.mockResolvedValue(response); + fetchWithErrorHandlingMock.mockResolvedValue(response); const result = await service.startSubscriptionWithCrypto(request); @@ -366,7 +428,7 @@ describe('SubscriptionService', () => { const config = createMockConfig(); const service = new SubscriptionService(config); - handleFetchMock.mockResolvedValue(mockPricingResponse); + fetchWithErrorHandlingMock.mockResolvedValue(mockPricingResponse); const result = await service.getPricing(); @@ -382,16 +444,16 @@ describe('SubscriptionService', () => { recurringInterval: RECURRING_INTERVALS.month, }; - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.updatePaymentMethodCard(request); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL( + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL( config.env, 'subscriptions/sub_123456789/payment-method/card', ), - { + options: { method: 'PATCH', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -399,7 +461,7 @@ describe('SubscriptionService', () => { subscriptionId: undefined, }), }, - ); + }); }); }); @@ -415,16 +477,16 @@ describe('SubscriptionService', () => { billingCycles: 3, }; - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.updatePaymentMethodCrypto(request); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL( + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL( config.env, 'subscriptions/sub_123456789/payment-method/crypto', ), - { + options: { method: 'PATCH', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -432,7 +494,7 @@ describe('SubscriptionService', () => { subscriptionId: undefined, }), }, - ); + }); }); }); }); @@ -440,7 +502,7 @@ describe('SubscriptionService', () => { describe('getBillingPortalUrl', () => { it('should get billing portal url successfully', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockResolvedValue({ + fetchWithErrorHandlingMock.mockResolvedValue({ url: 'https://billing-portal.com', }); @@ -455,7 +517,7 @@ describe('SubscriptionService', () => { it('should get shield subscription eligibility successfully', async () => { await withMockSubscriptionService(async ({ service }) => { const mockResponse = createMockEligibilityResponse(); - handleFetchMock.mockResolvedValue([mockResponse]); + fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); const results = await service.getSubscriptionsEligibilities(); @@ -471,7 +533,7 @@ describe('SubscriptionService', () => { assignedAt: '2024-01-01T00:00:00Z', }); - handleFetchMock.mockResolvedValue([mockResponse]); + fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); const results = await service.getSubscriptionsEligibilities({ balanceCategory: '1k-9.9k', @@ -483,7 +545,7 @@ describe('SubscriptionService', () => { it('should get shield subscription eligibility with default values', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockResolvedValue([ + fetchWithErrorHandlingMock.mockResolvedValue([ { product: PRODUCT_TYPES.SHIELD, }, @@ -504,17 +566,19 @@ describe('SubscriptionService', () => { it('should pass balanceCategory as query parameter when provided', async () => { await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - handleFetchMock.mockResolvedValue([mockResponse]); + fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); await service.getSubscriptionsEligibilities({ balanceCategory: '100-999', }); - expect(handleFetchMock).toHaveBeenCalledWith( - expect.stringContaining('balanceCategory=100-999'), + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith( expect.objectContaining({ - method: 'GET', - headers: MOCK_HEADERS, + url: expect.stringContaining('balanceCategory=100-999'), + options: expect.objectContaining({ + method: 'GET', + headers: MOCK_HEADERS, + }), }), ); expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); @@ -524,15 +588,17 @@ describe('SubscriptionService', () => { it('should not pass balanceCategory query parameter when not provided', async () => { await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - handleFetchMock.mockResolvedValue([mockResponse]); + fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); await service.getSubscriptionsEligibilities(); - expect(handleFetchMock).toHaveBeenCalledWith( - expect.not.stringContaining('balanceCategory'), + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith( expect.objectContaining({ - method: 'GET', - headers: MOCK_HEADERS, + url: expect.not.stringMatching(/balanceCategory/u), + options: expect.objectContaining({ + method: 'GET', + headers: MOCK_HEADERS, + }), }), ); expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); @@ -543,37 +609,37 @@ describe('SubscriptionService', () => { describe('submitUserEvent', () => { it('should submit user event successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.submitUserEvent({ event: SubscriptionUserEvent.ShieldEntryModalViewed, }); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL(config.env, 'user-events'), - { + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL(config.env, 'user-events'), + options: { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ event: SubscriptionUserEvent.ShieldEntryModalViewed, }), }, - ); + }); }); }); it('should submit user event with cohort successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.submitUserEvent({ event: SubscriptionUserEvent.ShieldEntryModalViewed, cohort: 'post_tx', }); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL(config.env, 'user-events'), - { + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL(config.env, 'user-events'), + options: { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -581,7 +647,7 @@ describe('SubscriptionService', () => { cohort: 'post_tx', }), }, - ); + }); }); }); }); @@ -589,27 +655,29 @@ describe('SubscriptionService', () => { describe('assignUserToCohort', () => { it('should assign user to cohort successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.assignUserToCohort({ cohort: 'post_tx' }); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL(config.env, 'cohorts/assign'), - { + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL(config.env, 'cohorts/assign'), + options: { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ cohort: 'post_tx', }), }, - ); + }); expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); }); }); it('should handle cohort assignment errors', async () => { await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + fetchWithErrorHandlingMock.mockRejectedValue( + new Error('Network error'), + ); await expect( service.assignUserToCohort({ cohort: 'wallet_home' }), @@ -621,7 +689,7 @@ describe('SubscriptionService', () => { describe('submitSponsorshipIntents', () => { it('should submit sponsorship intents successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.submitSponsorshipIntents({ chainId: '0x1', @@ -632,9 +700,9 @@ describe('SubscriptionService', () => { paymentTokenSymbol: 'USDT', }); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL(config.env, 'transaction-sponsorship/intents'), - { + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL(config.env, 'transaction-sponsorship/intents'), + options: { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -646,7 +714,7 @@ describe('SubscriptionService', () => { paymentTokenSymbol: 'USDT', }), }, - ); + }); }); }); }); @@ -654,16 +722,16 @@ describe('SubscriptionService', () => { describe('linkRewards', () => { it('should link rewards successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + fetchWithErrorHandlingMock.mockResolvedValue({}); await service.linkRewards({ rewardAccountId: 'eip155:1:0x1234567890123456789012345678901234567890', }); - expect(handleFetchMock).toHaveBeenCalledWith( - SUBSCRIPTION_URL(config.env, 'rewards/link'), - { + expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ + url: SUBSCRIPTION_URL(config.env, 'rewards/link'), + options: { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -671,7 +739,7 @@ describe('SubscriptionService', () => { 'eip155:1:0x1234567890123456789012345678901234567890', }), }, - ); + }); }); }); }); From d425321ae676634cf1fad055445a0431ae7046e1 Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 4 Feb 2026 20:16:42 +0800 Subject: [PATCH 03/12] chore: updated ChangeLog --- packages/subscription-controller/CHANGELOG.md | 1 + .../subscription-controller/src/SubscriptionController.ts | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/subscription-controller/CHANGELOG.md b/packages/subscription-controller/CHANGELOG.md index fcb637ca518..f548a7d4506 100644 --- a/packages/subscription-controller/CHANGELOG.md +++ b/packages/subscription-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Updated SubscriptionServiceError to include more information for Sentry reporting. ([#7835](https://github.com/MetaMask/core/pull/7835)) - Bump `@metamask/transaction-controller` from `^62.12.0` to `^62.13.0` ([#7802](https://github.com/MetaMask/core/pull/7802)) ## [5.4.2] diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index 233604c8e56..ff1048c4636 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -410,7 +410,6 @@ export class SubscriptionController extends StaticIntervalPollingController()< currentSubscriptions, newSubscriptions, ); - console.log('areSubscriptionsEqual', areSubscriptionsEqual); // check if the new trialed products are different from the current trialed products const areTrialedProductsEqual = this.#areTrialedProductsEqual( currentTrialedProducts, @@ -441,11 +440,8 @@ export class SubscriptionController extends StaticIntervalPollingController()< state.lastSubscription = newLastSubscription; state.rewardAccountId = newRewardAccountId; }); - console.log('update state'); - console.log('trigger access token refresh'); // trigger access token refresh to ensure the user has the latest access token if subscription state change this.triggerAccessTokenRefresh(); - console.log('trigger access token refresh done'); } return newSubscriptions; From 8f3469e9c7db5a605c86a768b721dc90c36aaaab Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 4 Feb 2026 21:49:04 +0800 Subject: [PATCH 04/12] feat: improve error handling for #getAuthorizationHeader method --- packages/subscription-controller/CHANGELOG.md | 2 +- .../src/SubscriptionService.test.ts | 146 ++++++++---------- .../src/SubscriptionService.ts | 73 ++++----- 3 files changed, 104 insertions(+), 117 deletions(-) diff --git a/packages/subscription-controller/CHANGELOG.md b/packages/subscription-controller/CHANGELOG.md index 89dd4aac30d..a9b3b4fbb3b 100644 --- a/packages/subscription-controller/CHANGELOG.md +++ b/packages/subscription-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Updated SubscriptionServiceError to include more information for Sentry reporting. ([#7835](https://github.com/MetaMask/core/pull/7835)) +- Updated `SubscriptionServiceError` to include more information for Sentry reporting. ([#7835](https://github.com/MetaMask/core/pull/7835)) - Bump `@metamask/transaction-controller` from `^62.12.0` to `^62.14.0` ([#7802](https://github.com/MetaMask/core/pull/7802), [#7832](https://github.com/MetaMask/core/pull/7832)) ## [5.4.2] diff --git a/packages/subscription-controller/src/SubscriptionService.test.ts b/packages/subscription-controller/src/SubscriptionService.test.ts index 181645b60c5..d2e49f0521f 100644 --- a/packages/subscription-controller/src/SubscriptionService.test.ts +++ b/packages/subscription-controller/src/SubscriptionService.test.ts @@ -1,4 +1,4 @@ -import { fetchWithErrorHandling, HttpError } from '@metamask/controller-utils'; +import { handleFetch, HttpError } from '@metamask/controller-utils'; import { Env, @@ -32,10 +32,10 @@ import { // Mock the handleFetch function jest.mock('@metamask/controller-utils', () => ({ ...jest.requireActual('@metamask/controller-utils'), - fetchWithErrorHandling: jest.fn(), + handleFetch: jest.fn(), })); -const fetchWithErrorHandlingMock = fetchWithErrorHandling as jest.Mock; +const handleFetchMock = handleFetch as jest.Mock; // Mock data const MOCK_SUBSCRIPTION: Subscription = { @@ -190,7 +190,7 @@ describe('SubscriptionService', () => { describe('getSubscriptions', () => { it('should fetch subscriptions successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({ + handleFetchMock.mockResolvedValue({ customerId: 'cus_1', subscriptions: [MOCK_SUBSCRIPTION], trialedProducts: [], @@ -209,9 +209,7 @@ describe('SubscriptionService', () => { it('should throw SubscriptionServiceError for network errors', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockRejectedValue( - new Error('Network error'), - ); + handleFetchMock.mockRejectedValue(new Error('Network error')); const error = await service.getSubscriptions().then( () => { @@ -231,7 +229,7 @@ describe('SubscriptionService', () => { }); await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockRejectedValue('string error'); + handleFetchMock.mockRejectedValue('string error'); await expect(service.getSubscriptions()).rejects.toThrow( SubscriptionServiceError, @@ -241,7 +239,7 @@ describe('SubscriptionService', () => { it('should include httpStatus details for HttpError responses', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockRejectedValue( + handleFetchMock.mockRejectedValue( new HttpError(503, 'Service unavailable'), ); @@ -297,7 +295,7 @@ describe('SubscriptionService', () => { describe('cancelSubscription', () => { it('should cancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.cancelSubscription({ subscriptionId: 'sub_123456789' }); @@ -307,9 +305,7 @@ describe('SubscriptionService', () => { it('should throw SubscriptionServiceError for network errors', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockRejectedValue( - new Error('Network error'), - ); + handleFetchMock.mockRejectedValue(new Error('Network error')); await expect( service.cancelSubscription({ subscriptionId: 'sub_123456789' }), @@ -321,7 +317,7 @@ describe('SubscriptionService', () => { describe('uncancelSubscription', () => { it('should cancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.unCancelSubscription({ subscriptionId: 'sub_123456789' }); @@ -331,9 +327,7 @@ describe('SubscriptionService', () => { it('should throw SubscriptionServiceError for network errors', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockRejectedValue( - new Error('Network error'), - ); + handleFetchMock.mockRejectedValue(new Error('Network error')); await expect( service.unCancelSubscription({ subscriptionId: 'sub_123456789' }), @@ -345,9 +339,7 @@ describe('SubscriptionService', () => { describe('startSubscription', () => { it('should start subscription successfully', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockResolvedValue( - MOCK_START_SUBSCRIPTION_RESPONSE, - ); + handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE); const result = await service.startSubscriptionWithCard( MOCK_START_SUBSCRIPTION_REQUEST, @@ -366,9 +358,7 @@ describe('SubscriptionService', () => { recurringInterval: RECURRING_INTERVALS.month, }; - fetchWithErrorHandlingMock.mockResolvedValue( - MOCK_START_SUBSCRIPTION_RESPONSE, - ); + handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE); const result = await service.startSubscriptionWithCard(request); @@ -409,7 +399,7 @@ describe('SubscriptionService', () => { status: SUBSCRIPTION_STATUSES.active, }; - fetchWithErrorHandlingMock.mockResolvedValue(response); + handleFetchMock.mockResolvedValue(response); const result = await service.startSubscriptionWithCrypto(request); @@ -428,7 +418,7 @@ describe('SubscriptionService', () => { const config = createMockConfig(); const service = new SubscriptionService(config); - fetchWithErrorHandlingMock.mockResolvedValue(mockPricingResponse); + handleFetchMock.mockResolvedValue(mockPricingResponse); const result = await service.getPricing(); @@ -444,16 +434,16 @@ describe('SubscriptionService', () => { recurringInterval: RECURRING_INTERVALS.month, }; - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.updatePaymentMethodCard(request); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL( + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL( config.env, 'subscriptions/sub_123456789/payment-method/card', ), - options: { + { method: 'PATCH', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -461,7 +451,7 @@ describe('SubscriptionService', () => { subscriptionId: undefined, }), }, - }); + ); }); }); @@ -477,16 +467,16 @@ describe('SubscriptionService', () => { billingCycles: 3, }; - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.updatePaymentMethodCrypto(request); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL( + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL( config.env, 'subscriptions/sub_123456789/payment-method/crypto', ), - options: { + { method: 'PATCH', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -494,7 +484,7 @@ describe('SubscriptionService', () => { subscriptionId: undefined, }), }, - }); + ); }); }); }); @@ -502,7 +492,7 @@ describe('SubscriptionService', () => { describe('getBillingPortalUrl', () => { it('should get billing portal url successfully', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockResolvedValue({ + handleFetchMock.mockResolvedValue({ url: 'https://billing-portal.com', }); @@ -517,7 +507,7 @@ describe('SubscriptionService', () => { it('should get shield subscription eligibility successfully', async () => { await withMockSubscriptionService(async ({ service }) => { const mockResponse = createMockEligibilityResponse(); - fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); + handleFetchMock.mockResolvedValue([mockResponse]); const results = await service.getSubscriptionsEligibilities(); @@ -533,7 +523,7 @@ describe('SubscriptionService', () => { assignedAt: '2024-01-01T00:00:00Z', }); - fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); + handleFetchMock.mockResolvedValue([mockResponse]); const results = await service.getSubscriptionsEligibilities({ balanceCategory: '1k-9.9k', @@ -545,7 +535,7 @@ describe('SubscriptionService', () => { it('should get shield subscription eligibility with default values', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockResolvedValue([ + handleFetchMock.mockResolvedValue([ { product: PRODUCT_TYPES.SHIELD, }, @@ -566,19 +556,17 @@ describe('SubscriptionService', () => { it('should pass balanceCategory as query parameter when provided', async () => { await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); + handleFetchMock.mockResolvedValue([mockResponse]); await service.getSubscriptionsEligibilities({ balanceCategory: '100-999', }); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith( + expect(handleFetchMock).toHaveBeenCalledWith( + expect.stringContaining('balanceCategory=100-999'), expect.objectContaining({ - url: expect.stringContaining('balanceCategory=100-999'), - options: expect.objectContaining({ - method: 'GET', - headers: MOCK_HEADERS, - }), + method: 'GET', + headers: MOCK_HEADERS, }), ); expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); @@ -588,17 +576,15 @@ describe('SubscriptionService', () => { it('should not pass balanceCategory query parameter when not provided', async () => { await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - fetchWithErrorHandlingMock.mockResolvedValue([mockResponse]); + handleFetchMock.mockResolvedValue([mockResponse]); await service.getSubscriptionsEligibilities(); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith( + expect(handleFetchMock).toHaveBeenCalledWith( + expect.not.stringMatching(/balanceCategory/u), expect.objectContaining({ - url: expect.not.stringMatching(/balanceCategory/u), - options: expect.objectContaining({ - method: 'GET', - headers: MOCK_HEADERS, - }), + method: 'GET', + headers: MOCK_HEADERS, }), ); expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); @@ -609,37 +595,37 @@ describe('SubscriptionService', () => { describe('submitUserEvent', () => { it('should submit user event successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.submitUserEvent({ event: SubscriptionUserEvent.ShieldEntryModalViewed, }); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL(config.env, 'user-events'), - options: { + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL(config.env, 'user-events'), + { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ event: SubscriptionUserEvent.ShieldEntryModalViewed, }), }, - }); + ); }); }); it('should submit user event with cohort successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.submitUserEvent({ event: SubscriptionUserEvent.ShieldEntryModalViewed, cohort: 'post_tx', }); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL(config.env, 'user-events'), - options: { + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL(config.env, 'user-events'), + { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -647,7 +633,7 @@ describe('SubscriptionService', () => { cohort: 'post_tx', }), }, - }); + ); }); }); }); @@ -655,29 +641,27 @@ describe('SubscriptionService', () => { describe('assignUserToCohort', () => { it('should assign user to cohort successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.assignUserToCohort({ cohort: 'post_tx' }); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL(config.env, 'cohorts/assign'), - options: { + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL(config.env, 'cohorts/assign'), + { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ cohort: 'post_tx', }), }, - }); + ); expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); }); }); it('should handle cohort assignment errors', async () => { await withMockSubscriptionService(async ({ service }) => { - fetchWithErrorHandlingMock.mockRejectedValue( - new Error('Network error'), - ); + handleFetchMock.mockRejectedValue(new Error('Network error')); await expect( service.assignUserToCohort({ cohort: 'wallet_home' }), @@ -689,7 +673,7 @@ describe('SubscriptionService', () => { describe('submitSponsorshipIntents', () => { it('should submit sponsorship intents successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.submitSponsorshipIntents({ chainId: '0x1', @@ -700,9 +684,9 @@ describe('SubscriptionService', () => { paymentTokenSymbol: 'USDT', }); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL(config.env, 'transaction-sponsorship/intents'), - options: { + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL(config.env, 'transaction-sponsorship/intents'), + { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -714,7 +698,7 @@ describe('SubscriptionService', () => { paymentTokenSymbol: 'USDT', }), }, - }); + ); }); }); }); @@ -722,16 +706,16 @@ describe('SubscriptionService', () => { describe('linkRewards', () => { it('should link rewards successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - fetchWithErrorHandlingMock.mockResolvedValue({}); + handleFetchMock.mockResolvedValue({}); await service.linkRewards({ rewardAccountId: 'eip155:1:0x1234567890123456789012345678901234567890', }); - expect(fetchWithErrorHandlingMock).toHaveBeenCalledWith({ - url: SUBSCRIPTION_URL(config.env, 'rewards/link'), - options: { + expect(handleFetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL(config.env, 'rewards/link'), + { method: 'POST', headers: MOCK_HEADERS, body: JSON.stringify({ @@ -739,7 +723,7 @@ describe('SubscriptionService', () => { 'eip155:1:0x1234567890123456789012345678901234567890', }), }, - }); + ); }); }); }); diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index b6338a1b56e..6f97f8ebea3 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -1,4 +1,4 @@ -import { fetchWithErrorHandling, HttpError } from '@metamask/controller-utils'; +import { handleFetch, HttpError } from '@metamask/controller-utils'; import { getEnvUrls, SubscriptionControllerErrorMessage } from './constants'; import type { Env } from './constants'; @@ -213,16 +213,13 @@ export class SubscriptionService implements ISubscriptionService { }); } - const response = await fetchWithErrorHandling({ - url: url.toString(), - options: { - method, - headers: { - 'Content-Type': 'application/json', - ...headers, - }, - body: body ? JSON.stringify(body) : undefined, + const response = await handleFetch(url.toString(), { + method, + headers: { + 'Content-Type': 'application/json', + ...headers, }, + body: body ? JSON.stringify(body) : undefined, }); return response; @@ -246,6 +243,37 @@ export class SubscriptionService implements ISubscriptionService { } } + async getPricing(): Promise { + const path = 'pricing'; + return await this.#makeRequest(path); + } + + async getBillingPortalUrl(): Promise { + const path = 'billing-portal'; + return await this.#makeRequest(path); + } + + // eslint-disable-next-line @typescript-eslint/naming-convention + async #getAuthorizationHeader(): Promise<{ Authorization: string }> { + try { + const accessToken = await this.authUtils.getAccessToken(); + return { Authorization: `Bearer ${accessToken}` }; + } catch (error) { + const errorContext = this.#buildErrorContext(error); + const errorMessage = errorContext.message; + + throw new SubscriptionServiceError( + `Failed to get authorization header. ${errorMessage}`, + { + cause: error instanceof Error ? error : undefined, + details: { + error: errorContext.details, + }, + }, + ); + } + } + /** * Builds the error context for the SubscriptionService error. * @@ -286,29 +314,4 @@ export class SubscriptionService implements ISubscriptionService { }, }; } - - // eslint-disable-next-line @typescript-eslint/naming-convention - async #getAuthorizationHeader(): Promise<{ Authorization: string }> { - try { - const accessToken = await this.authUtils.getAccessToken(); - return { Authorization: `Bearer ${accessToken}` }; - } catch (error) { - const errorMessage = - error instanceof Error ? error.message : JSON.stringify(error); - - throw new SubscriptionServiceError( - `Failed to get authorization header. ${errorMessage}`, - ); - } - } - - async getPricing(): Promise { - const path = 'pricing'; - return await this.#makeRequest(path); - } - - async getBillingPortalUrl(): Promise { - const path = 'billing-portal'; - return await this.#makeRequest(path); - } } From 8ddac2eae842fde50270d34e315c56acbcd45032 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 5 Feb 2026 00:32:18 +0800 Subject: [PATCH 05/12] chore: minor refactoring --- .../src/SubscriptionService.ts | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index 6f97f8ebea3..186afedf6d3 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -224,22 +224,18 @@ export class SubscriptionService implements ISubscriptionService { return response; } catch (error) { - const errorContext = this.#buildErrorContext(error); - const errorMessage = errorContext.message; + const { message, details } = this.#buildErrorContext(error); - throw new SubscriptionServiceError( - `Failed to make request. ${errorMessage}`, - { - cause: error instanceof Error ? error : undefined, - details: { - request: { - method, - url: url.toString(), - }, - error: errorContext.details, + throw new SubscriptionServiceError(`Failed to make request. ${message}`, { + cause: error instanceof Error ? error : undefined, + details: { + request: { + method, + url: url.toString(), }, + error: details, }, - ); + }); } } @@ -290,7 +286,6 @@ export class SubscriptionService implements ISubscriptionService { details: { name: 'HttpError', message: error.message, - stack: error.stack, httpStatus: error.httpStatus, }, }; @@ -302,7 +297,6 @@ export class SubscriptionService implements ISubscriptionService { details: { name: error.name, message: error.message, - stack: error.stack, }, }; } From 578a5e5a487fcfa42e71411387b869659680d3d4 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 5 Feb 2026 22:03:18 +0800 Subject: [PATCH 06/12] feat: replace 'handleFetch' with client side 'fetch' api in SubscriptionService --- .../src/SubscriptionService.ts | 266 +++++++++++------- .../subscription-controller/src/constants.ts | 20 ++ .../subscription-controller/src/errors.ts | 57 +++- packages/subscription-controller/src/index.ts | 6 +- 4 files changed, 231 insertions(+), 118 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index 186afedf6d3..7a55bafaeb7 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -1,8 +1,14 @@ -import { handleFetch, HttpError } from '@metamask/controller-utils'; - -import { getEnvUrls, SubscriptionControllerErrorMessage } from './constants'; +import { + getEnvUrls, + SubscriptionControllerErrorMessage, + SubscriptionServiceErrorMessage, +} from './constants'; import type { Env } from './constants'; -import { SubscriptionServiceError } from './errors'; +import { + createSentryError, + getErrorFromResponse, + SubscriptionServiceError, +} from './errors'; import type { AssignCohortRequest, AuthUtils, @@ -30,6 +36,8 @@ import type { export type SubscriptionServiceConfig = { env: Env; auth: AuthUtils; + fetchFunction: typeof fetch; + captureException?: (error: Error) => void; }; export const SUBSCRIPTION_URL = (env: Env, path: string): string => @@ -38,24 +46,38 @@ export const SUBSCRIPTION_URL = (env: Env, path: string): string => export class SubscriptionService implements ISubscriptionService { readonly #env: Env; + readonly #fetch: typeof fetch; + + readonly #captureException?: (error: Error) => void; + public authUtils: AuthUtils; constructor(config: SubscriptionServiceConfig) { this.#env = config.env; this.authUtils = config.auth; + this.#fetch = config.fetchFunction; + this.#captureException = config.captureException; } async getSubscriptions(): Promise { const path = 'subscriptions'; - return await this.#makeRequest(path); + return await this.#makeRequest({ + path, + errorMessage: SubscriptionServiceErrorMessage.FailedToGetSubscriptions, + }); } async cancelSubscription( params: CancelSubscriptionRequest, ): Promise { const path = `subscriptions/${params.subscriptionId}/cancel`; - return await this.#makeRequest(path, 'POST', { - cancelAtPeriodEnd: params.cancelAtPeriodEnd, + return await this.#makeRequest({ + path, + method: 'POST', + body: { + cancelAtPeriodEnd: params.cancelAtPeriodEnd, + }, + errorMessage: SubscriptionServiceErrorMessage.FailedToCancelSubscription, }); } @@ -63,7 +85,13 @@ export class SubscriptionService implements ISubscriptionService { subscriptionId: string; }): Promise { const path = `subscriptions/${params.subscriptionId}/uncancel`; - return await this.#makeRequest(path, 'POST', {}); + return await this.#makeRequest({ + path, + method: 'POST', + body: {}, + errorMessage: + SubscriptionServiceErrorMessage.FailedToUncancelSubscription, + }); } async startSubscriptionWithCard( @@ -76,37 +104,57 @@ export class SubscriptionService implements ISubscriptionService { } const path = 'subscriptions/card'; - return await this.#makeRequest(path, 'POST', request); + return await this.#makeRequest({ + path, + method: 'POST', + body: request, + errorMessage: + SubscriptionServiceErrorMessage.FailedToStartSubscriptionWithCard, + }); } async startSubscriptionWithCrypto( request: StartCryptoSubscriptionRequest, ): Promise { const path = 'subscriptions/crypto'; - return await this.#makeRequest(path, 'POST', request); + return await this.#makeRequest({ + path, + method: 'POST', + body: request, + errorMessage: + SubscriptionServiceErrorMessage.FailedToStartSubscriptionWithCrypto, + }); } async updatePaymentMethodCard( request: UpdatePaymentMethodCardRequest, ): Promise { const path = `subscriptions/${request.subscriptionId}/payment-method/card`; - return await this.#makeRequest( + return await this.#makeRequest({ path, - 'PATCH', - { + method: 'PATCH', + body: { ...request, subscriptionId: undefined, }, - ); + errorMessage: + SubscriptionServiceErrorMessage.FailedToUpdatePaymentMethodCard, + }); } async updatePaymentMethodCrypto( request: UpdatePaymentMethodCryptoRequest, ): Promise { const path = `subscriptions/${request.subscriptionId}/payment-method/crypto`; - await this.#makeRequest(path, 'PATCH', { - ...request, - subscriptionId: undefined, + await this.#makeRequest({ + path, + method: 'PATCH', + body: { + ...request, + subscriptionId: undefined, + }, + errorMessage: + SubscriptionServiceErrorMessage.FailedToUpdatePaymentMethodCrypto, }); } @@ -124,12 +172,12 @@ export class SubscriptionService implements ISubscriptionService { if (request?.balanceCategory !== undefined) { query = { balanceCategory: request.balanceCategory }; } - const results = await this.#makeRequest( + const results = await this.#makeRequest({ path, - 'GET', - undefined, - query, - ); + queryParams: query, + errorMessage: + SubscriptionServiceErrorMessage.FailedToGetSubscriptionsEligibilities, + }); return results.map((result) => ({ ...result, @@ -149,7 +197,12 @@ export class SubscriptionService implements ISubscriptionService { */ async submitUserEvent(request: SubmitUserEventRequest): Promise { const path = 'user-events'; - await this.#makeRequest(path, 'POST', request); + await this.#makeRequest({ + path, + method: 'POST', + body: request, + errorMessage: SubscriptionServiceErrorMessage.FailedToSubmitUserEvent, + }); } /** @@ -160,7 +213,12 @@ export class SubscriptionService implements ISubscriptionService { */ async assignUserToCohort(request: AssignCohortRequest): Promise { const path = 'cohorts/assign'; - await this.#makeRequest(path, 'POST', request); + await this.#makeRequest({ + path, + method: 'POST', + body: request, + errorMessage: SubscriptionServiceErrorMessage.FailedToAssignUserToCohort, + }); } /** @@ -176,7 +234,13 @@ export class SubscriptionService implements ISubscriptionService { request: SubmitSponsorshipIntentsRequest, ): Promise { const path = 'transaction-sponsorship/intents'; - await this.#makeRequest(path, 'POST', request); + await this.#makeRequest({ + path, + method: 'POST', + body: request, + errorMessage: + SubscriptionServiceErrorMessage.FailedToSubmitSponsorshipIntents, + }); } /** @@ -190,20 +254,54 @@ export class SubscriptionService implements ISubscriptionService { request: LinkRewardsRequest, ): Promise { const path = 'rewards/link'; - return await this.#makeRequest( + return await this.#makeRequest({ + path, + method: 'POST', + body: request, + errorMessage: SubscriptionServiceErrorMessage.FailedToLinkRewards, + }); + } + + async getPricing(): Promise { + const path = 'pricing'; + return await this.#makeRequest({ + path, + errorMessage: SubscriptionServiceErrorMessage.FailedToGetPricing, + }); + } + + async getBillingPortalUrl(): Promise { + const path = 'billing-portal'; + return await this.#makeRequest({ path, - 'POST', - request, - ); + errorMessage: SubscriptionServiceErrorMessage.FailedToGetBillingPortalUrl, + }); } - async #makeRequest( - path: string, - method: 'GET' | 'POST' | 'DELETE' | 'PUT' | 'PATCH' = 'GET', - body?: Record, - queryParams?: Record, - ): Promise { - const headers = await this.#getAuthorizationHeader(); + /** + * Makes a request to the Subscription Service backend. + * + * @param params - The request object containing the path, method, body, query parameters, and error message. + * @param params.path - The path of the request. + * @param params.method - The method of the request. + * @param params.body - The body of the request. + * @param params.queryParams - The query parameters of the request. + * @param params.errorMessage - The error message to throw if the request fails. + * @returns The result of the request. + */ + async #makeRequest({ + path, + method = 'GET', + body, + queryParams, + errorMessage, + }: { + path: string; + method?: 'GET' | 'POST' | 'DELETE' | 'PUT' | 'PATCH'; + body?: Record; + queryParams?: Record; + errorMessage: string; + }): Promise { const url = new URL(SUBSCRIPTION_URL(this.#env, path)); try { @@ -213,7 +311,8 @@ export class SubscriptionService implements ISubscriptionService { }); } - const response = await handleFetch(url.toString(), { + const headers = await this.#getAuthorizationHeader(); + const response = await this.#fetch(url.toString(), { method, headers: { 'Content-Type': 'application/json', @@ -222,90 +321,41 @@ export class SubscriptionService implements ISubscriptionService { body: body ? JSON.stringify(body) : undefined, }); - return response; + if (!response.ok) { + const error = await getErrorFromResponse(response); + throw error; + } + + const data = await response.json(); + return data; } catch (error) { - const { message, details } = this.#buildErrorContext(error); + console.error(errorMessage, error); + const errorMessageWithUrl = `${errorMessage} (url: ${url.toString()})`; + this.#captureException?.( + createSentryError(errorMessageWithUrl, error as Error), + ); - throw new SubscriptionServiceError(`Failed to make request. ${message}`, { - cause: error instanceof Error ? error : undefined, - details: { - request: { - method, - url: url.toString(), - }, - error: details, + throw new SubscriptionServiceError( + `Failed to make request. ${errorMessageWithUrl}`, + { + cause: error instanceof Error ? error : undefined, }, - }); + ); } } - async getPricing(): Promise { - const path = 'pricing'; - return await this.#makeRequest(path); - } - - async getBillingPortalUrl(): Promise { - const path = 'billing-portal'; - return await this.#makeRequest(path); - } - // eslint-disable-next-line @typescript-eslint/naming-convention async #getAuthorizationHeader(): Promise<{ Authorization: string }> { try { const accessToken = await this.authUtils.getAccessToken(); return { Authorization: `Bearer ${accessToken}` }; } catch (error) { - const errorContext = this.#buildErrorContext(error); - const errorMessage = errorContext.message; - - throw new SubscriptionServiceError( - `Failed to get authorization header. ${errorMessage}`, - { - cause: error instanceof Error ? error : undefined, - details: { - error: errorContext.details, - }, - }, - ); - } - } + const errorMessage = + error instanceof Error + ? error.message + : 'Unknown error when getting authorization header'; - /** - * Builds the error context for the SubscriptionService error. - * - * @param error - The error to build the context for. - * @returns The SubscriptionService error context. - */ - #buildErrorContext(error: unknown): { - message: string; - details: Record; - } { - if (error instanceof HttpError) { - return { - message: `${error.message} (status=${error.httpStatus})`, - details: { - name: 'HttpError', - message: error.message, - httpStatus: error.httpStatus, - }, - }; + throw new Error(`Failed to get authorization header. ${errorMessage}`); } - - if (error instanceof Error) { - return { - message: error.message, - details: { - name: error.name, - message: error.message, - }, - }; - } - - return { - message: `Non-Error thrown: ${JSON.stringify(error)}`, - details: { - nonErrorThrown: JSON.stringify(error), - }, - }; } } diff --git a/packages/subscription-controller/src/constants.ts b/packages/subscription-controller/src/constants.ts index e3a5a2c1fa4..ef96c1375fd 100644 --- a/packages/subscription-controller/src/constants.ts +++ b/packages/subscription-controller/src/constants.ts @@ -49,6 +49,26 @@ export enum SubscriptionControllerErrorMessage { LinkRewardsFailed = `${controllerName} - Failed to link rewards`, } +export enum SubscriptionServiceErrorMessage { + // Default error message for API Error + FailedToMakeRequest = 'Failed to make request', + + FailedToGetSubscriptions = 'Failed to get subscriptions', + FailedToCancelSubscription = 'Failed to cancel subscription', + FailedToUncancelSubscription = 'Failed to uncancel subscription', + FailedToStartSubscriptionWithCard = 'Failed to start subscription with card', + FailedToStartSubscriptionWithCrypto = 'Failed to start subscription with crypto', + FailedToUpdatePaymentMethodCard = 'Failed to update payment method card', + FailedToUpdatePaymentMethodCrypto = 'Failed to update payment method crypto', + FailedToGetSubscriptionsEligibilities = 'Failed to get subscriptions eligibilities', + FailedToSubmitUserEvent = 'Failed to submit user event', + FailedToAssignUserToCohort = 'Failed to assign user to cohort', + FailedToSubmitSponsorshipIntents = 'Failed to submit sponsorship intents', + FailedToLinkRewards = 'Failed to link rewards', + FailedToGetPricing = 'Failed to get pricing', + FailedToGetBillingPortalUrl = 'Failed to get billing portal url', +} + export const DEFAULT_POLLING_INTERVAL = 5 * 60 * 1_000; // 5 minutes export const ACTIVE_SUBSCRIPTION_STATUSES = [ diff --git a/packages/subscription-controller/src/errors.ts b/packages/subscription-controller/src/errors.ts index 50c9c540a04..cab13c85d12 100644 --- a/packages/subscription-controller/src/errors.ts +++ b/packages/subscription-controller/src/errors.ts @@ -1,26 +1,65 @@ -export type SubscriptionServiceErrorDetails = Record; - export class SubscriptionServiceError extends Error { /** * The underlying error that caused this error. */ cause?: Error; - /** - * Additional details about the error. - */ - details?: SubscriptionServiceErrorDetails; - constructor( message: string, options?: { cause?: Error; - details?: SubscriptionServiceErrorDetails; }, ) { super(message); this.name = 'SubscriptionServiceError'; - this.details = options?.details; this.cause = options?.cause; } } + +/** + * Get an error from a response. + * + * @param response - The response to get an error from. + * @returns An error. + */ +export async function getErrorFromResponse(response: Response): Promise { + const contentType = response.headers?.get('content-type'); + const statusCode = response.status; + try { + if (contentType?.includes('application/json')) { + const json = await response.json(); + const errorMessage = json?.error ?? json?.message ?? 'Unknown error'; + const networkError = `error: ${errorMessage}, statusCode: ${statusCode}`; + return new Error(networkError); + } else if (contentType?.includes('text/plain')) { + const text = await response.text(); + const networkError = `error: ${text}, statusCode: ${statusCode}`; + return new Error(networkError); + } + + const error = + 'data' in response && typeof response.data === 'string' + ? response.data + : 'Unknown error'; + const networkError = `error: ${error}, statusCode: ${statusCode}`; + return new Error(networkError); + } catch { + return new Error(`HTTP ${statusCode} error`); + } +} + +/** + * Creates an error instance with a readable message and the root cause. + * + * @param message - The error message to create a Sentry error from. + * @param cause - The inner error to create a Sentry error from. + * @returns A Sentry error. + */ +export function createSentryError(message: string, cause: Error): Error { + const sentryError = new Error(message) as Error & { + cause: Error; + }; + sentryError.cause = cause; + + return sentryError; +} diff --git a/packages/subscription-controller/src/index.ts b/packages/subscription-controller/src/index.ts index f219651b544..01e76893efc 100644 --- a/packages/subscription-controller/src/index.ts +++ b/packages/subscription-controller/src/index.ts @@ -86,6 +86,10 @@ export { MODAL_TYPE, } from './types'; export { SubscriptionServiceError } from './errors'; -export { Env, SubscriptionControllerErrorMessage } from './constants'; +export { + Env, + SubscriptionControllerErrorMessage, + SubscriptionServiceErrorMessage, +} from './constants'; export type { SubscriptionServiceConfig } from './SubscriptionService'; export { SubscriptionService } from './SubscriptionService'; From a41b5334faeac539e1366f816652726d3931ff08 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 5 Feb 2026 22:03:28 +0800 Subject: [PATCH 07/12] test: updated tests --- .../src/SubscriptionService.test.ts | 284 +++++++++++------- .../src/errors.test.ts | 151 ++++++++++ 2 files changed, 330 insertions(+), 105 deletions(-) create mode 100644 packages/subscription-controller/src/errors.test.ts diff --git a/packages/subscription-controller/src/SubscriptionService.test.ts b/packages/subscription-controller/src/SubscriptionService.test.ts index d2e49f0521f..07f2926ca93 100644 --- a/packages/subscription-controller/src/SubscriptionService.test.ts +++ b/packages/subscription-controller/src/SubscriptionService.test.ts @@ -1,9 +1,8 @@ -import { handleFetch, HttpError } from '@metamask/controller-utils'; - import { Env, getEnvUrls, SubscriptionControllerErrorMessage, + SubscriptionServiceErrorMessage, } from './constants'; import { SubscriptionServiceError } from './errors'; import { @@ -29,14 +28,6 @@ import { SubscriptionUserEvent, } from './types'; -// Mock the handleFetch function -jest.mock('@metamask/controller-utils', () => ({ - ...jest.requireActual('@metamask/controller-utils'), - handleFetch: jest.fn(), -})); - -const handleFetchMock = handleFetch as jest.Mock; - // Mock data const MOCK_SUBSCRIPTION: Subscription = { id: 'sub_123456789', @@ -116,6 +107,38 @@ function createMockEligibilityResponse( }; } +type MockConfig = SubscriptionServiceConfig & { + fetchMock: jest.Mock; + captureExceptionMock: jest.Mock; +}; + +type MockResponseOptions = { + ok?: boolean; + status?: number; + jsonData?: unknown; + textData?: string; + contentType?: string | null; +}; + +function createMockResponse({ + ok = true, + status = 200, + jsonData, + textData = '', + contentType = 'application/json', +}: MockResponseOptions): Response { + return { + ok, + status, + headers: { + get: (key: string) => + key.toLowerCase() === 'content-type' ? contentType : null, + }, + json: jest.fn().mockResolvedValue(jsonData), + text: jest.fn().mockResolvedValue(textData), + } as unknown as Response; +} + /** * Creates a mock subscription service config for testing * @@ -123,15 +146,20 @@ function createMockEligibilityResponse( * @param [params.env] - The environment to use for the config * @returns The mock configuration object */ -function createMockConfig({ - env = Env.DEV, -}: { env?: Env } = {}): SubscriptionServiceConfig { +function createMockConfig({ env = Env.DEV }: { env?: Env } = {}): MockConfig { + const fetchMock = jest.fn(); + const captureExceptionMock = jest.fn(); + return { env, auth: { getAccessToken: jest.fn().mockResolvedValue(MOCK_ACCESS_TOKEN), }, - }; + fetchFunction: fetchMock, + captureException: captureExceptionMock, + fetchMock, + captureExceptionMock, + } as MockConfig; } /** @@ -190,11 +218,15 @@ describe('SubscriptionService', () => { describe('getSubscriptions', () => { it('should fetch subscriptions successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({ - customerId: 'cus_1', - subscriptions: [MOCK_SUBSCRIPTION], - trialedProducts: [], - }); + config.fetchMock.mockResolvedValue( + createMockResponse({ + jsonData: { + customerId: 'cus_1', + subscriptions: [MOCK_SUBSCRIPTION], + trialedProducts: [], + }, + }), + ); const result = await service.getSubscriptions(); @@ -208,8 +240,9 @@ describe('SubscriptionService', () => { }); it('should throw SubscriptionServiceError for network errors', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + await withMockSubscriptionService(async ({ service, config }) => { + const networkError = new Error('Network error'); + config.fetchMock.mockRejectedValue(networkError); const error = await service.getSubscriptions().then( () => { @@ -220,50 +253,43 @@ describe('SubscriptionService', () => { expect(error).toBeInstanceOf(SubscriptionServiceError); const serviceError = error as SubscriptionServiceError; - expect(serviceError.details).toMatchObject({ - request: { - method: 'GET', - url: `${getTestUrl(Env.DEV)}/v1/subscriptions`, - }, - }); + expect(serviceError.message).toBe( + `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, + ); + expect(serviceError.cause).toBe(networkError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); }); - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue('string error'); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue('string error'); + + const requestPromise = service.getSubscriptions(); - await expect(service.getSubscriptions()).rejects.toThrow( - SubscriptionServiceError, + await expect(requestPromise).rejects.toThrow(SubscriptionServiceError); + await expect(requestPromise).rejects.toThrow( + `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, ); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); }); }); - it('should include httpStatus details for HttpError responses', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue( - new HttpError(503, 'Service unavailable'), + it('should throw SubscriptionServiceError for non-ok responses', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ + ok: false, + status: 500, + jsonData: { error: 'Internal Server Error' }, + }), ); - const error = await service.getSubscriptions().then( - () => { - throw new Error('Expected getSubscriptions to throw'); - }, - (rejection) => rejection, - ); + const requestPromise = service.getSubscriptions(); - expect(error).toBeInstanceOf(SubscriptionServiceError); - const serviceError = error as SubscriptionServiceError; - expect(serviceError.message).toContain('status=503'); - expect(serviceError.details).toMatchObject({ - request: { - method: 'GET', - url: `${getTestUrl(Env.DEV)}/v1/subscriptions`, - }, - error: { - httpStatus: 503, - message: 'Service unavailable', - name: 'HttpError', - }, - }); + await expect(requestPromise).rejects.toThrow(SubscriptionServiceError); + await expect(requestPromise).rejects.toThrow( + `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, + ); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); }); }); @@ -274,8 +300,11 @@ describe('SubscriptionService', () => { 'string error', ); - await expect(service.getSubscriptions()).rejects.toThrow( - SubscriptionServiceError, + const requestPromise = service.getSubscriptions(); + + await expect(requestPromise).rejects.toThrow(SubscriptionServiceError); + await expect(requestPromise).rejects.toThrow( + `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, ); }); @@ -285,8 +314,11 @@ describe('SubscriptionService', () => { new Error('Wallet is locked'), ); - await expect(service.getSubscriptions()).rejects.toThrow( - SubscriptionServiceError, + const requestPromise = service.getSubscriptions(); + + await expect(requestPromise).rejects.toThrow(SubscriptionServiceError); + await expect(requestPromise).rejects.toThrow( + `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, ); }); }); @@ -295,7 +327,9 @@ describe('SubscriptionService', () => { describe('cancelSubscription', () => { it('should cancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.cancelSubscription({ subscriptionId: 'sub_123456789' }); @@ -304,8 +338,8 @@ describe('SubscriptionService', () => { }); it('should throw SubscriptionServiceError for network errors', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); await expect( service.cancelSubscription({ subscriptionId: 'sub_123456789' }), @@ -317,7 +351,9 @@ describe('SubscriptionService', () => { describe('uncancelSubscription', () => { it('should cancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.unCancelSubscription({ subscriptionId: 'sub_123456789' }); @@ -326,8 +362,8 @@ describe('SubscriptionService', () => { }); it('should throw SubscriptionServiceError for network errors', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); await expect( service.unCancelSubscription({ subscriptionId: 'sub_123456789' }), @@ -338,8 +374,10 @@ describe('SubscriptionService', () => { describe('startSubscription', () => { it('should start subscription successfully', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: MOCK_START_SUBSCRIPTION_RESPONSE }), + ); const result = await service.startSubscriptionWithCard( MOCK_START_SUBSCRIPTION_REQUEST, @@ -358,7 +396,9 @@ describe('SubscriptionService', () => { recurringInterval: RECURRING_INTERVALS.month, }; - handleFetchMock.mockResolvedValue(MOCK_START_SUBSCRIPTION_RESPONSE); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: MOCK_START_SUBSCRIPTION_RESPONSE }), + ); const result = await service.startSubscriptionWithCard(request); @@ -382,7 +422,7 @@ describe('SubscriptionService', () => { describe('startCryptoSubscription', () => { it('should start crypto subscription successfully', async () => { - await withMockSubscriptionService(async ({ service }) => { + await withMockSubscriptionService(async ({ service, config }) => { const request: StartCryptoSubscriptionRequest = { products: [PRODUCT_TYPES.SHIELD], isTrialRequested: false, @@ -399,7 +439,9 @@ describe('SubscriptionService', () => { status: SUBSCRIPTION_STATUSES.active, }; - handleFetchMock.mockResolvedValue(response); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: response }), + ); const result = await service.startSubscriptionWithCrypto(request); @@ -418,7 +460,9 @@ describe('SubscriptionService', () => { const config = createMockConfig(); const service = new SubscriptionService(config); - handleFetchMock.mockResolvedValue(mockPricingResponse); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: mockPricingResponse }), + ); const result = await service.getPricing(); @@ -434,11 +478,13 @@ describe('SubscriptionService', () => { recurringInterval: RECURRING_INTERVALS.month, }; - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.updatePaymentMethodCard(request); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL( config.env, 'subscriptions/sub_123456789/payment-method/card', @@ -467,11 +513,13 @@ describe('SubscriptionService', () => { billingCycles: 3, }; - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.updatePaymentMethodCrypto(request); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL( config.env, 'subscriptions/sub_123456789/payment-method/crypto', @@ -491,10 +539,14 @@ describe('SubscriptionService', () => { describe('getBillingPortalUrl', () => { it('should get billing portal url successfully', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockResolvedValue({ - url: 'https://billing-portal.com', - }); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ + jsonData: { + url: 'https://billing-portal.com', + }, + }), + ); const result = await service.getBillingPortalUrl(); @@ -505,9 +557,11 @@ describe('SubscriptionService', () => { describe('getShieldSubscriptionEligibility', () => { it('should get shield subscription eligibility successfully', async () => { - await withMockSubscriptionService(async ({ service }) => { + await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - handleFetchMock.mockResolvedValue([mockResponse]); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: [mockResponse] }), + ); const results = await service.getSubscriptionsEligibilities(); @@ -516,14 +570,16 @@ describe('SubscriptionService', () => { }); it('should get shield subscription eligibility with cohort information', async () => { - await withMockSubscriptionService(async ({ service }) => { + await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse({ cohorts: MOCK_COHORTS, assignedCohort: 'post_tx', assignedAt: '2024-01-01T00:00:00Z', }); - handleFetchMock.mockResolvedValue([mockResponse]); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: [mockResponse] }), + ); const results = await service.getSubscriptionsEligibilities({ balanceCategory: '1k-9.9k', @@ -534,12 +590,16 @@ describe('SubscriptionService', () => { }); it('should get shield subscription eligibility with default values', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockResolvedValue([ - { - product: PRODUCT_TYPES.SHIELD, - }, - ]); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ + jsonData: [ + { + product: PRODUCT_TYPES.SHIELD, + }, + ], + }), + ); const results = await service.getSubscriptionsEligibilities(); @@ -556,13 +616,15 @@ describe('SubscriptionService', () => { it('should pass balanceCategory as query parameter when provided', async () => { await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - handleFetchMock.mockResolvedValue([mockResponse]); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: [mockResponse] }), + ); await service.getSubscriptionsEligibilities({ balanceCategory: '100-999', }); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( expect.stringContaining('balanceCategory=100-999'), expect.objectContaining({ method: 'GET', @@ -576,11 +638,13 @@ describe('SubscriptionService', () => { it('should not pass balanceCategory query parameter when not provided', async () => { await withMockSubscriptionService(async ({ service, config }) => { const mockResponse = createMockEligibilityResponse(); - handleFetchMock.mockResolvedValue([mockResponse]); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: [mockResponse] }), + ); await service.getSubscriptionsEligibilities(); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( expect.not.stringMatching(/balanceCategory/u), expect.objectContaining({ method: 'GET', @@ -595,13 +659,15 @@ describe('SubscriptionService', () => { describe('submitUserEvent', () => { it('should submit user event successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.submitUserEvent({ event: SubscriptionUserEvent.ShieldEntryModalViewed, }); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL(config.env, 'user-events'), { method: 'POST', @@ -616,14 +682,16 @@ describe('SubscriptionService', () => { it('should submit user event with cohort successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.submitUserEvent({ event: SubscriptionUserEvent.ShieldEntryModalViewed, cohort: 'post_tx', }); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL(config.env, 'user-events'), { method: 'POST', @@ -641,11 +709,13 @@ describe('SubscriptionService', () => { describe('assignUserToCohort', () => { it('should assign user to cohort successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.assignUserToCohort({ cohort: 'post_tx' }); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL(config.env, 'cohorts/assign'), { method: 'POST', @@ -660,8 +730,8 @@ describe('SubscriptionService', () => { }); it('should handle cohort assignment errors', async () => { - await withMockSubscriptionService(async ({ service }) => { - handleFetchMock.mockRejectedValue(new Error('Network error')); + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); await expect( service.assignUserToCohort({ cohort: 'wallet_home' }), @@ -673,7 +743,9 @@ describe('SubscriptionService', () => { describe('submitSponsorshipIntents', () => { it('should submit sponsorship intents successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.submitSponsorshipIntents({ chainId: '0x1', @@ -684,7 +756,7 @@ describe('SubscriptionService', () => { paymentTokenSymbol: 'USDT', }); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL(config.env, 'transaction-sponsorship/intents'), { method: 'POST', @@ -706,14 +778,16 @@ describe('SubscriptionService', () => { describe('linkRewards', () => { it('should link rewards successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - handleFetchMock.mockResolvedValue({}); + config.fetchMock.mockResolvedValue( + createMockResponse({ jsonData: {} }), + ); await service.linkRewards({ rewardAccountId: 'eip155:1:0x1234567890123456789012345678901234567890', }); - expect(handleFetchMock).toHaveBeenCalledWith( + expect(config.fetchMock).toHaveBeenCalledWith( SUBSCRIPTION_URL(config.env, 'rewards/link'), { method: 'POST', diff --git a/packages/subscription-controller/src/errors.test.ts b/packages/subscription-controller/src/errors.test.ts new file mode 100644 index 00000000000..93c0f63140a --- /dev/null +++ b/packages/subscription-controller/src/errors.test.ts @@ -0,0 +1,151 @@ +import { + createSentryError, + getErrorFromResponse, + SubscriptionServiceError, +} from './errors'; + +type MockResponseOptions = { + status?: number; + contentType?: string | null; + jsonData?: unknown; + textData?: string; + jsonThrows?: boolean; + data?: string; +}; + +function createMockResponse({ + status = 500, + contentType = 'application/json', + jsonData = {}, + textData = 'plain error', + jsonThrows = false, + data, +}: MockResponseOptions): Response { + return { + status, + headers: { + get: (key: string) => + key.toLowerCase() === 'content-type' ? contentType : null, + }, + json: jsonThrows + ? jest.fn().mockRejectedValue(new Error('bad json')) + : jest.fn().mockResolvedValue(jsonData), + text: jest.fn().mockResolvedValue(textData), + data, + } as unknown as Response; +} + +describe('errors', () => { + describe('SubscriptionServiceError', () => { + it('sets name and cause', () => { + const cause = new Error('root cause'); + const error = new SubscriptionServiceError('message', { cause }); + + expect(error.name).toBe('SubscriptionServiceError'); + expect(error.message).toBe('message'); + expect(error.cause).toBe(cause); + }); + }); + + describe('createSentryError', () => { + it('wraps the cause on the error', () => { + const cause = new Error('inner'); + const error = createSentryError('outer', cause); + + expect(error.message).toBe('outer'); + expect((error as Error & { cause: Error }).cause).toBe(cause); + }); + }); + + describe('getErrorFromResponse', () => { + it('uses JSON error message when content-type is json', async () => { + const response = createMockResponse({ + contentType: 'application/json', + status: 400, + jsonData: { error: 'Bad request' }, + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toContain('error: Bad request'); + expect(error.message).toContain('statusCode: 400'); + }); + + it('uses JSON message when error field is missing', async () => { + const response = createMockResponse({ + contentType: 'application/json', + status: 422, + jsonData: { message: 'Unprocessable' }, + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toContain('error: Unprocessable'); + expect(error.message).toContain('statusCode: 422'); + }); + + it('uses Unknown error when JSON has no message', async () => { + const response = createMockResponse({ + contentType: 'application/json', + status: 418, + jsonData: { detail: 'teapot' }, + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toContain('error: Unknown error'); + expect(error.message).toContain('statusCode: 418'); + }); + + it('uses text body when content-type is text/plain', async () => { + const response = createMockResponse({ + contentType: 'text/plain', + status: 503, + textData: 'Service unavailable', + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toContain('error: Service unavailable'); + expect(error.message).toContain('statusCode: 503'); + }); + + it('uses response data when content-type is missing', async () => { + const response = createMockResponse({ + contentType: null, + status: 500, + data: 'fallback data', + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toContain('error: fallback data'); + expect(error.message).toContain('statusCode: 500'); + }); + + it('uses Unknown error when response data is not a string', async () => { + const response = createMockResponse({ + contentType: null, + status: 500, + data: { code: 'UNKNOWN' } as unknown as string, + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toContain('error: Unknown error'); + expect(error.message).toContain('statusCode: 500'); + }); + + it('returns generic HTTP error when parsing fails', async () => { + const response = createMockResponse({ + contentType: 'application/json', + status: 502, + jsonThrows: true, + }); + + const error = await getErrorFromResponse(response); + + expect(error.message).toBe('HTTP 502 error'); + }); + }); +}); From 75ef860a4f0433cd41479a92a34c21b12c1445cc Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 5 Feb 2026 23:04:41 +0800 Subject: [PATCH 08/12] chore: updated ChangeLog --- packages/subscription-controller/CHANGELOG.md | 6 ++++++ .../src/SubscriptionController.test.ts | 18 +----------------- .../src/SubscriptionController.ts | 11 +---------- .../src/SubscriptionService.ts | 6 +++++- .../subscription-controller/src/constants.ts | 3 --- 5 files changed, 13 insertions(+), 31 deletions(-) diff --git a/packages/subscription-controller/CHANGELOG.md b/packages/subscription-controller/CHANGELOG.md index a9b3b4fbb3b..cf337ea3684 100644 --- a/packages/subscription-controller/CHANGELOG.md +++ b/packages/subscription-controller/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING**: Added two new params, `captureException` and `fetchFunction` to `SubscriptionService` constructor args. ([#7835](https://github.com/MetaMask/core/pull/7835)) + - `fetchFunction` is to use the client provided `Fetch` API. + - `captureException` is to capture the error thrown and report to Sentry. + ### Changed - Updated `SubscriptionServiceError` to include more information for Sentry reporting. ([#7835](https://github.com/MetaMask/core/pull/7835)) diff --git a/packages/subscription-controller/src/SubscriptionController.test.ts b/packages/subscription-controller/src/SubscriptionController.test.ts index 37b33f23fce..628d2b2735d 100644 --- a/packages/subscription-controller/src/SubscriptionController.test.ts +++ b/packages/subscription-controller/src/SubscriptionController.test.ts @@ -447,23 +447,7 @@ describe('SubscriptionController', () => { }); await expect(controller.getSubscriptions()).rejects.toThrow( - 'Failed to trigger access token refresh. Wallet is locked', - ); - }, - ); - - await withController( - async ({ controller, mockService, mockPerformSignOut }) => { - mockService.getSubscriptions.mockResolvedValue( - MOCK_GET_SUBSCRIPTIONS_RESPONSE, - ); - mockPerformSignOut.mockImplementation(() => { - // eslint-disable-next-line @typescript-eslint/only-throw-error - throw 'string error'; - }); - - await expect(controller.getSubscriptions()).rejects.toThrow( - `Failed to trigger access token refresh. ${JSON.stringify('string error')}`, + 'Wallet is locked', ); }, ); diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index ff1048c4636..26a9393dc4f 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -960,16 +960,7 @@ export class SubscriptionController extends StaticIntervalPollingController()< // We perform a sign out to clear the access token from the authentication // controller. Next time the access token is requested, a new access token // will be fetched. - try { - this.messenger.call('AuthenticationController:performSignOut'); - } catch (error) { - const errorMessage = - error instanceof Error ? error.message : JSON.stringify(error); - - throw new Error( - `Failed to trigger access token refresh. ${errorMessage}`, - ); - } + this.messenger.call('AuthenticationController:performSignOut'); } #getProductPriceByProductAndPlan( diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index 7a55bafaeb7..aadc5478414 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -330,9 +330,13 @@ export class SubscriptionService implements ISubscriptionService { return data; } catch (error) { console.error(errorMessage, error); + const errorMessageWithUrl = `${errorMessage} (url: ${url.toString()})`; this.#captureException?.( - createSentryError(errorMessageWithUrl, error as Error), + createSentryError( + errorMessageWithUrl, + error instanceof Error ? error : new Error(errorMessage), + ), ); throw new SubscriptionServiceError( diff --git a/packages/subscription-controller/src/constants.ts b/packages/subscription-controller/src/constants.ts index ef96c1375fd..c7fa0ecc456 100644 --- a/packages/subscription-controller/src/constants.ts +++ b/packages/subscription-controller/src/constants.ts @@ -50,9 +50,6 @@ export enum SubscriptionControllerErrorMessage { } export enum SubscriptionServiceErrorMessage { - // Default error message for API Error - FailedToMakeRequest = 'Failed to make request', - FailedToGetSubscriptions = 'Failed to get subscriptions', FailedToCancelSubscription = 'Failed to cancel subscription', FailedToUncancelSubscription = 'Failed to uncancel subscription', From 5ff83bad8d784a6671a62a40bcfb000c6b10fd01 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 5 Feb 2026 23:51:32 +0800 Subject: [PATCH 09/12] feat: more errors handling --- .../src/SubscriptionService.test.ts | 60 ++++++++++++++++++- .../src/SubscriptionService.ts | 47 ++++++++++++--- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionService.test.ts b/packages/subscription-controller/src/SubscriptionService.test.ts index 07f2926ca93..ff04d102e60 100644 --- a/packages/subscription-controller/src/SubscriptionService.test.ts +++ b/packages/subscription-controller/src/SubscriptionService.test.ts @@ -4,6 +4,7 @@ import { SubscriptionControllerErrorMessage, SubscriptionServiceErrorMessage, } from './constants'; +import * as constants from './constants'; import { SubscriptionServiceError } from './errors'; import { SUBSCRIPTION_URL, @@ -239,6 +240,54 @@ describe('SubscriptionService', () => { }); }); + it('should throw when URL construction fails', async () => { + const config = createMockConfig({ env: 'invalid' as Env }); + const service = new SubscriptionService(config); + + await expect(service.getSubscriptions()).rejects.toThrow( + 'invalid environment configuration', + ); + expect(config.fetchMock).not.toHaveBeenCalled(); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + const capturedError = config.captureExceptionMock.mock + .calls[0][0] as Error & { cause?: Error }; + expect(capturedError.message).toBe( + 'Failed to get subscription API URL. invalid environment configuration', + ); + }); + + it('should capture non-Error URL construction failures', async () => { + const config = createMockConfig(); + const service = new SubscriptionService(config); + const getEnvUrlsSpy = jest + .spyOn(constants, 'getEnvUrls') + .mockImplementation(() => { + // eslint-disable-next-line @typescript-eslint/only-throw-error + throw 'string error'; + }); + + try { + const error = await service + .getSubscriptions() + .catch((rejection) => rejection); + expect(error).toBe('string error'); + } finally { + getEnvUrlsSpy.mockRestore(); + } + + expect(config.fetchMock).not.toHaveBeenCalled(); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + const capturedError = config.captureExceptionMock.mock + .calls[0][0] as Error & { cause?: Error }; + expect(capturedError.message).toBe( + 'Failed to get subscription API URL. Unknown error when getting subscription API URL', + ); + expect(capturedError.cause).toBeInstanceOf(Error); + expect(capturedError.cause?.message).toBe( + 'Unknown error when getting subscription API URL', + ); + }); + it('should throw SubscriptionServiceError for network errors', async () => { await withMockSubscriptionService(async ({ service, config }) => { const networkError = new Error('Network error'); @@ -269,6 +318,13 @@ describe('SubscriptionService', () => { await expect(requestPromise).rejects.toThrow( `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, ); + const error = await requestPromise.catch((rejection) => rejection); + expect(error).toBeInstanceOf(SubscriptionServiceError); + const serviceError = error as SubscriptionServiceError; + expect(serviceError.cause).toBeInstanceOf(Error); + expect(serviceError.cause?.message).toBe( + SubscriptionServiceErrorMessage.FailedToGetSubscriptions, + ); expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); }); }); @@ -304,7 +360,7 @@ describe('SubscriptionService', () => { await expect(requestPromise).rejects.toThrow(SubscriptionServiceError); await expect(requestPromise).rejects.toThrow( - `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, + 'Failed to get authorization header. Unknown error when getting authorization header', ); }); @@ -318,7 +374,7 @@ describe('SubscriptionService', () => { await expect(requestPromise).rejects.toThrow(SubscriptionServiceError); await expect(requestPromise).rejects.toThrow( - `Failed to make request. ${SubscriptionServiceErrorMessage.FailedToGetSubscriptions} (url: ${getTestUrl(Env.DEV)}/v1/subscriptions)`, + 'Failed to get authorization header. Wallet is locked', ); }); }); diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index aadc5478414..89316abdabe 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -302,7 +302,8 @@ export class SubscriptionService implements ISubscriptionService { queryParams?: Record; errorMessage: string; }): Promise { - const url = new URL(SUBSCRIPTION_URL(this.#env, path)); + const url = this.#getSubscriptionApiUrl(path); + const headers = await this.#getAuthorizationHeader(); try { if (queryParams) { @@ -311,7 +312,6 @@ export class SubscriptionService implements ISubscriptionService { }); } - const headers = await this.#getAuthorizationHeader(); const response = await this.#fetch(url.toString(), { method, headers: { @@ -332,17 +332,16 @@ export class SubscriptionService implements ISubscriptionService { console.error(errorMessage, error); const errorMessageWithUrl = `${errorMessage} (url: ${url.toString()})`; + const errorToCapture = + error instanceof Error ? error : new Error(errorMessage); this.#captureException?.( - createSentryError( - errorMessageWithUrl, - error instanceof Error ? error : new Error(errorMessage), - ), + createSentryError(errorMessageWithUrl, errorToCapture), ); throw new SubscriptionServiceError( `Failed to make request. ${errorMessageWithUrl}`, { - cause: error instanceof Error ? error : undefined, + cause: errorToCapture, }, ); } @@ -359,7 +358,39 @@ export class SubscriptionService implements ISubscriptionService { ? error.message : 'Unknown error when getting authorization header'; - throw new Error(`Failed to get authorization header. ${errorMessage}`); + this.#captureException?.( + createSentryError( + `Failed to get authorization header. ${errorMessage}`, + error instanceof Error ? error : new Error(errorMessage), + ), + ); + + throw new SubscriptionServiceError( + `Failed to get authorization header. ${errorMessage}`, + { + cause: error instanceof Error ? error : new Error(errorMessage), + }, + ); + } + } + + #getSubscriptionApiUrl(path: string): URL { + try { + const url = new URL(SUBSCRIPTION_URL(this.#env, path)); + return url; + } catch (error) { + const errorMessage = + error instanceof Error + ? error.message + : 'Unknown error when getting subscription API URL'; + this.#captureException?.( + createSentryError( + `Failed to get subscription API URL. ${errorMessage}`, + error instanceof Error ? error : new Error(errorMessage), + ), + ); + + throw error; } } } From f4d0744371c5645ed1862d56ffc21b1202f66ee2 Mon Sep 17 00:00:00 2001 From: lwin Date: Mon, 9 Feb 2026 12:42:14 +0800 Subject: [PATCH 10/12] feat: added ModuleLogger --- .../subscription-controller/src/SubscriptionService.ts | 5 ++++- packages/subscription-controller/src/logger.ts | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 packages/subscription-controller/src/logger.ts diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index 89316abdabe..ca1eaa833d7 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -9,6 +9,7 @@ import { getErrorFromResponse, SubscriptionServiceError, } from './errors'; +import { createModuleLogger, projectLogger } from './logger'; import type { AssignCohortRequest, AuthUtils, @@ -43,6 +44,8 @@ export type SubscriptionServiceConfig = { export const SUBSCRIPTION_URL = (env: Env, path: string): string => `${getEnvUrls(env).subscriptionApiUrl}/v1/${path}`; +const log = createModuleLogger(projectLogger, 'SubscriptionService'); + export class SubscriptionService implements ISubscriptionService { readonly #env: Env; @@ -329,7 +332,7 @@ export class SubscriptionService implements ISubscriptionService { const data = await response.json(); return data; } catch (error) { - console.error(errorMessage, error); + log(errorMessage, error); const errorMessageWithUrl = `${errorMessage} (url: ${url.toString()})`; const errorToCapture = diff --git a/packages/subscription-controller/src/logger.ts b/packages/subscription-controller/src/logger.ts new file mode 100644 index 00000000000..ca017b5ba54 --- /dev/null +++ b/packages/subscription-controller/src/logger.ts @@ -0,0 +1,7 @@ +import { createProjectLogger, createModuleLogger } from '@metamask/utils'; + +import { controllerName } from './constants'; + +export const projectLogger = createProjectLogger(controllerName); + +export { createModuleLogger }; From 166b72a1904c506540cd2491eab133bc1cef4b8d Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 10 Feb 2026 16:30:23 +0800 Subject: [PATCH 11/12] chore: include 'errorCode' in SubscriptionService error message --- .../src/SubscriptionService.test.ts | 174 ++++++++++++++++-- .../src/SubscriptionService.ts | 4 +- .../src/errors.test.ts | 93 ++++++++-- .../subscription-controller/src/errors.ts | 34 +++- packages/subscription-controller/src/index.ts | 1 + packages/subscription-controller/src/types.ts | 9 + 6 files changed, 284 insertions(+), 31 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionService.test.ts b/packages/subscription-controller/src/SubscriptionService.test.ts index ff04d102e60..a64902d8ee0 100644 --- a/packages/subscription-controller/src/SubscriptionService.test.ts +++ b/packages/subscription-controller/src/SubscriptionService.test.ts @@ -240,6 +240,31 @@ describe('SubscriptionService', () => { }); }); + it('should send correct URL and headers', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ + jsonData: { + customerId: 'cus_1', + subscriptions: [], + trialedProducts: [], + }, + }), + ); + + await service.getSubscriptions(); + + expect(config.fetchMock).toHaveBeenCalledWith( + SUBSCRIPTION_URL(config.env, 'subscriptions'), + { + method: 'GET', + headers: MOCK_HEADERS, + body: undefined, + }, + ); + }); + }); + it('should throw when URL construction fails', async () => { const config = createMockConfig({ env: 'invalid' as Env }); const service = new SubscriptionService(config); @@ -402,10 +427,27 @@ describe('SubscriptionService', () => { ).rejects.toThrow(SubscriptionServiceError); }); }); + + it('should throw SubscriptionServiceError for non-ok responses', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ + ok: false, + status: 404, + jsonData: { message: 'Subscription not found' }, + }), + ); + + await expect( + service.cancelSubscription({ subscriptionId: 'sub_invalid' }), + ).rejects.toThrow(SubscriptionServiceError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); }); describe('uncancelSubscription', () => { - it('should cancel subscription successfully', async () => { + it('should uncancel subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { config.fetchMock.mockResolvedValue( createMockResponse({ jsonData: {} }), @@ -474,22 +516,32 @@ describe('SubscriptionService', () => { SubscriptionControllerErrorMessage.SubscriptionProductsEmpty, ); }); + + it('should throw SubscriptionServiceError for network errors', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); + + await expect( + service.startSubscriptionWithCard(MOCK_START_SUBSCRIPTION_REQUEST), + ).rejects.toThrow(SubscriptionServiceError); + }); + }); }); describe('startCryptoSubscription', () => { + const MOCK_CRYPTO_REQUEST: StartCryptoSubscriptionRequest = { + products: [PRODUCT_TYPES.SHIELD], + isTrialRequested: false, + recurringInterval: RECURRING_INTERVALS.month, + billingCycles: 3, + chainId: '0x1', + payerAddress: '0x0000000000000000000000000000000000000001', + tokenSymbol: 'USDC', + rawTransaction: '0xdeadbeef', + }; + it('should start crypto subscription successfully', async () => { await withMockSubscriptionService(async ({ service, config }) => { - const request: StartCryptoSubscriptionRequest = { - products: [PRODUCT_TYPES.SHIELD], - isTrialRequested: false, - recurringInterval: RECURRING_INTERVALS.month, - billingCycles: 3, - chainId: '0x1', - payerAddress: '0x0000000000000000000000000000000000000001', - tokenSymbol: 'USDC', - rawTransaction: '0xdeadbeef', - }; - const response = { subscriptionId: 'sub_crypto_123', status: SUBSCRIPTION_STATUSES.active, @@ -499,7 +551,8 @@ describe('SubscriptionService', () => { createMockResponse({ jsonData: response }), ); - const result = await service.startSubscriptionWithCrypto(request); + const result = + await service.startSubscriptionWithCrypto(MOCK_CRYPTO_REQUEST); expect(result).toStrictEqual(response); }); @@ -591,6 +644,25 @@ describe('SubscriptionService', () => { ); }); }); + + it('should throw SubscriptionServiceError for crypto payment method errors', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); + + await expect( + service.updatePaymentMethodCrypto({ + subscriptionId: 'sub_123456789', + chainId: '0x1', + payerAddress: '0x0000000000000000000000000000000000000001', + tokenSymbol: 'USDC', + rawTransaction: '0xdeadbeef', + recurringInterval: RECURRING_INTERVALS.month, + billingCycles: 3, + }), + ).rejects.toThrow(SubscriptionServiceError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); }); describe('getBillingPortalUrl', () => { @@ -710,6 +782,17 @@ describe('SubscriptionService', () => { expect(config.auth.getAccessToken).toHaveBeenCalledTimes(1); }); }); + + it('should throw SubscriptionServiceError for network errors', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); + + await expect(service.getSubscriptionsEligibilities()).rejects.toThrow( + SubscriptionServiceError, + ); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); }); describe('submitUserEvent', () => { @@ -760,6 +843,19 @@ describe('SubscriptionService', () => { ); }); }); + + it('should throw SubscriptionServiceError for network errors', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); + + await expect( + service.submitUserEvent({ + event: SubscriptionUserEvent.ShieldEntryModalViewed, + }), + ).rejects.toThrow(SubscriptionServiceError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); }); describe('assignUserToCohort', () => { @@ -829,6 +925,24 @@ describe('SubscriptionService', () => { ); }); }); + + it('should throw SubscriptionServiceError for network errors', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); + + await expect( + service.submitSponsorshipIntents({ + chainId: '0x1', + address: '0x1234567890123456789012345678901234567890', + products: [PRODUCT_TYPES.SHIELD], + recurringInterval: RECURRING_INTERVALS.month, + billingCycles: 12, + paymentTokenSymbol: 'USDT', + }), + ).rejects.toThrow(SubscriptionServiceError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); }); describe('linkRewards', () => { @@ -856,5 +970,39 @@ describe('SubscriptionService', () => { ); }); }); + + it('should throw SubscriptionServiceError for network errors', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockRejectedValue(new Error('Network error')); + + await expect( + service.linkRewards({ + rewardAccountId: + 'eip155:1:0x1234567890123456789012345678901234567890', + }), + ).rejects.toThrow(SubscriptionServiceError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); + + it('should throw SubscriptionServiceError for non-ok responses', async () => { + await withMockSubscriptionService(async ({ service, config }) => { + config.fetchMock.mockResolvedValue( + createMockResponse({ + ok: false, + status: 400, + jsonData: { message: 'Bad request' }, + }), + ); + + await expect( + service.linkRewards({ + rewardAccountId: + 'eip155:1:0x1234567890123456789012345678901234567890', + }), + ).rejects.toThrow(SubscriptionServiceError); + expect(config.captureExceptionMock).toHaveBeenCalledTimes(1); + }); + }); }); }); diff --git a/packages/subscription-controller/src/SubscriptionService.ts b/packages/subscription-controller/src/SubscriptionService.ts index ca1eaa833d7..915fdb2cb57 100644 --- a/packages/subscription-controller/src/SubscriptionService.ts +++ b/packages/subscription-controller/src/SubscriptionService.ts @@ -6,7 +6,7 @@ import { import type { Env } from './constants'; import { createSentryError, - getErrorFromResponse, + getSubscriptionErrorFromResponse, SubscriptionServiceError, } from './errors'; import { createModuleLogger, projectLogger } from './logger'; @@ -325,7 +325,7 @@ export class SubscriptionService implements ISubscriptionService { }); if (!response.ok) { - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); throw error; } diff --git a/packages/subscription-controller/src/errors.test.ts b/packages/subscription-controller/src/errors.test.ts index 93c0f63140a..caa5b9cb318 100644 --- a/packages/subscription-controller/src/errors.test.ts +++ b/packages/subscription-controller/src/errors.test.ts @@ -1,6 +1,7 @@ import { + composeSubscriptionApiErrorMessage, createSentryError, - getErrorFromResponse, + getSubscriptionErrorFromResponse, SubscriptionServiceError, } from './errors'; @@ -45,6 +46,14 @@ describe('errors', () => { expect(error.message).toBe('message'); expect(error.cause).toBe(cause); }); + + it('sets name and message without cause', () => { + const error = new SubscriptionServiceError('message'); + + expect(error.name).toBe('SubscriptionServiceError'); + expect(error.message).toBe('message'); + expect(error.cause).toBeUndefined(); + }); }); describe('createSentryError', () => { @@ -58,27 +67,41 @@ describe('errors', () => { }); describe('getErrorFromResponse', () => { - it('uses JSON error message when content-type is json', async () => { + it('uses JSON message when content-type is json', async () => { const response = createMockResponse({ contentType: 'application/json', status: 400, - jsonData: { error: 'Bad request' }, + jsonData: { message: 'Bad request' }, }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toContain('error: Bad request'); expect(error.message).toContain('statusCode: 400'); }); - it('uses JSON message when error field is missing', async () => { + it('includes errorCode in message when present in JSON response', async () => { + const response = createMockResponse({ + contentType: 'application/json', + status: 400, + jsonData: { message: 'Invalid input', errorCode: 'E400' }, + }); + + const error = await getSubscriptionErrorFromResponse(response); + + expect(error.message).toContain('error: Invalid input'); + expect(error.message).toContain('statusCode: 400'); + expect(error.message).toContain('errorCode: E400'); + }); + + it('uses JSON message field for error message', async () => { const response = createMockResponse({ contentType: 'application/json', status: 422, jsonData: { message: 'Unprocessable' }, }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toContain('error: Unprocessable'); expect(error.message).toContain('statusCode: 422'); @@ -91,7 +114,7 @@ describe('errors', () => { jsonData: { detail: 'teapot' }, }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toContain('error: Unknown error'); expect(error.message).toContain('statusCode: 418'); @@ -104,7 +127,7 @@ describe('errors', () => { textData: 'Service unavailable', }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toContain('error: Service unavailable'); expect(error.message).toContain('statusCode: 503'); @@ -117,7 +140,7 @@ describe('errors', () => { data: 'fallback data', }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toContain('error: fallback data'); expect(error.message).toContain('statusCode: 500'); @@ -130,7 +153,7 @@ describe('errors', () => { data: { code: 'UNKNOWN' } as unknown as string, }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toContain('error: Unknown error'); expect(error.message).toContain('statusCode: 500'); @@ -143,9 +166,57 @@ describe('errors', () => { jsonThrows: true, }); - const error = await getErrorFromResponse(response); + const error = await getSubscriptionErrorFromResponse(response); expect(error.message).toBe('HTTP 502 error'); }); }); + + describe('composeSubscriptionApiErrorMessage', () => { + it('composes message with message and statusCode', () => { + const result = composeSubscriptionApiErrorMessage( + { message: 'Not found' }, + 404, + ); + + expect(result).toBe('error: Not found, statusCode: 404'); + }); + + it('appends errorCode when present', () => { + const result = composeSubscriptionApiErrorMessage( + { message: 'Invalid', errorCode: 'INVALID_INPUT' }, + 400, + ); + + expect(result).toBe( + 'error: Invalid, statusCode: 400, errorCode: INVALID_INPUT', + ); + }); + + it('uses Unknown error when message is undefined', () => { + const result = composeSubscriptionApiErrorMessage({}, 500); + + expect(result).toBe('error: Unknown error, statusCode: 500'); + }); + + it('uses Unknown status code when statusCode is undefined', () => { + const result = composeSubscriptionApiErrorMessage( + { message: 'Error' }, + undefined as unknown as number, + ); + + expect(result).toBe('error: Error, statusCode: Unknown status code'); + }); + + it('handles both message and statusCode being undefined', () => { + const result = composeSubscriptionApiErrorMessage( + {}, + undefined as unknown as number, + ); + + expect(result).toBe( + 'error: Unknown error, statusCode: Unknown status code', + ); + }); + }); }); diff --git a/packages/subscription-controller/src/errors.ts b/packages/subscription-controller/src/errors.ts index cab13c85d12..29a940cfc87 100644 --- a/packages/subscription-controller/src/errors.ts +++ b/packages/subscription-controller/src/errors.ts @@ -1,3 +1,5 @@ +import { SubscriptionApiError } from './types'; + export class SubscriptionServiceError extends Error { /** * The underlying error that caused this error. @@ -22,15 +24,19 @@ export class SubscriptionServiceError extends Error { * @param response - The response to get an error from. * @returns An error. */ -export async function getErrorFromResponse(response: Response): Promise { +export async function getSubscriptionErrorFromResponse( + response: Response, +): Promise { const contentType = response.headers?.get('content-type'); const statusCode = response.status; try { if (contentType?.includes('application/json')) { - const json = await response.json(); - const errorMessage = json?.error ?? json?.message ?? 'Unknown error'; - const networkError = `error: ${errorMessage}, statusCode: ${statusCode}`; - return new Error(networkError); + const json = (await response.json()) as SubscriptionApiError; + const subscriptionApiErrorMessage = composeSubscriptionApiErrorMessage( + json, + statusCode, + ); + return new Error(subscriptionApiErrorMessage); } else if (contentType?.includes('text/plain')) { const text = await response.text(); const networkError = `error: ${text}, statusCode: ${statusCode}`; @@ -63,3 +69,21 @@ export function createSentryError(message: string, cause: Error): Error { return sentryError; } + +/** + * Compose an error message from a Subscription API error. + * + * @param error - The Subscription API error to compose an error message from. + * @returns An error message. + * @param statusCode - The status code of the response. + */ +export function composeSubscriptionApiErrorMessage( + error: SubscriptionApiError, + statusCode: number, +): string { + let baseErrorMessage = `error: ${error.message ?? 'Unknown error'}, statusCode: ${statusCode ?? 'Unknown status code'}`; + if (error.errorCode) { + baseErrorMessage += `, errorCode: ${error.errorCode}`; + } + return baseErrorMessage; +} diff --git a/packages/subscription-controller/src/index.ts b/packages/subscription-controller/src/index.ts index 01e76893efc..faa87dbca01 100644 --- a/packages/subscription-controller/src/index.ts +++ b/packages/subscription-controller/src/index.ts @@ -28,6 +28,7 @@ export { getDefaultSubscriptionControllerState, } from './SubscriptionController'; export type { + SubscriptionApiError, Subscription, AuthUtils, CancelSubscriptionRequest, diff --git a/packages/subscription-controller/src/types.ts b/packages/subscription-controller/src/types.ts index e765e434ee1..b7504b2b128 100644 --- a/packages/subscription-controller/src/types.ts +++ b/packages/subscription-controller/src/types.ts @@ -1,5 +1,14 @@ import type { CaipAccountId, Hex } from '@metamask/utils'; +/** + * Error response from the Subscription API. + */ +export type SubscriptionApiError = { + errorCode?: string; + message?: string; + statusCode?: number; +}; + export const PRODUCT_TYPES = { SHIELD: 'shield', } as const; From f06b61ba7a490648845c183efdbe1ebc06a66cf2 Mon Sep 17 00:00:00 2001 From: lwin Date: Tue, 10 Feb 2026 17:12:24 +0800 Subject: [PATCH 12/12] chore: handle error in 'getSubscriptionErrorFromResponse' function --- .../src/errors.test.ts | 26 ++++++++++++++++--- .../subscription-controller/src/errors.ts | 5 ++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/subscription-controller/src/errors.test.ts b/packages/subscription-controller/src/errors.test.ts index caa5b9cb318..7b805568ae7 100644 --- a/packages/subscription-controller/src/errors.test.ts +++ b/packages/subscription-controller/src/errors.test.ts @@ -12,6 +12,7 @@ type MockResponseOptions = { textData?: string; jsonThrows?: boolean; data?: string; + jsonThrowsWithString?: boolean; }; function createMockResponse({ @@ -20,17 +21,22 @@ function createMockResponse({ jsonData = {}, textData = 'plain error', jsonThrows = false, + jsonThrowsWithString = false, data, }: MockResponseOptions): Response { + let json = jest.fn().mockResolvedValue(jsonData); + if (jsonThrows) { + json = jest.fn().mockRejectedValue(new Error('bad json')); + } else if (jsonThrowsWithString) { + json = jest.fn().mockRejectedValue('string error'); + } return { status, headers: { get: (key: string) => key.toLowerCase() === 'content-type' ? contentType : null, }, - json: jsonThrows - ? jest.fn().mockRejectedValue(new Error('bad json')) - : jest.fn().mockResolvedValue(jsonData), + json, text: jest.fn().mockResolvedValue(textData), data, } as unknown as Response; @@ -168,7 +174,19 @@ describe('errors', () => { const error = await getSubscriptionErrorFromResponse(response); - expect(error.message).toBe('HTTP 502 error'); + expect(error.message).toBe('HTTP 502 error: bad json'); + }); + + it('returns generic HTTP error when parsing fails for string error', async () => { + const response = createMockResponse({ + contentType: 'application/json', + status: 502, + jsonThrowsWithString: true, + }); + + const error = await getSubscriptionErrorFromResponse(response); + + expect(error.message).toBe('HTTP 502 error: Unknown error'); }); }); diff --git a/packages/subscription-controller/src/errors.ts b/packages/subscription-controller/src/errors.ts index 29a940cfc87..cb2e29c0f1f 100644 --- a/packages/subscription-controller/src/errors.ts +++ b/packages/subscription-controller/src/errors.ts @@ -49,8 +49,9 @@ export async function getSubscriptionErrorFromResponse( : 'Unknown error'; const networkError = `error: ${error}, statusCode: ${statusCode}`; return new Error(networkError); - } catch { - return new Error(`HTTP ${statusCode} error`); + } catch (error) { + const errMessage = error instanceof Error ? error.message : 'Unknown error'; + return new Error(`HTTP ${statusCode} error: ${errMessage}`); } }