View Issue Details

IDProjectCategoryView StatusLast Update
0011985Talerwallet (iOS App)public2026-11-09 22:33
Reporterbot-cg [agent] Assigned ToResolution deadline
ImplementerMarcS ReviewerBohdan  
Release signed byMarcS Deployed byMarcS  
Verifieravalos CommunicatorCM_7  
Data protection authority notification requiredNo  
CSIRT notification requiredNo  
Estimated effort (hours)Actual effort (hours)0 
Fast-trackNo
Work packagenone
PrioritynormalSeverityminorReproducibilityhave not tried
Status planningResolutionopen 
Summary0011985: Three *Done sheets have no failure branch: a failed accept, manual withdrawal or deposit leaves its LoadingView spinning, two of them with the back button hidden
DescriptionThree sheets run a wallet-core call from `.task`, keep the result in a
`@State`, and render a `LoadingView` until it arrives. None of them has a
branch for the call failing, so a failure leaves the spinner up -- and two
of the three hide their own back button while it is up.

The pattern, in each file:

**1. `P2pAcceptDone` (TalerWallet1/Views/Sheets/P2P_Sheets/P2pAcceptDone.swift:23-53)**
is the clearest: its `body` is *unconditionally* a `LoadingView`, and the
only way off the screen is the `dismissTop` inside the success path.

        private func viewDidLoad() async {
            if incoming {
                if let _ = try? await model.acceptPeerPushCredit(transactionId) {
                    if let url { controller.removeURL(url) }
                    dismissTop(stack.push())
                }
            } else {
                if let _ = try? await model.confirmPeerPullDebit(transactionId) {
                    ...
                    dismissTop(stack.push())
                }
            }
        }

        var body: some View {
            let message = String(localized: "Accepting...", comment: "loading")
            LoadingView(...)
                .navigationBarBackButtonHidden(true)
                .task { await viewDidLoad() }
        }

On failure nothing is assigned, `dismissTop` is not reached, the back
button is hidden, and "Accepting..." stays. The `interactiveDismissDisabled()`
line immediately below is commented out, so a swipe is the only exit -- by
accident, since the comment beside it says the screen was meant to be
dismissable "only with the Done button".

**2. `ManualWithdrawDone` (TalerWallet1/Views/Actions/Banking/ManualWithdrawDone.swift:30-68)**
has the same shape with a success branch: `transactionId` stays nil, so
`body` keeps taking its `else` and re-renders the `LoadingView`, whose
`.task` has already run.

**3. `DepositAmountAction` (TalerWallet1/Views/Actions/Banking/DepositAmountAction.swift:78-118)**
is the variant that does recover -- `depositStarted = false` returns the
user to the amount screen -- but says nothing about why, and its loading
state is the subject of 11983.

**What the user does see.** All of these call wallet-core with the default
`viewHandles: false`, so `sendRequest`'s catch does call `setError`
(WalletModel.swift:202-209) and the global error presentation appears. So
the complaint is not that the app is silent; it is that

* the error is the generic one -- on the deposit path, measured today, that
  is "Internal core error" over a "Copy the error JSON" button (11934);
* and dismissing it returns the user to a screen that is still spinning,
  with its own back button hidden and its `.task` already spent, so the
  spinner is now permanent.

**Already filed as one instance**: 11980, `WithdrawAcceptDone`, where
wallet-core answered 7046 with the state that stopped it and the view
discarded it with `try?`. The patch attached there adds the missing
branch. This report is the other three sites, because the same patch has
to be written three more times and the pattern is the thing worth fixing.
Steps To ReproduceFor the clearest instance (`P2pAcceptDone`):

1. Receive a peer-push payment whose purse is no longer acceptable -- an
   expired purse is the easiest, and this harness produces one by minting a
   `taler://pay-push/` URI and opening it after `purse_expiration`
   (scripts/p2p_kyc_purse.py mints them; the spool records `minted`).
2. Open the URI in the wallet, accept the terms, and press
   "Accept and receive".

Observed: the generic error appears; dismissing it leaves "Accepting..."
with no back button. The screen never changes again.

Expected: the sheet shows what happened and offers a way out -- which is
what the patch on 11980 does for `WithdrawAcceptDone`:

        } catch {
            acceptError = error
        }
        ...
        } else if let acceptError {
            ErrorView(stack.push(), error: acceptError,
                      devMode: developerMode) { dismissTop(stack.push()) }
        }
Additional InformationThe three sites need the same three changes each: `try?` becomes
`try` in a `do/catch`, the catch stores the error, and `body` grows an
error branch with a dismissal. `P2pAcceptDone` additionally needs a success
*state* -- it currently has none, because success dismisses the sheet
outright.

