From c810417addd7b0a7450b7161eb8905b098caefaa Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Sun, 23 Aug 2026 10:19:41 -0300 Subject: [PATCH 1/4] fix(kyc): scope the KYC route by URL and report unmapped steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carry the KYC context in the route URL instead of go_router `extra`. The post-lock restore re-pushes the captured location by bare path and has no `extra` to give, so the flow rebuilt unscoped after a background lock: the API then computed `processStatus` over every globally required step instead of only those the entered flow needs, reporting a user who had finished Ident as still in progress for steps that do not gate buying. Report every route into the unsupported-step handoff as a non-fatal event. The handoff page already gave the user something to do, but the gap itself stayed invisible — every call in the flow returns 200, so a step name missing from the client mapping table only surfaced through a user report. The event carries the step's wire identifier and nothing else, through the existing DSN-gated crash-reporting channel. --- .../unsupported_kyc_step_exception.dart | 23 +++++ .../buy/widgets/payment_action_button.dart | 15 ++- lib/screens/kyc/cubits/kyc/kyc_cubit.dart | 41 ++++++-- lib/screens/sell/widgets/sell_button.dart | 10 +- lib/setup/error_handling/crash_reporting.dart | 24 +++++ lib/setup/routing/router_config.dart | 3 +- lib/setup/routing/routes/app_routes.dart | 17 ++++ .../exceptions/exception_surface_test.dart | 3 + .../unsupported_kyc_step_exception_test.dart | 25 +++++ .../widgets/payment_action_button_test.dart | 8 +- .../kyc/cubits/kyc/kyc_cubit_test.dart | 96 ++++++++++++++++++- .../error_handling/crash_reporting_test.dart | 10 ++ .../routing/boot_navigation_apply_test.dart | 36 ++++++- test/setup/routing/kyc_route_query_test.dart | 18 ++++ 14 files changed, 304 insertions(+), 25 deletions(-) create mode 100644 lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart create mode 100644 test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart create mode 100644 test/setup/routing/kyc_route_query_test.dart diff --git a/lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart b/lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart new file mode 100644 index 000000000..a04ee021b --- /dev/null +++ b/lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart @@ -0,0 +1,23 @@ +import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; + +/// Reported — never thrown — when the API routes an account to a KYC step this +/// app has no screen for, or reports a pending review whose step list holds no +/// required step we can name. +/// +/// The app maps a subset of [KycStepName] to screens; anything outside it lands +/// on the generic handoff page. Nothing else marks that: every request in the +/// flow returns 200, so without this event the only signal that the mapping +/// table has a gap is a user complaint. +/// +/// [stepName] is the API's wire identifier for the step, or null when the API +/// named no step at all. It carries nothing about the user. +class UnsupportedKycStepException implements Exception { + final KycStepName? stepName; + + const UnsupportedKycStepException(this.stepName); + + @override + String toString() => + 'UnsupportedKycStepException: no screen for KYC step ' + '${stepName?.value ?? ''}'; +} diff --git a/lib/screens/buy/widgets/payment_action_button.dart b/lib/screens/buy/widgets/payment_action_button.dart index 972ed141b..23bfda468 100644 --- a/lib/screens/buy/widgets/payment_action_button.dart +++ b/lib/screens/buy/widgets/payment_action_button.dart @@ -66,7 +66,10 @@ class PaymentActionButton extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 20), child: AppFilledButton( onPressed: () async { - await context.pushNamed(AppRoutes.kyc, extra: paymentState.context); + await context.pushNamed( + AppRoutes.kyc, + queryParameters: kycRouteQuery(paymentState.context), + ); if (context.mounted) { context.read().getPaymentInfo( amount: amountController.text, @@ -83,7 +86,10 @@ class PaymentActionButton extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 20), child: AppFilledButton( onPressed: () async { - await context.pushNamed(AppRoutes.kyc, extra: paymentState.context); + await context.pushNamed( + AppRoutes.kyc, + queryParameters: kycRouteQuery(paymentState.context), + ); if (context.mounted) { context.read().getPaymentInfo( amount: amountController.text, @@ -128,7 +134,10 @@ class PaymentActionButton extends StatelessWidget { padding: const EdgeInsets.symmetric(vertical: 20), child: AppFilledButton( onPressed: () async { - await context.pushNamed(AppRoutes.kyc, extra: paymentState.context); + await context.pushNamed( + AppRoutes.kyc, + queryParameters: kycRouteQuery(paymentState.context), + ); if (context.mounted) { context.read().getPaymentInfo( amount: amountController.text, diff --git a/lib/screens/kyc/cubits/kyc/kyc_cubit.dart b/lib/screens/kyc/cubits/kyc/kyc_cubit.dart index d4c34f658..2fc9e4b45 100644 --- a/lib/screens/kyc/cubits/kyc/kyc_cubit.dart +++ b/lib/screens/kyc/cubits/kyc/kyc_cubit.dart @@ -6,6 +6,7 @@ import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:realunit_wallet/packages/service/app_store.dart'; import 'package:realunit_wallet/packages/service/dfx/dfx_kyc_service.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/api_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_level_dto.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; import 'package:realunit_wallet/packages/service/dfx/models/legal/real_unit_legal_agreement.dart'; @@ -15,6 +16,7 @@ import 'package:realunit_wallet/packages/service/dfx/models/wallet/real_unit_reg import 'package:realunit_wallet/packages/service/dfx/real_unit_legal_service.dart'; import 'package:realunit_wallet/packages/service/dfx/real_unit_registration_service.dart'; import 'package:realunit_wallet/packages/wallet/wallet.dart'; +import 'package:realunit_wallet/setup/error_handling/crash_reporting.dart'; part 'kyc_state.dart'; @@ -26,6 +28,10 @@ class KycCubit extends Cubit { final RealUnitLegalService _legalService; final AppStore _appStore; + /// Sink for the unmapped-step report — see [_emitUnsupportedStep]. Injectable + /// so tests can observe the report without the crash reporter running. + final NonFatalReporter _report; + /// Offline fallback ONLY. The legal disclaimer gate is server-driven via /// `_legalService.getLegalInfo()`; this per-session flag is used solely when /// that endpoint is unreachable (pre-rollout backend or outage) so the @@ -53,12 +59,14 @@ class KycCubit extends Cubit { DfxKycService kycService, RealUnitRegistrationService registrationService, RealUnitLegalService legalService, - AppStore appStore, - ) : _kycService = kycService, - _registrationService = registrationService, - _legalService = legalService, - _appStore = appStore, - super(const KycInitial()); + AppStore appStore, { + NonFatalReporter report = reportNonFatal, + }) : _kycService = kycService, + _registrationService = registrationService, + _legalService = legalService, + _appStore = appStore, + _report = report, + super(const KycInitial()); Future checkKyc({String? context}) async { _kycContext = context ?? _kycContext; @@ -245,12 +253,12 @@ class KycCubit extends Cubit { (s) => s.isRequired && s.status != KycStepStatus.completed, ); if (pending == null) { - emit(const KycUnsupportedStepFailure(null)); + _emitUnsupportedStep(null); return; } final step = _mapStepName(pending.name); if (step == null) { - emit(KycUnsupportedStepFailure(pending.name)); + _emitUnsupportedStep(pending.name); return; } emit(KycPending(step)); @@ -324,13 +332,13 @@ class KycCubit extends Cubit { // in the i18n message). final currentStep = kycStatus.currentStep; if (currentStep == null) { - emit(const KycUnsupportedStepFailure(null)); + _emitUnsupportedStep(null); return; } final kycStep = _mapStepName(currentStep.name); if (kycStep == null) { - emit(KycUnsupportedStepFailure(currentStep.name)); + _emitUnsupportedStep(currentStep.name); return; } @@ -343,6 +351,19 @@ class KycCubit extends Cubit { ); } + /// Routes an unmapped step to the generic handoff page and reports the + /// occurrence. + /// + /// The report is the only trace this leaves: every call in the flow returned + /// 200 and the user sees a handoff screen, so a step name missing from + /// [_mapStepName] is otherwise invisible until someone writes in. Reporting + /// before the emit keeps the event even when the user leaves the flow on this + /// screen. + void _emitUnsupportedStep(KycStepName? stepName) { + _report(UnsupportedKycStepException(stepName)); + emit(KycUnsupportedStepFailure(stepName)); + } + KycStep? _mapStepName(KycStepName name) => switch (name) { KycStepName.contactData => KycStep.registration, KycStepName.personalData => KycStep.personalData, diff --git a/lib/screens/sell/widgets/sell_button.dart b/lib/screens/sell/widgets/sell_button.dart index 81dcdca74..cd77e3a0a 100644 --- a/lib/screens/sell/widgets/sell_button.dart +++ b/lib/screens/sell/widgets/sell_button.dart @@ -24,11 +24,17 @@ class SellButton extends StatelessWidget { listener: (context, state) async { if (state is SellPaymentInfoFailure) { if (state.error == .kycRequired) { - await context.pushNamed(AppRoutes.kyc, extra: state.context); + await context.pushNamed( + AppRoutes.kyc, + queryParameters: kycRouteQuery(state.context), + ); return; } if (state.error == .registrationRequired) { - await context.pushNamed(AppRoutes.kyc, extra: state.context); + await context.pushNamed( + AppRoutes.kyc, + queryParameters: kycRouteQuery(state.context), + ); return; } if (state.error == .bitboxDisconnected) { diff --git a/lib/setup/error_handling/crash_reporting.dart b/lib/setup/error_handling/crash_reporting.dart index b9aa5bac2..af5f8a268 100644 --- a/lib/setup/error_handling/crash_reporting.dart +++ b/lib/setup/error_handling/crash_reporting.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:developer' as developer; import 'package:flutter/foundation.dart'; @@ -56,6 +57,29 @@ Future initCrashReporting({ } } +/// Sink for a condition the app caught and handled but that should not have +/// happened. Matches [reportNonFatal] so a caller can hold the seam as a field +/// and a test can record instead of report. +typedef NonFatalReporter = void Function(Object error); + +/// Records [error] as a non-fatal event: a `developer.log` line for an attached +/// developer plus, when the crash reporter is running, an error event carrying +/// the same object. +/// +/// Same channel and same pinned option surface as the uncaught-error path — no +/// PII, no attachments, no breadcrumb widening — so callers must pass an error +/// object that describes the condition in its own `toString()` and nothing +/// about the user. +/// +/// Gated on the same [crashReportingDsn] that decides whether [initCrashReporting] +/// starts the SDK at all: without an injected DSN — every local and test build — +/// nothing was ever started, and the report is a pure log line. +void reportNonFatal(Object error) { + developer.log('non-fatal: $error', name: 'WalletApp', error: error); + if (crashReportingDsn.isEmpty) return; + unawaited(Sentry.captureException(error)); +} + /// Applies the pinned option set. The guarantee is exactly this list — an /// upstream default flip outside it is not caught here: no PII, no /// screenshots, no performance tracing, no session telemetry. Native crash diff --git a/lib/setup/routing/router_config.dart b/lib/setup/routing/router_config.dart index 72c9d050f..41acb90cb 100644 --- a/lib/setup/routing/router_config.dart +++ b/lib/setup/routing/router_config.dart @@ -222,7 +222,8 @@ final GoRouter routerConfig = GoRouter( GoRoute( name: AppRoutes.kyc, path: '/kyc', - builder: (_, state) => KycPageManager(kycContext: state.extra as String?), + builder: (_, state) => + KycPageManager(kycContext: state.uri.queryParameters['context']), ), GoRoute( diff --git a/lib/setup/routing/routes/app_routes.dart b/lib/setup/routing/routes/app_routes.dart index d0ebf58ff..9beffaafb 100644 --- a/lib/setup/routing/routes/app_routes.dart +++ b/lib/setup/routing/routes/app_routes.dart @@ -14,3 +14,20 @@ abstract final class AppRoutes { static const webView = 'webView'; } + +/// Query parameters that scope the [AppRoutes.kyc] flow to the context the user +/// entered it from (`RealunitBuy`, `RealunitSell`, …), as the API reported it. +/// +/// The context travels in the URL rather than in `extra` because the flow has a +/// second entry: after a background lock the boot ladder re-pushes the captured +/// location by bare path (`BootNavRestore` in `boot_navigation.dart`), which +/// carries no `extra`. Without the query the restored flow rebuilds unscoped, +/// and the API then computes `processStatus` over every globally required step +/// instead of only those the entered flow needs — reporting a user who has +/// finished `Ident` as still in progress for steps that do not gate buying. +/// +/// A null [kycContext] means the API attached none; the route is then entered +/// unscoped exactly as before, never with an invented context. +Map kycRouteQuery(String? kycContext) => { + if (kycContext != null) 'context': kycContext, +}; diff --git a/test/packages/service/dfx/exceptions/exception_surface_test.dart b/test/packages/service/dfx/exceptions/exception_surface_test.dart index 55387f3cb..b39e55835 100644 --- a/test/packages/service/dfx/exceptions/exception_surface_test.dart +++ b/test/packages/service/dfx/exceptions/exception_surface_test.dart @@ -7,6 +7,8 @@ import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/pay_exce import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/sell_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/transfer_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/registration_rejected_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; import 'package:realunit_wallet/packages/storage/secure_storage.dart'; import 'package:realunit_wallet/packages/wallet/exceptions/signing_cancelled_exception.dart'; @@ -42,6 +44,7 @@ void main() { const TransferGasFundingUnavailableException(), const TransferConfirmMismatchException(), const TransferAlreadyConfirmedException(code: 'TEST', message: 'test'), + const UnsupportedKycStepException(KycStepName.statutes), ]; for (final ex in exceptions) { diff --git a/test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart b/test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart new file mode 100644 index 000000000..15cd29e6b --- /dev/null +++ b/test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart @@ -0,0 +1,25 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; + +// The rendered string is the whole payload of the report — the crash reporter +// receives nothing else about the occurrence. If it does not name the step, the +// event cannot say which entry the client's mapping table is missing. +void main() { + group('$UnsupportedKycStepException', () { + test('names the step by its API wire identifier', () { + const exception = UnsupportedKycStepException(KycStepName.residencePermit); + + expect(exception.stepName, KycStepName.residencePermit); + expect(exception.toString(), contains(KycStepName.residencePermit.value)); + }); + + test('renders readably when the API named no step at all', () { + const exception = UnsupportedKycStepException(null); + + expect(exception.stepName, isNull); + expect(exception.toString(), isNot(contains('null'))); + expect(exception.toString(), isNotEmpty); + }); + }); +} diff --git a/test/screens/buy/widgets/payment_action_button_test.dart b/test/screens/buy/widgets/payment_action_button_test.dart index a05cdc96a..3d660bb7b 100644 --- a/test/screens/buy/widgets/payment_action_button_test.dart +++ b/test/screens/buy/widgets/payment_action_button_test.dart @@ -31,7 +31,7 @@ void main() { // value (mirrors the registration / KYC gates), so the re-fetch is // asserted on both paths. bool? emailCaptureResult; - String? kycExtra; + String? kycContext; setUpAll(() { registerFallbackValue(Currency.chf); @@ -43,7 +43,7 @@ void main() { amountController = TextEditingController(text: '250'); pushedRoutes = []; emailCaptureResult = true; - kycExtra = null; + kycContext = null; when(() => converterCubit.state) .thenReturn(const BuyConverterState(currency: Currency.eur)); @@ -92,7 +92,7 @@ void main() { path: '/kyc', builder: (_, state) { pushedRoutes.add(AppRoutes.kyc); - kycExtra = state.extra as String?; + kycContext = state.uri.queryParameters['context']; return _EmailCaptureStub( onReady: (popContext) { WidgetsBinding.instance.addPostFrameCallback((_) { @@ -199,7 +199,7 @@ void main() { // Routed to KYC confirm-email, not to email capture. expect(pushedRoutes, [AppRoutes.kyc]); - expect(kycExtra, 'RealunitBuy'); + expect(kycContext, 'RealunitBuy'); // After the KYC flow returns, the quote is re-fetched with the // current amount + currency so a now-confirmed email surfaces the CTA. verify( diff --git a/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart b/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart index 804e99e6f..c5922f0e8 100644 --- a/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart +++ b/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart @@ -7,6 +7,7 @@ import 'package:mocktail/mocktail.dart'; import 'package:realunit_wallet/packages/service/app_store.dart'; import 'package:realunit_wallet/packages/service/dfx/dfx_kyc_service.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/api_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_level_dto.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_session_dto.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_step_dto.dart'; @@ -150,6 +151,9 @@ void main() { late RealUnitLegalService legalService; late AppStore appStore; late AWallet wallet; + // Everything the cubit hands to the non-fatal reporter. Injected in + // `buildCubit` so no test reaches the real crash-reporting sink. + late List reported; setUpAll(() { registerFallbackValue([]); @@ -161,6 +165,7 @@ void main() { legalService = _MockRealUnitLegalService(); appStore = _MockAppStore(); wallet = _MockAWallet(); + reported = []; when(() => appStore.wallet).thenReturn(wallet); // Default: software wallet — most tests don't care about the signing // capability gate. @@ -176,7 +181,13 @@ void main() { when(() => legalService.getLegalInfo()).thenAnswer((_) async => _legalInfo()); }); - KycCubit buildCubit() => KycCubit(kycService, registrationService, legalService, appStore); + KycCubit buildCubit() => KycCubit( + kycService, + registrationService, + legalService, + appStore, + report: reported.add, + ); group('$KycCubit checkKyc', () { blocTest( @@ -1180,7 +1191,7 @@ void main() { // Server reports all agreements accepted (default stub), so the // disclaimer gate passes and both runs reach the completed state. - final cubit = KycCubit(kycService, registrationService, legalService, appStore); + final cubit = buildCubit(); final states = []; final sub = cubit.stream.listen(states.add); @@ -1316,6 +1327,87 @@ void main() { ); }); + // The handoff screen alone leaves the gap silent: every call in the flow + // returned 200, so nothing tells us which step name `_mapStepName` is missing + // until a user writes in. Each route into the handoff must therefore also + // report the step it could not render. + group('$KycCubit unsupported-step reporting', () { + blocTest( + 'reports the step name when the continued session asks for an unmapped step', + setUp: () { + when(() => kycService.getKycStatus()).thenAnswer( + (_) async => _kycStatus(level: KycLevel.level30), + ); + when(() => kycService.getUser()).thenAnswer((_) async => _user()); + when(() => kycService.continueKyc()).thenAnswer( + (_) async => _session( + level: KycLevel.level30, + steps: const [], + currentStep: _currentStep(KycStepName.residencePermit), + ), + ); + }, + build: buildCubit, + act: (cubit) => cubit.checkKyc(), + expect: () => [ + const KycLoading(), + const KycUnsupportedStepFailure(KycStepName.residencePermit), + ], + verify: (_) { + expect(reported, hasLength(1)); + final error = reported.single as UnsupportedKycStepException; + expect(error.stepName, KycStepName.residencePermit); + // The wire identifier has to be in the rendered event, otherwise the + // report cannot say which mapping entry is missing. + expect(error.toString(), contains(KycStepName.residencePermit.value)); + }, + ); + + blocTest( + 'reports a null step when PendingReview names no required step', + setUp: () { + when(() => kycService.getKycStatus()).thenAnswer( + (_) async => _kycStatus( + level: KycLevel.level50, + processStatus: KycProcessStatus.pendingReview, + steps: [_step(KycStepName.ident, status: KycStepStatus.completed)], + ), + ); + when(() => kycService.getUser()).thenAnswer((_) async => _user()); + }, + build: buildCubit, + act: (cubit) => cubit.checkKyc(), + expect: () => [ + const KycLoading(), + const KycUnsupportedStepFailure(null), + ], + verify: (_) { + expect(reported, hasLength(1)); + expect((reported.single as UnsupportedKycStepException).stepName, isNull); + }, + ); + + blocTest( + 'reports nothing when every step the API asks for is mapped', + setUp: () { + when(() => kycService.getKycStatus()).thenAnswer( + (_) async => _kycStatus(level: KycLevel.level30), + ); + when(() => kycService.getUser()).thenAnswer((_) async => _user()); + when(() => kycService.continueKyc()).thenAnswer( + (_) async => _session( + level: KycLevel.level30, + steps: const [], + currentStep: _currentStep(KycStepName.ident), + ), + ); + }, + build: buildCubit, + act: (cubit) => cubit.checkKyc(), + verify: (_) => expect(reported, isEmpty), + ); + }); + group('$KycCubit acceptLegalDisclaimer', () { // Happy path: the user accepts on the disclaimer page. `acceptLegalDisclaimer` // records acceptance server-side (`acceptLegal` with the outstanding diff --git a/test/setup/error_handling/crash_reporting_test.dart b/test/setup/error_handling/crash_reporting_test.dart index b1aa3a1b6..76c4d593f 100644 --- a/test/setup/error_handling/crash_reporting_test.dart +++ b/test/setup/error_handling/crash_reporting_test.dart @@ -54,4 +54,14 @@ void main() { ); }); }); + + group('reportNonFatal', () { + // Same contract as the init above: reporting infrastructure must never be + // able to take the app down. Without an injected DSN the reporter never + // started, so the call has to fall through to the log line and return. + test('is inert and non-throwing when the reporter never started', () { + expect(crashReportingDsn, isEmpty); + expect(() => reportNonFatal(StateError('nothing is listening')), returnsNormally); + }); + }); } diff --git a/test/setup/routing/boot_navigation_apply_test.dart b/test/setup/routing/boot_navigation_apply_test.dart index 64d5a9067..e7462de5d 100644 --- a/test/setup/routing/boot_navigation_apply_test.dart +++ b/test/setup/routing/boot_navigation_apply_test.dart @@ -32,10 +32,13 @@ void main() { path: '/dashboard', builder: (_, _) => const Text('dashboard'), ), + // Mirrors the production builder: the KYC context is read off the URL, + // not off `extra`, so a restore by bare location keeps the flow scoped. GoRoute( name: AppRoutes.kyc, path: '/kyc', - builder: (_, _) => const Text('kyc'), + builder: (_, state) => + Text('kyc:${state.uri.queryParameters['context'] ?? 'unscoped'}'), ), GoRoute( name: AppRoutes.buyPaymentDetails, @@ -84,7 +87,7 @@ void main() { ); await tester.pumpAndSettle(); - expect(find.text('kyc'), findsOneWidget); + expect(find.text('kyc:unscoped'), findsOneWidget); expect(effectiveLocation(router.routerDelegate.currentConfiguration), '/kyc'); expect(cleared, isTrue); @@ -99,6 +102,33 @@ void main() { }, ); + // The KYC flow is scoped by the context its entry point reported, and a + // background lock must not silently drop that scope. The restore re-pushes + // the captured location by bare path — it has no `extra` to give — so the + // context has to ride in the URL to survive. + testWidgets( + 'restore keeps the KYC context the flow was entered with', + (tester) async { + final router = buildRouter(); + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + await tester.pumpAndSettle(); + + applyBootNavAction( + resolveAfterRelock('/kyc?context=RealunitBuy'), + router, + onLoadWallet: () {}, + onClearResume: () {}, + ); + await tester.pumpAndSettle(); + + expect(find.text('kyc:RealunitBuy'), findsOneWidget); + expect( + effectiveLocation(router.routerDelegate.currentConfiguration), + '/kyc?context=RealunitBuy', + ); + }, + ); + testWidgets('restoring /dashboard itself stays a plain go (nothing to pop)', ( tester, ) async { @@ -354,7 +384,7 @@ void main() { expect(router.canPop(), isTrue); router.pop(); await tester.pumpAndSettle(); - expect(find.text('kyc'), findsOneWidget); + expect(find.text('kyc:unscoped'), findsOneWidget); }, ); diff --git a/test/setup/routing/kyc_route_query_test.dart b/test/setup/routing/kyc_route_query_test.dart new file mode 100644 index 000000000..655a9e603 --- /dev/null +++ b/test/setup/routing/kyc_route_query_test.dart @@ -0,0 +1,18 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:realunit_wallet/setup/routing/routes/app_routes.dart'; + +void main() { + group('kycRouteQuery', () { + test('carries the context the entry point was handed', () { + expect(kycRouteQuery('RealunitBuy'), {'context': 'RealunitBuy'}); + expect(kycRouteQuery('RealunitSell'), {'context': 'RealunitSell'}); + }); + + // Only the API knows which context a gate belongs to. When it attached + // none, the route is entered unscoped — exactly as before — rather than + // with a context the app made up. + test('omits the parameter when the API attached no context', () { + expect(kycRouteQuery(null), isEmpty); + }); + }); +} From 3f69d3649e49189d23a985d1f7f4f1c314cd1cc4 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Wed, 26 Aug 2026 01:42:59 +0000 Subject: [PATCH 2/4] fix(kyc): match naming convention, harden context/report edge cases Rename UnsupportedKycStepException to KycUnsupportedStepException to match the feature's Kyc-prefixed naming convention (KycUnsupportedStepFailure, KycUnsupportedStepPage, KycLevelRequiredException). Treat an empty-string context the same as absent so an API-supplied empty value can never overwrite a real stored context. Wrap the Sentry call in reportNonFatal in a try/catch so a reporting-layer failure can never propagate into a caller, matching the swallow-on-failure contract already used by initCrashReporting. Co-Authored-By: Claude Sonnet 5 --- ...rt => kyc_unsupported_step_exception.dart} | 6 ++-- lib/screens/kyc/cubits/kyc/kyc_cubit.dart | 6 ++-- lib/setup/error_handling/crash_reporting.dart | 7 +++- .../exceptions/exception_surface_test.dart | 4 +-- ... kyc_unsupported_step_exception_test.dart} | 8 ++--- .../kyc/cubits/kyc/kyc_cubit_test.dart | 34 +++++++++++++++++-- 6 files changed, 49 insertions(+), 16 deletions(-) rename lib/packages/service/dfx/exceptions/{unsupported_kyc_step_exception.dart => kyc_unsupported_step_exception.dart} (82%) rename test/packages/service/dfx/exceptions/{unsupported_kyc_step_exception_test.dart => kyc_unsupported_step_exception_test.dart} (81%) diff --git a/lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart b/lib/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart similarity index 82% rename from lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart rename to lib/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart index a04ee021b..e9b0c9ff2 100644 --- a/lib/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart +++ b/lib/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart @@ -11,13 +11,13 @@ import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; /// /// [stepName] is the API's wire identifier for the step, or null when the API /// named no step at all. It carries nothing about the user. -class UnsupportedKycStepException implements Exception { +class KycUnsupportedStepException implements Exception { final KycStepName? stepName; - const UnsupportedKycStepException(this.stepName); + const KycUnsupportedStepException(this.stepName); @override String toString() => - 'UnsupportedKycStepException: no screen for KYC step ' + 'KycUnsupportedStepException: no screen for KYC step ' '${stepName?.value ?? ''}'; } diff --git a/lib/screens/kyc/cubits/kyc/kyc_cubit.dart b/lib/screens/kyc/cubits/kyc/kyc_cubit.dart index 2fc9e4b45..3ad4a16d4 100644 --- a/lib/screens/kyc/cubits/kyc/kyc_cubit.dart +++ b/lib/screens/kyc/cubits/kyc/kyc_cubit.dart @@ -6,7 +6,7 @@ import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:realunit_wallet/packages/service/app_store.dart'; import 'package:realunit_wallet/packages/service/dfx/dfx_kyc_service.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/api_exception.dart'; -import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_level_dto.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; import 'package:realunit_wallet/packages/service/dfx/models/legal/real_unit_legal_agreement.dart'; @@ -69,7 +69,7 @@ class KycCubit extends Cubit { super(const KycInitial()); Future checkKyc({String? context}) async { - _kycContext = context ?? _kycContext; + _kycContext = (context == null || context.isEmpty) ? _kycContext : context; final generation = ++_runGeneration; try { await _runCheckKyc(generation).timeout(_checkKycTimeout); @@ -360,7 +360,7 @@ class KycCubit extends Cubit { /// before the emit keeps the event even when the user leaves the flow on this /// screen. void _emitUnsupportedStep(KycStepName? stepName) { - _report(UnsupportedKycStepException(stepName)); + _report(KycUnsupportedStepException(stepName)); emit(KycUnsupportedStepFailure(stepName)); } diff --git a/lib/setup/error_handling/crash_reporting.dart b/lib/setup/error_handling/crash_reporting.dart index af5f8a268..8ed99bbcc 100644 --- a/lib/setup/error_handling/crash_reporting.dart +++ b/lib/setup/error_handling/crash_reporting.dart @@ -77,7 +77,12 @@ typedef NonFatalReporter = void Function(Object error); void reportNonFatal(Object error) { developer.log('non-fatal: $error', name: 'WalletApp', error: error); if (crashReportingDsn.isEmpty) return; - unawaited(Sentry.captureException(error)); + try { + unawaited(Sentry.captureException(error)); + } catch (_) { + // Best-effort, same as initCrashReporting: a reporting-layer failure must + // never propagate into the caller that is reporting through us. + } } /// Applies the pinned option set. The guarantee is exactly this list — an diff --git a/test/packages/service/dfx/exceptions/exception_surface_test.dart b/test/packages/service/dfx/exceptions/exception_surface_test.dart index b39e55835..208e30234 100644 --- a/test/packages/service/dfx/exceptions/exception_surface_test.dart +++ b/test/packages/service/dfx/exceptions/exception_surface_test.dart @@ -7,7 +7,7 @@ import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/pay_exce import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/sell_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/transfer_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/registration_rejected_exception.dart'; -import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; import 'package:realunit_wallet/packages/storage/secure_storage.dart'; import 'package:realunit_wallet/packages/wallet/exceptions/signing_cancelled_exception.dart'; @@ -44,7 +44,7 @@ void main() { const TransferGasFundingUnavailableException(), const TransferConfirmMismatchException(), const TransferAlreadyConfirmedException(code: 'TEST', message: 'test'), - const UnsupportedKycStepException(KycStepName.statutes), + const KycUnsupportedStepException(KycStepName.statutes), ]; for (final ex in exceptions) { diff --git a/test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart b/test/packages/service/dfx/exceptions/kyc_unsupported_step_exception_test.dart similarity index 81% rename from test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart rename to test/packages/service/dfx/exceptions/kyc_unsupported_step_exception_test.dart index 15cd29e6b..1bfc57954 100644 --- a/test/packages/service/dfx/exceptions/unsupported_kyc_step_exception_test.dart +++ b/test/packages/service/dfx/exceptions/kyc_unsupported_step_exception_test.dart @@ -1,21 +1,21 @@ import 'package:flutter_test/flutter_test.dart'; -import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; // The rendered string is the whole payload of the report — the crash reporter // receives nothing else about the occurrence. If it does not name the step, the // event cannot say which entry the client's mapping table is missing. void main() { - group('$UnsupportedKycStepException', () { + group('$KycUnsupportedStepException', () { test('names the step by its API wire identifier', () { - const exception = UnsupportedKycStepException(KycStepName.residencePermit); + const exception = KycUnsupportedStepException(KycStepName.residencePermit); expect(exception.stepName, KycStepName.residencePermit); expect(exception.toString(), contains(KycStepName.residencePermit.value)); }); test('renders readably when the API named no step at all', () { - const exception = UnsupportedKycStepException(null); + const exception = KycUnsupportedStepException(null); expect(exception.stepName, isNull); expect(exception.toString(), isNot(contains('null'))); diff --git a/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart b/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart index c5922f0e8..1b03dec75 100644 --- a/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart +++ b/test/screens/kyc/cubits/kyc/kyc_cubit_test.dart @@ -7,7 +7,7 @@ import 'package:mocktail/mocktail.dart'; import 'package:realunit_wallet/packages/service/app_store.dart'; import 'package:realunit_wallet/packages/service/dfx/dfx_kyc_service.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/api_exception.dart'; -import 'package:realunit_wallet/packages/service/dfx/exceptions/unsupported_kyc_step_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_level_dto.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_session_dto.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/dto/kyc_step_dto.dart'; @@ -1325,6 +1325,34 @@ void main() { const KycCompleted(), ], ); + + blocTest( + 'an empty context is treated as absent — the stored context is kept', + setUp: () { + when(() => kycService.getKycStatus(context: 'RealunitBuy')).thenAnswer( + (_) async => _kycStatus( + level: KycLevel.level50, + processStatus: KycProcessStatus.completed, + ), + ); + when(() => kycService.getUser()).thenAnswer((_) async => _user()); + }, + build: buildCubit, + act: (cubit) async { + await cubit.checkKyc(context: 'RealunitBuy'); + // An API-supplied empty string must not overwrite a real context. + await cubit.checkKyc(context: ''); + }, + verify: (_) { + verify(() => kycService.getKycStatus(context: 'RealunitBuy')).called(2); + }, + expect: () => [ + const KycLoading(), + const KycCompleted(), + const KycLoading(), + const KycCompleted(), + ], + ); }); // The handoff screen alone leaves the gap silent: every call in the flow @@ -1355,7 +1383,7 @@ void main() { ], verify: (_) { expect(reported, hasLength(1)); - final error = reported.single as UnsupportedKycStepException; + final error = reported.single as KycUnsupportedStepException; expect(error.stepName, KycStepName.residencePermit); // The wire identifier has to be in the rendered event, otherwise the // report cannot say which mapping entry is missing. @@ -1383,7 +1411,7 @@ void main() { ], verify: (_) { expect(reported, hasLength(1)); - expect((reported.single as UnsupportedKycStepException).stepName, isNull); + expect((reported.single as KycUnsupportedStepException).stepName, isNull); }, ); From 8fecc8a89e712853b9fd96bb1f01be0ec0e847b6 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Wed, 26 Aug 2026 01:58:19 +0000 Subject: [PATCH 3/4] fix(kyc): close remaining review gaps in context and reporting Normalize an empty-string context to absent in kycRouteQuery too, not just in KycCubit, so an API-supplied empty context can never produce an invented /kyc?context= query. Add sell-side coverage for context forwarding on kycRequired/registrationRequired, matching the existing buy-side test. Replace the sync-only try/catch around the Sentry call in reportNonFatal with Future.ignore(), which also discards an asynchronous rejection instead of letting it surface as an unhandled error. Fix an import-order regression introduced by the previous rename. Co-Authored-By: Claude Sonnet 5 --- lib/setup/error_handling/crash_reporting.dart | 8 +-- lib/setup/routing/routes/app_routes.dart | 2 +- .../exceptions/exception_surface_test.dart | 2 +- .../sell/sell_button_call_site_test.dart | 52 ++++++++++++++++++- test/setup/routing/kyc_route_query_test.dart | 6 +++ 5 files changed, 64 insertions(+), 6 deletions(-) diff --git a/lib/setup/error_handling/crash_reporting.dart b/lib/setup/error_handling/crash_reporting.dart index 8ed99bbcc..29f50156e 100644 --- a/lib/setup/error_handling/crash_reporting.dart +++ b/lib/setup/error_handling/crash_reporting.dart @@ -78,10 +78,12 @@ void reportNonFatal(Object error) { developer.log('non-fatal: $error', name: 'WalletApp', error: error); if (crashReportingDsn.isEmpty) return; try { - unawaited(Sentry.captureException(error)); + Sentry.captureException(error).ignore(); } catch (_) { - // Best-effort, same as initCrashReporting: a reporting-layer failure must - // never propagate into the caller that is reporting through us. + // A caller-visible throw here must never happen, since this runs before + // a required emit in KycCubit. `.ignore()` discards both the eventual + // value and any asynchronous error; the try/catch covers a synchronous + // throw from the call itself. } } diff --git a/lib/setup/routing/routes/app_routes.dart b/lib/setup/routing/routes/app_routes.dart index 9beffaafb..876f7a308 100644 --- a/lib/setup/routing/routes/app_routes.dart +++ b/lib/setup/routing/routes/app_routes.dart @@ -29,5 +29,5 @@ abstract final class AppRoutes { /// A null [kycContext] means the API attached none; the route is then entered /// unscoped exactly as before, never with an invented context. Map kycRouteQuery(String? kycContext) => { - if (kycContext != null) 'context': kycContext, + if (kycContext != null && kycContext.isNotEmpty) 'context': kycContext, }; diff --git a/test/packages/service/dfx/exceptions/exception_surface_test.dart b/test/packages/service/dfx/exceptions/exception_surface_test.dart index 208e30234..6992880bf 100644 --- a/test/packages/service/dfx/exceptions/exception_surface_test.dart +++ b/test/packages/service/dfx/exceptions/exception_surface_test.dart @@ -2,12 +2,12 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/api_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/buy_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/pay_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/sell_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/transfer_exceptions.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/registration_rejected_exception.dart'; -import 'package:realunit_wallet/packages/service/dfx/exceptions/kyc_unsupported_step_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/models/kyc/kyc_level.dart'; import 'package:realunit_wallet/packages/storage/secure_storage.dart'; import 'package:realunit_wallet/packages/wallet/exceptions/signing_cancelled_exception.dart'; diff --git a/test/screens/sell/sell_button_call_site_test.dart b/test/screens/sell/sell_button_call_site_test.dart index 6ff6dfb0d..f0752ff02 100644 --- a/test/screens/sell/sell_button_call_site_test.dart +++ b/test/screens/sell/sell_button_call_site_test.dart @@ -22,6 +22,7 @@ import 'package:realunit_wallet/screens/sell/cubits/sell_payment_info/sell_payme import 'package:realunit_wallet/screens/sell/widgets/sell_button.dart'; import 'package:realunit_wallet/screens/sell/widgets/sell_confirm_sheet.dart'; import 'package:realunit_wallet/screens/sell/widgets/sell_executed_sheet.dart'; +import 'package:realunit_wallet/setup/routing/routes/app_routes.dart'; import 'package:realunit_wallet/styles/currency.dart'; import 'package:realunit_wallet/styles/themes.dart'; @@ -105,7 +106,10 @@ void main() { when(() => sellConverterCubit.state).thenReturn(const SellConverterState()); }); - Future<_RouteCapturingObserver> pumpSellButton(WidgetTester tester) async { + Future<_RouteCapturingObserver> pumpSellButton( + WidgetTester tester, { + ValueChanged? onKycPushed, + }) async { final observer = _RouteCapturingObserver(); final router = GoRouter( observers: [observer], @@ -124,6 +128,14 @@ void main() { ), ), ), + GoRoute( + name: AppRoutes.kyc, + path: '/kyc', + builder: (_, state) { + onKycPushed?.call(state.uri.queryParameters['context']); + return const Scaffold(body: Text('kyc')); + }, + ), ], ); addTearDown(router.dispose); @@ -214,4 +226,42 @@ void main() { }, ); }); + + group('$SellButton KYC context forwarding', () { + testWidgets( + 'forwards the API-supplied context to the KYC route on kycRequired', + (tester) async { + whenListen( + sellPaymentInfoCubit, + Stream.value( + const SellPaymentInfoFailure(.kycRequired, context: 'RealunitSell'), + ), + initialState: const SellPaymentInfoInitial(), + ); + + String? pushedContext; + await pumpSellButton(tester, onKycPushed: (context) => pushedContext = context); + + expect(pushedContext, 'RealunitSell'); + }, + ); + + testWidgets( + 'forwards the API-supplied context to the KYC route on registrationRequired', + (tester) async { + whenListen( + sellPaymentInfoCubit, + Stream.value( + const SellPaymentInfoFailure(.registrationRequired, context: 'RealunitSell'), + ), + initialState: const SellPaymentInfoInitial(), + ); + + String? pushedContext; + await pumpSellButton(tester, onKycPushed: (context) => pushedContext = context); + + expect(pushedContext, 'RealunitSell'); + }, + ); + }); } diff --git a/test/setup/routing/kyc_route_query_test.dart b/test/setup/routing/kyc_route_query_test.dart index 655a9e603..3f0997433 100644 --- a/test/setup/routing/kyc_route_query_test.dart +++ b/test/setup/routing/kyc_route_query_test.dart @@ -14,5 +14,11 @@ void main() { test('omits the parameter when the API attached no context', () { expect(kycRouteQuery(null), isEmpty); }); + + // An empty string is not a real context either — treat it the same as + // null rather than inventing a query the API never attached. + test('omits the parameter when the API attached an empty context', () { + expect(kycRouteQuery(''), isEmpty); + }); }); } From 87bb0e3de68e7fc9a47ce34bdd0c69faae0d2c73 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Wed, 26 Aug 2026 02:07:55 +0000 Subject: [PATCH 4/4] docs(kyc): note empty context in kycRouteQuery doc comment The doc comment described the null case only; the function has treated empty the same as null since the previous commit. Co-Authored-By: Claude Sonnet 5 --- lib/setup/routing/routes/app_routes.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/setup/routing/routes/app_routes.dart b/lib/setup/routing/routes/app_routes.dart index 876f7a308..a76c73f1a 100644 --- a/lib/setup/routing/routes/app_routes.dart +++ b/lib/setup/routing/routes/app_routes.dart @@ -26,8 +26,8 @@ abstract final class AppRoutes { /// instead of only those the entered flow needs — reporting a user who has /// finished `Ident` as still in progress for steps that do not gate buying. /// -/// A null [kycContext] means the API attached none; the route is then entered -/// unscoped exactly as before, never with an invented context. +/// A null or empty [kycContext] means the API attached none; the route is +/// then entered unscoped exactly as before, never with an invented context. Map kycRouteQuery(String? kycContext) => { if (kycContext != null && kycContext.isNotEmpty) 'context': kycContext, };