From 53ac5730a16a7dec58aaae61c209ce45a141d1df Mon Sep 17 00:00:00 2001 From: Jiexi Luan-Huang Date: Thu, 17 Sep 2026 15:18:15 -0700 Subject: [PATCH] Throw instead of silent fail --- packages/kyc-controller/CHANGELOG.md | 3 + .../kyc-controller/src/KycController.test.ts | 126 +++++++++++------- packages/kyc-controller/src/KycController.ts | 24 ++-- 3 files changed, 89 insertions(+), 64 deletions(-) diff --git a/packages/kyc-controller/CHANGELOG.md b/packages/kyc-controller/CHANGELOG.md index af3ce152015..4258b928696 100644 --- a/packages/kyc-controller/CHANGELOG.md +++ b/packages/kyc-controller/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** `KycController` methods that previously recorded a failure only on state (`phase: 'error'`) now also throw after recording it. + - Affects `initialize` (vendor customer creation), `createVendorCustomer`, `acceptTermsAndStartSession` (missing T&C2 / email / terms), `checkKycRequired`, and the MoonPay frame `fail` callback (`handleFrameMessage`). + - A product-scoped MoonPay auto-run therefore rejects `handleFrameMessage` / `onAuthenticated` when the KYC-required check fails. - Bump `@metamask/profile-sync-controller` from `^32.1.0` to `^32.1.1` ([#10220](https://github.com/MetaMask/core/pull/10220)) ## [0.3.0] diff --git a/packages/kyc-controller/src/KycController.test.ts b/packages/kyc-controller/src/KycController.test.ts index e67e970bcf3..d83582ddf40 100644 --- a/packages/kyc-controller/src/KycController.test.ts +++ b/packages/kyc-controller/src/KycController.test.ts @@ -659,7 +659,9 @@ describe('KycController', () => { }, }, async ({ controller, handlers }) => { - await controller.acceptTermsAndStartSession(); + await expect( + controller.acceptTermsAndStartSession(), + ).rejects.toThrow(/Missing T&C2 acceptance/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch(/Missing T&C2 acceptance/u); @@ -885,10 +887,12 @@ describe('KycController', () => { }, }, async ({ controller }) => { - await controller.acceptTermsAndStartSession({ - providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, - idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, - }); + await expect( + controller.acceptTermsAndStartSession({ + providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, + idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, + }), + ).rejects.toThrow(/Missing email/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch(/Missing email/u); @@ -898,11 +902,13 @@ describe('KycController', () => { it('fails when no disclaimers were accepted', async () => { await withController(async ({ controller }) => { - await controller.acceptTermsAndStartSession({ - email: 'a@b.co', - providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, - idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, - }); + await expect( + controller.acceptTermsAndStartSession({ + email: 'a@b.co', + providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, + idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, + }), + ).rejects.toThrow(/Missing terms acceptance/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch(/Missing terms acceptance/u); @@ -1162,7 +1168,9 @@ describe('KycController', () => { async ({ controller, handlers, launcher, moonPayFrames }) => { handlers.checkKycRequired.mockRejectedValue(new Error('down')); - await moonPayFrames.options.onAuthenticated(); + await expect( + moonPayFrames.options.onAuthenticated(), + ).rejects.toThrow(/KYC check failed/u); expect(controller.state.phase).toBe('error'); expect(launcher.launch).not.toHaveBeenCalled(); @@ -1220,7 +1228,9 @@ describe('KycController', () => { it('records an error when the handler reports a failure', async () => { await withController(({ controller, moonPayFrames }) => { - moonPayFrames.options.fail('Check frame returned status: failed'); + expect(() => + moonPayFrames.options.fail('Check frame returned status: failed'), + ).toThrow('Check frame returned status: failed'); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toBe( @@ -1233,9 +1243,9 @@ describe('KycController', () => { describe('checkKycRequired', () => { it('fails without an access token', async () => { await withController(async ({ controller }) => { - expect(await controller.checkKycRequired({ product: 'ramps' })).toBe( - false, - ); + await expect( + controller.checkKycRequired({ product: 'ramps' }), + ).rejects.toThrow(/Missing moonpayAccessToken/u); expect(controller.state.error).toMatch(/Missing moonpayAccessToken/u); }); }); @@ -1244,9 +1254,9 @@ describe('KycController', () => { await withController( { options: { state: { moonpayAccessToken: 'a' } } }, async ({ controller }) => { - expect(await controller.checkKycRequired({ product: 'ramps' })).toBe( - false, - ); + await expect( + controller.checkKycRequired({ product: 'ramps' }), + ).rejects.toThrow(/Missing country/u); expect(controller.state.error).toMatch(/Missing country/u); }, ); @@ -1293,9 +1303,9 @@ describe('KycController', () => { async ({ controller, handlers }) => { handlers.checkKycRequired.mockRejectedValue(new Error('down')); - expect(await controller.checkKycRequired({ product: 'ramps' })).toBe( - false, - ); + await expect( + controller.checkKycRequired({ product: 'ramps' }), + ).rejects.toThrow(/KYC check failed/u); expect(controller.state.error).toMatch(/KYC check failed/u); }, ); @@ -2699,7 +2709,9 @@ describe('KycController', () => { await withController(async ({ controller, handlers }) => { handlers.createVendorCustomer.mockRejectedValue(new Error('iron down')); - await controller.initialize({ email: 'a@b.co', vendor: 'iron' }); + await expect( + controller.initialize({ email: 'a@b.co', vendor: 'iron' }), + ).rejects.toThrow(/Vendor customer creation failed/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch( @@ -2722,7 +2734,9 @@ describe('KycController', () => { new Error('iron down'), ); - await controller.initialize({ email: 'a@b.co', vendor: 'iron' }); + await expect( + controller.initialize({ email: 'a@b.co', vendor: 'iron' }), + ).rejects.toThrow(/Vendor customer creation failed/u); expect(controller.state.phase).toBe('error'); expect( @@ -3041,10 +3055,12 @@ describe('KycController', () => { await withController(async ({ controller, handlers }) => { handlers.createVendorCustomer.mockRejectedValue(new Error('nope')); - await controller.createVendorCustomer({ - vendor: 'iron', - email: 'a@b.co', - }); + await expect( + controller.createVendorCustomer({ + vendor: 'iron', + email: 'a@b.co', + }), + ).rejects.toThrow(/Vendor customer creation failed/u); expect(controller.state.activeVendor).toBe('iron'); expect(controller.state.email).toBe('a@b.co'); @@ -3064,10 +3080,12 @@ describe('KycController', () => { async ({ controller, handlers }) => { handlers.createVendorCustomer.mockRejectedValue(new Error('nope')); - await controller.createVendorCustomer({ - vendor: 'iron', - email: 'a@b.co', - }); + await expect( + controller.createVendorCustomer({ + vendor: 'iron', + email: 'a@b.co', + }), + ).rejects.toThrow(/Vendor customer creation failed/u); expect(controller.state.phase).toBe('error'); expect( @@ -3568,11 +3586,13 @@ describe('KycController', () => { }, }, async ({ controller }) => { - // @ts-expect-error T&C2 flags are required - await controller.acceptTermsAndStartSession({ - email: 'a@b.co', - product: 'money', - }); + await expect( + // @ts-expect-error T&C2 flags are required + controller.acceptTermsAndStartSession({ + email: 'a@b.co', + product: 'money', + }), + ).rejects.toThrow(/Missing T&C2 acceptance/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch(/Missing T&C2 acceptance/u); @@ -3594,11 +3614,13 @@ describe('KycController', () => { }, }, async ({ controller, handlers }) => { - // @ts-expect-error both T&C2 flags are required - await controller.acceptTermsAndStartSession({ - email: 'a@b.co', - providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, - }); + await expect( + // @ts-expect-error both T&C2 flags are required + controller.acceptTermsAndStartSession({ + email: 'a@b.co', + providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, + }), + ).rejects.toThrow(/Missing T&C2 acceptance/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch(/Missing T&C2 acceptance/u); @@ -3657,10 +3679,12 @@ describe('KycController', () => { }, }, async ({ controller }) => { - await controller.acceptTermsAndStartSession({ - providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, - idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, - }); + await expect( + controller.acceptTermsAndStartSession({ + providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, + idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, + }), + ).rejects.toThrow(/Missing email/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch(/Missing email/u); @@ -3680,11 +3704,13 @@ describe('KycController', () => { }, }, async ({ controller }) => { - await controller.acceptTermsAndStartSession({ - email: 'a@b.co', - providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, - idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, - }); + await expect( + controller.acceptTermsAndStartSession({ + email: 'a@b.co', + providerDisclaimersAccepted: MOCK_SUMSUB_DISCLAIMERS_ACCEPTED, + idosDisclaimersAccepted: MOCK_IDOS_DISCLAIMERS_ACCEPTED, + }), + ).rejects.toThrow(/Missing disclaimer acceptance/u); expect(controller.state.phase).toBe('error'); expect(controller.state.error).toMatch( diff --git a/packages/kyc-controller/src/KycController.ts b/packages/kyc-controller/src/KycController.ts index 1553a8e41be..aac4bf2b5b8 100644 --- a/packages/kyc-controller/src/KycController.ts +++ b/packages/kyc-controller/src/KycController.ts @@ -950,7 +950,6 @@ export class KycController extends BaseController< return; } this.#fail(`Vendor customer creation failed: ${String(error)}`); - return; } } @@ -1182,7 +1181,6 @@ export class KycController extends BaseController< !isValidConsentRecordList(idosDisclaimersAccepted) ) { this.#fail('Missing T&C2 acceptance flags.'); - return; } const credentialReusabilityConsentGiven = params?.credentialReusabilityConsentGiven ?? false; @@ -1244,11 +1242,9 @@ export class KycController extends BaseController< ); if (!email) { this.#fail('Missing email for consents session.'); - return; } if (acceptedDisclaimerIds.length === 0) { this.#fail('Missing disclaimer acceptance.'); - return; } const generation = this.#generation; @@ -1542,11 +1538,9 @@ export class KycController extends BaseController< ); if (!email) { this.#fail('Missing email for session creation.'); - return; } if (!termsAcceptedAt || acceptedDisclaimerIds.length === 0) { this.#fail('Missing terms acceptance for session creation.'); - return; } // A new session invalidates any authentication carried over from a prior @@ -1666,9 +1660,11 @@ export class KycController extends BaseController< * document-verification sub-flow is launched. When no product is set, this is * a no-op and the flow stays at `form` for the consumer to drive manually. * - * Errors are already recorded on state by `checkKycRequired` (`error` - * phase) and `startSumSub` (`sumsub.status = 'failed'`); this method swallows - * them so it can be awaited safely from the frame-message handler. + * `checkKycRequired` records the error phase and rethrows, so this method + * (and therefore `handleFrameMessage`) rejects when the check fails. + * `startSumSub` records `sumsub.status = 'failed'`; this method swallows + * that rethrown error (e.g. SDK unavailable) so it does not surface as an + * unhandled rejection from the frame-message handler. */ async #continueAfterAuthentication(): Promise { const product = this.state.activeProduct; @@ -1745,6 +1741,8 @@ export class KycController extends BaseController< * @param params.product - The consuming feature. * @param params.country - Optional alpha-3 country override. * @returns Whether KYC is required. + * @throws If the access token or country is missing, or the service call + * fails. The error is also recorded on controller state (`phase: 'error'`). */ async checkKycRequired(params: { product: KycProduct; @@ -1755,12 +1753,10 @@ export class KycController extends BaseController< this.#fail( 'Missing moonpayAccessToken — repeat the authentication step.', ); - return false; } const country = params.country ?? this.state.geoCountry; if (!country) { this.#fail('Missing country for KYC-required check.'); - return false; } // Capture the flow generation so we can detect a `reset()` that happens @@ -1798,7 +1794,6 @@ export class KycController extends BaseController< return false; } this.#fail(`KYC check failed: ${String(error)}`); - return false; } } @@ -2609,14 +2604,15 @@ export class KycController extends BaseController< } /** - * Transitions to the error phase with a message. + * Transitions to the error phase with a message, then throws. * * @param message - The error message. */ - #fail(message: string): void { + #fail(message: string): never { this.#applyUpdate((state) => { state.error = message; state.phase = 'error'; }); + throw new Error(message); } }