Worth deciding once rather than three times: whether these sheets should
keep `viewHandles: false` (global error presentation, as now) or take
`viewHandles: true` and present the error themselves. Mixing the two is
what produces today's behaviour, where the user is told something went
wrong by one layer and left stuck by another.

Found by auditing the app for the defect class in the report that started
this series: a flow that stops without telling the user how to continue.
There are 82 `try? await model.` call sites under `TalerWallet1/Views`;
these three are the ones where the discarded error leaves a terminal
waiting state, which is why they are the three in this report.
TagsNo tags attached.
Attached Files
11985-done-sheets-failure-branch.diff (6,989 bytes)   
diff --git a/TalerWallet1/Views/Actions/Banking/ManualWithdrawDone.swift b/TalerWallet1/Views/Actions/Banking/ManualWithdrawDone.swift
index 270b29b1..b1c10184 100755
--- a/TalerWallet1/Views/Actions/Banking/ManualWithdrawDone.swift
+++ b/TalerWallet1/Views/Actions/Banking/ManualWithdrawDone.swift
@@ -22,8 +22,15 @@ struct ManualWithdrawDone: View {
     @EnvironmentObject private var controller: Controller
     @EnvironmentObject private var model: WalletModel
 
+    @AppStorage("developerMode") var developerMode: Bool = false
     @State private var acceptManualWithdrawalResult: AcceptManualWithdrawalResult?
     @State private var transactionId: String?
+    /// Why the withdrawal was not accepted, if it was not.
+    ///
+    /// `transactionId` stayed nil on failure, so `body` kept taking its
+    /// `else` and re-rendered the `LoadingView` whose `.task` had already
+    /// run: a spinner that could never resolve.  Issue 11985.
+    @State private var acceptError: Error? = nil
     @State private var talerTX: TalerTransaction = TalerTransaction(dummyCurrency: DEMOCURRENCY)
 
     let navTitle = String(localized: "Wire Transfer")
@@ -31,14 +38,24 @@ struct ManualWithdrawDone: View {
     @MainActor
     private func viewDidLoad() async {
         if transactionId == nil {
-            if let result = try? await model.acceptManualWithdrawal(amountToTransfer,
+            do {
+                let result = try await model.acceptManualWithdrawal(amountToTransfer,
                                                            baseUrl: baseURL,
                                                        restrictAge: nil)    // No age restriction
-            {
                 if let url {
                     controller.removeURL(url)
                 }
-                transactionId = result.transactionId
+                transactionId = result?.transactionId
+                if transactionId == nil {
+                    // Success with no body: handled explicitly, because
+                    // falling through is how the permanent spinner comes
+                    // back.
+                    symLog.log("acceptManualWithdrawal returned no response")
+                    acceptError = WalletBackendError.deserializationError
+                }
+            } catch {
+                symLog.log("acceptManualWithdrawal failed: \(error)")
+                acceptError = error
             }
         }
     }
@@ -59,6 +76,9 @@ struct ManualWithdrawDone: View {
                                     showDone: .prominent,
                                          url: nil,
                                  withActions: false)
+            } else if let acceptError {
+                ErrorView(stack.push(), error: acceptError,
+                          devMode: developerMode) { dismissTop(stack.push()) }
             } else {
                 LoadingView(stack: stack.push(), scopeInfo: nil, message: baseURL.trimURL)
                     .task { await viewDidLoad() }
diff --git a/TalerWallet1/Views/Sheets/P2P_Sheets/P2pAcceptDone.swift b/TalerWallet1/Views/Sheets/P2P_Sheets/P2pAcceptDone.swift
index 31952719..ae5f1c5e 100644
--- a/TalerWallet1/Views/Sheets/P2P_Sheets/P2pAcceptDone.swift
+++ b/TalerWallet1/Views/Sheets/P2P_Sheets/P2pAcceptDone.swift
@@ -21,22 +21,40 @@ struct P2pAcceptDone: View {
     @EnvironmentObject private var controller: Controller
     @EnvironmentObject private var model: WalletModel
 
+    @AppStorage("developerMode") var developerMode: Bool = false
+
+    /// Why the accept did not go through, if it did not.
+    ///
+    /// There was no such state and no branch for it: `body` was
+    /// unconditionally a `LoadingView` with
+    /// `navigationBarBackButtonHidden(true)`, and the only way off the
+    /// screen was the `dismissTop` inside the success path.  So a failure
+    /// assigned nothing, dismissed nothing, and left "Accepting..." up for
+    /// good -- the `interactiveDismissDisabled()` line below is commented
+    /// out, which is the only reason a swipe still worked.  Issue 11985.
+    ///
+    /// The error itself was never silent: both calls use the default
+    /// `viewHandles: false`, so `sendRequest`'s catch calls `setError` and
+    /// the global presentation appears.  What was missing is that
+    /// dismissing *that* returned the user to a screen whose `.task` had
+    /// already run and which could therefore never change again.
+    @State private var acceptError: Error? = nil
+
     @MainActor
     private func viewDidLoad() async {
-        if incoming {
-            if let _ = try? await model.acceptPeerPushCredit(transactionId) {
-                if let url {
-                    controller.removeURL(url)
-                }
-                dismissTop(stack.push())
+        do {
+            if incoming {
+                _ = try await model.acceptPeerPushCredit(transactionId)
+            } else {
+                _ = try await model.confirmPeerPullDebit(transactionId)
             }
-        } else {
-            if let _ = try? await model.confirmPeerPullDebit(transactionId) {
-                if let url {
-                    controller.removeURL(url)
-                }
-                dismissTop(stack.push())
+            if let url {
+                controller.removeURL(url)
             }
+            dismissTop(stack.push())
+        } catch {
+            symLog.log("accept failed: \(error)")
+            acceptError = error
         }
     }
 
@@ -48,11 +66,21 @@ struct P2pAcceptDone: View {
         let navTitle = incoming ? String(localized: "Received", comment: "Nav Title, short")
                                 : String(localized: "Paid", comment: "Nav Title, short")
         let message = String(localized: "Accepting...", comment: "loading")
-        LoadingView(stack: stack.push(), scopeInfo: nil, message: message)
-            .navigationBarBackButtonHidden(true)
-//            .interactiveDismissDisabled()           // can only use "Done" button to dismiss
+        ZStack {
+            if let acceptError {
+                // A way out, which this screen did not have: the back
+                // button is hidden while it is loading, and on failure the
+                // loading was permanent.
+                ErrorView(stack.push(), error: acceptError,
+                          devMode: developerMode) { dismissTop(stack.push()) }
+            } else {
+                LoadingView(stack: stack.push(), scopeInfo: nil, message: message)
+                    .navigationBarBackButtonHidden(true)
+//                  .interactiveDismissDisabled()     // can only use "Done" button to dismiss
+                    .task { await viewDidLoad() }
+            }
+        }
             .navigationTitle(navTitle)
-            .task { await viewDidLoad() }
             .onAppear() {
                 symLog.log("onAppear")
                 DebugViewC.shared.setSheetID(incoming ? SHEET_RCV_P2P_ACCEPT

Required notifications

Address or linkRemarkAdded byNotifiedActions

The plan

IDSummaryStatusAssigned ToEstimated effort (hours)
This issue has no child issue. Work that is big enough to be divided is divided here, and each part is planned on its own.
This issue (hours)
Its children (hours)0
Together (hours)0
Nothing is missing: the plan can be finished.

Activities

bot-cg [agent]

2026-11-09 22:33

updater   ~0030358

Patch attached: `11985-done-sheets-failure-branch.diff`, for two of the
three sites. The third, `DepositAmountAction`, is in this issue's
description for completeness but its loading state belongs to 11983 and is
patched there.

`P2pAcceptDone`: `try?` becomes `try` in a `do/catch`, the catch stores the
error, and `body` becomes a `ZStack` with an `ErrorView` branch. That view
had no success state either -- success dismisses the sheet -- so the error
branch is the only state it has ever had besides the spinner.

`ManualWithdrawDone`: the same, plus an explicit branch for a success with
no body, since `acceptManualWithdrawal` returns an optional and letting nil
fall through is how the permanent spinner comes back.

Both use `ErrorView(stack.push(), error:, devMode:) { dismissTop(...) }`,
which is what the 11980 patch does for `WithdrawAcceptDone` -- the three
sheets then behave the same way on failure.

Built against iOS 27.2, Xcode 27.2: compiles, and both new log literals
(`accept failed`, `acceptManualWithdrawal returned no response`) are
present in the built binary.

Not addressed, and worth deciding once rather than three times: these
sheets keep `viewHandles: false`, so wallet-core's error still also reaches
the global presentation. The user now gets a sheet that explains itself AND
a global alert. Whether these screens should take `viewHandles: true` and
own their errors is the design question behind all three sites; this patch
makes them stop being stuck without pre-empting it.

Issue History

Date ModifiedUsernameFieldChange
2026-11-09 21:53bot-cgNew Issue
2026-11-09 22:22Christian GrothoffImplementer => MarcS
2026-11-09 22:22Christian GrothoffReviewer => Bohdan
2026-11-09 22:22Christian GrothoffRelease signed by => MarcS
2026-11-09 22:22Christian GrothoffDeployed by => MarcS
2026-11-09 22:22Christian GrothoffVerifier => avalos
2026-11-09 22:22Christian GrothoffStatusnew => planning
2026-11-09 22:32bot-cgFile Added: 11985-done-sheets-failure-branch.diff
2026-11-09 22:33bot-cgNote Added: 0030358