Client(iOS) - Fix the Tap to Pay wedge when a reader session dies under a ready state - #23
Open
Alex Arguello (alex-arguello) wants to merge 4 commits into
Conversation
Locking the phone during a charge tears down the reader session. The failure was reported and `sessionState` was left at `.ready`, which wedged the session permanently: `charge()` starts with `reinitializeIfNeeded()`, and that does nothing while the state says ready, so every later charge reused the dead session. Force-quitting the app was the only way out, and a host that gates its recovery affordance on a repairable state never offered one. A session-level reader failure now moves to `.sessionExpired`, which the transition matrix already permitted from `.ready` and which `reinitializeIfNeeded()` already repairs by re-fetching config and re-preparing the reader. The thrown error is unchanged, and so is the best-effort notify. Telling a dead session from a failed read is Apple's distinction, not one invented here: `PaymentCardReaderSession.ReadError` separates `noReaderSession`, `readerSessionExpired`, `readerTokenExpired` and `readerSessionAuthenticationError` from `readCancelled`, `cardReadFailed`, `paymentCardDeclined` and the rest. A cancelled tap must not invalidate, or dismissing the sheet would cost a config round trip and a reader re-prepare. The check has two tiers because the typed value does not survive the trip. The card-reader component catches the read error and rebuilds it as a title plus a description, so on a device only the case name reaches us, in text. The typed check runs first because it cannot be fooled; the text check is the one that fires in production. Tests fail before the change: the session stayed `.ready` and a second charge never re-prepared the reader. Sabotaging the classifier to treat every failure as session-level fails five of them, so the "must not invalidate" cases bite rather than passing by default. Charges in these tests are bounded, so a regression that wedges the path fails on the bound instead of the suite clock. `SessionManager.forceSessionExpiry` is now private and marked for deletion. It had no caller and claimed it was "called on 401s", which nothing did; 401s are handled by refresh and retry in the transport. Its test is replaced by one covering the transition a live caller actually takes.
Found on device, one step behind the wedge fix. After a session expired, pressing Enable Terminal reported "Reader ready" while the screen still showed the step failed, because both were true: initialize() ran every phase and the state never moved. The transition matrix is narrow, and `.sessionExpired` reaches only `.reinitializing`. So `.fetchingConfig`, `.initializingReader` and `.ready` were all rejected, and every result is discarded with `_ =`, which turns a rejected transition into silence. Attestation ran, config was fetched, the reader was prepared, and the host was left holding a session the SDK had just declared dead. initialize() now resets to `.idle` first, which is what "call this again" has always meant, and makes each following transition legal. The same hole was reachable from `.ready`, where it happened to end on the right state by way of the identity transition rather than by design. Two tests: recovering from an expired session, and re-initializing a healthy one. The first fails before this change with the state stuck at `.sessionExpired`. Also stops two error paths re-wrapping an error that is already a PayabliTTPError. `String(describing:)` on one prints the whole enum back, so a host showing `localizedDescription` displayed `readerSetupFailed(reason: "passcodeDisabled: ... doesn\'t ...")`, escape character included, instead of the sentence inside it. The two neighbouring phases in the same file already guarded against this; prepare-reader and charge did not.
…pping A launch argument only reaches the process it launched. Reopening the app from the Home screen dropped back to the sandbox default against a device enrolled in another environment, and the only sign was the host name in the header. It cost a device test: attestation failed with a generic error that says nothing about which backend was being asked. The choice is remembered now, so a device stays where it was put until told otherwise; pass `sandbox` explicitly to go back. The Config tab distinguishes the default, an argument, and a remembered argument. The name mapping this needed already existed in the Config tab, and adding a second copy is how the two drift, so there is one.
Each message describes one attempt, and nothing cleared them. After a dead reader session was recovered, step 4 still carried the error that had killed it while sitting ready to run again — reporting a failure that was no longer true, directly under the button that would disprove it. Enabling or re-initializing clears the outcomes of the session being replaced, and a charge clears its own before starting.
Copilot started reviewing on behalf of
Alex Arguello (alex-arguello)
August 10, 2026 03:53
View session
There was a problem hiding this comment.
Pull request overview
Fixes Tap to Pay recovery when the underlying reader session dies while the SDK remains .ready.
Changes:
- Classifies session-ending reader failures and triggers reinitialization.
- Makes
initialize()reset stale state and preserves typed error messages. - Adds recovery tests and updates demo diagnostics/configuration persistence.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
Tests/PayabliSDKTapToPayTests/SessionManagerTests.swift |
Verifies ready-to-expired transitions. |
Tests/PayabliSDKTapToPayTests/PayabliTTPReaderSessionRecoveryTests.swift |
Adds reader recovery and error tests. |
Sources/PayabliSDKTapToPay/SessionManager.swift |
Restricts the obsolete forced-expiry method. |
Sources/PayabliSDKTapToPay/ReaderFailureClassification.swift |
Classifies session-level reader failures. |
Sources/PayabliSDKTapToPay/PayabliTTP+Initialize.swift |
Resets state and preserves typed errors. |
Sources/PayabliSDKTapToPay/PayabliTTP+Charge.swift |
Expires dead sessions during charges. |
Example/PayabliDemo/TapToPay/PaymentTapToPayQAView.swift |
Clears stale attempt outcomes. |
Example/PayabliDemo/Configuration/DemoConfiguration.swift |
Persists environment overrides. |
Example/PayabliDemo/Configuration/ConfigurationQAView.swift |
Uses shared environment naming. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+92
to
+96
| if readerFailureInvalidatesSession(error) { | ||
| sessionManager.transition(to: .sessionExpired) | ||
| syncPublished() | ||
| multicaster.emit(.sessionExpired) | ||
| } |
Comment on lines
+348
to
+353
| do { | ||
| _ = try await expression() | ||
| XCTFail(message, file: file, line: line) | ||
| } catch { | ||
| // Expected. | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Locking the phone during a charge tears down the reader session. The SDK reported the failure and
left
sessionStateat.ready, which wedged the session permanently:charge()begins withreinitializeIfNeeded(), and that does nothing while the state says ready, so every later chargereused the dead session. Force-quitting the app was the only way out.
Found on an iPhone 11 Pro. Two more defects sat behind it, each reachable only once the one before
it was fixed. All three were found on the device.
What changed
A session-level reader failure now expires the session.
.readyto.sessionExpiredwas alreadya legal transition, and
reinitializeIfNeeded()already repairs that state by re-fetching config andpreparing the reader. The thrown error is unchanged, and so is the best-effort backend notify.
initialize()works from whatever state it finds. It previously ran every phase — attestation,config, preparing the reader — and then left the state exactly as it found it, because from
.sessionExpiredeach transition it attempts is rejected and every result is discarded with_ =.On screen that read as "Reader ready" next to a step marked failed. Both were true. It now resets to
.idlefirst, so each following transition is legal.Two error paths stopped re-wrapping an error that is already a
PayabliTTPError.String(describing:)on one prints the whole enum back, so a host showinglocalizedDescriptiondisplayed
readerSetupFailed(reason: "passcodeDisabled: ... doesn\'t ..."), escape characterincluded, instead of the sentence inside it. The two neighbouring phases in the same file already
guarded against this; prepare-reader and charge did not.
The sample app gets two fixes for problems this exposed: the environment override is remembered, and
a step's outcome is cleared when the session it described is rebuilt.
Classifying reader failures
Session-level failures invalidate the session; read-level failures do not. The set is Apple's:
PaymentCardReaderSession.ReadErrorseparatesnoReaderSession,readerSessionExpired,readerTokenExpiredandreaderSessionAuthenticationErrorfromreadCancelled,cardReadFailed,paymentCardDeclinedand the rest. A cancelled tap must not invalidate the session: dismissing thesheet would otherwise cost a config round trip and a reader re-prepare.
Two constraints require a two-tier check:
noReaderSessionis not aPaymentCardReaderError, which is the only enum the existing mapperchecks, so no read failure was ever matched by type.
rebuilds it as a title plus a description, so on a device only the case name arrives, in text. That
source is byte-identical to upstream by contract, so it cannot be fixed there.
The typed check runs first. The text check covers the stringified path, which is the one that occurs
in production.
Verification
Eleven new tests. The two that describe the wedge fail before the change with the state stuck at
.readyand the reader never re-prepared; theinitialize()test fails with the state stuck at.sessionExpired. Charges in these tests are bounded, so a regression that wedges the path fails inmilliseconds with a message naming the bound.
Sabotaging the classifier to treat every failure as session-level fails five tests, which is the
evidence that the cases that must not invalidate are doing work.
Full package suite green.
On an iPhone 11 Pro, against a live backend. The suite cannot cover this path, because the
stringified error only occurs with the real reader.
Enable the terminal, charge so Apple's sheet appears, lock the device, unlock, and charge again. The
event log records
sessionExpired, thenconfigReceived,readerInitializingandreaderReadyafter re-enabling, and the terminal returns to ready. On
mainthe same sequence repeatsnoReaderSessionindefinitely and the session never leaves.ready.Out of scope
/MoneyIn/initiateon a chargethat cannot complete, since the transaction id is minted before the reader is touched. It needs the
background-versus-lock behaviour measured on a device first.
and battery exhaustion run none of them, and the SDK persists nothing about an in-flight charge.
Tracked separately as a backend question.
SessionManager.forceSessionExpiryis now private and marked for deletion. It had no caller,and its documentation claimed it was "called on 401s", which no code did.
Parity
iOS first. Android has no card-present surface yet, so there is no counterpart binding to mirror and
its card-present band inherits the corrected state model. Sequencing, not an asymmetry to record.