Skip to content

refactor(psbt_flow): sanitize user-facing error messages - #2514

Draft
wired-pasteque wants to merge 4 commits into
developfrom
refactor-errors-psbt-flow
Draft

refactor(psbt_flow): sanitize user-facing error messages#2514
wired-pasteque wants to merge 4 commits into
developfrom
refactor-errors-psbt-flow

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

@wired-pasteque wired-pasteque self-assigned this Jul 28, 2026
@claude

This comment was marked as resolved.

@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, check the fixes

@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 1m 53s —— View job


Fix verification: all four findings addressed, CI green ✅

  • Identify commits pushed since the previous review — one fix commit: 9c50057cc ("fix: correct the animated qr failure path"), nothing newer
  • Verify fix 1: dead PsbtFlowUnexpectedFailure variant
  • Verify fix 2: isClosed guard after await
  • Verify fix 3: BBQR exception type (bdk.PsbtParseException)
  • Verify fix 4: stale timer cancelled on failure
  • Check for regressions in the fix commit
  • Check CI status and post verdict

Verdict

All four findings from the original review are fixed correctly, and the "Analyze and Test" CI run on this exact commit completed with success (run 30410434569). Nothing left blocking from my side.

  1. PsbtFlowUnexpectedFailure no longer dead — the generic catch in generate_psbt_qr_parts_usecase.dart:58 now returns Err(PsbtFlowUnexpectedFailure(e.toString())). The raw message goes only into Failure.logMessage, which the base class explicitly documents as "For logs / Sentry ONLY. MUST never reach the UI" — and the UI mapping (psbt_flow_failure_l10n.dart:9) shows the generic oopsSomethingWentWrong string. The sanitization invariant of this PR holds.

  2. isClosed guard addedshow_animated_qr_cubit.dart:34 returns after the await if the cubit was closed, with a comment explaining the popped-screen scenario. No more StateError on a closed cubit.

  3. Exception type corrected — the parser catch is now on bdk.PsbtParseException (generate_psbt_qr_parts_usecase.dart:49). I couldn't inspect the bull_sdk package locally (no pub cache on this runner), but the CI analyze job compiling this exact commit is the authoritative check — an unknown type in an on clause is an analyzer error, and CI passed.

  4. Stale timer cancelled — the Err arm now calls _timer?.cancel() before emitting the failure (show_animated_qr_cubit.dart:50), so an earlier success's timer can't keep cycling stale parts behind the error screen.

Regression check

The generic catch changed its failure type from PsbtFlowQrEncodingFailure to PsbtFlowUnexpectedFailure — no test asserts on that path (the test file only covers the invalid-base64, empty-PSBT, happy-path, and QrType.none cases, none of which changed behavior), and the empty-parts guard still returns PsbtFlowQrEncodingFailure as before. The oopsSomethingWentWrong key already exists in the template arb, so no new l10n work was needed.


@SatoshiPortal SatoshiPortal deleted a comment from claude Bot Jul 29, 2026
@ethicnology

Copy link
Copy Markdown
Member

Conflict management required plz @wired-pasteque

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants