View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0011985 | Taler | wallet (iOS App) | public | 2026-11-09 21:53 | 2026-11-09 22:33 |
| Reporter | bot-cg [agent] | Assigned To | Resolution deadline | ||
| Implementer | MarcS | Reviewer | Bohdan | ||
| Release signed by | MarcS | Deployed by | MarcS | ||
| Verifier | avalos | Communicator | CM_7 | ||
| Data protection authority notification required | No | ||||
| CSIRT notification required | No | ||||
| Estimated effort (hours) | Actual effort (hours) | 0 | |||
| Fast-track | No | ||||
| Work package | none | ||||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | planning | Resolution | open | ||
| Summary | 0011985: 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 | ||||
| Description | Three 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 Reproduce | For 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 Information | The 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. | ||||
| Tags | No 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
| ||||
| 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. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2026-11-09 21:53 | bot-cg | New Issue | |
| 2026-11-09 22:22 | Christian Grothoff | Implementer | => MarcS |
| 2026-11-09 22:22 | Christian Grothoff | Reviewer | => Bohdan |
| 2026-11-09 22:22 | Christian Grothoff | Release signed by | => MarcS |
| 2026-11-09 22:22 | Christian Grothoff | Deployed by | => MarcS |
| 2026-11-09 22:22 | Christian Grothoff | Verifier | => avalos |
| 2026-11-09 22:22 | Christian Grothoff | Status | new => planning |
| 2026-11-09 22:32 | bot-cg | File Added: 11985-done-sheets-failure-branch.diff | |
| 2026-11-09 22:33 | bot-cg | Note Added: 0030358 |