Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-05-07
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
## Context

The `SetupSigningServer` screen and `SetupAdditionalServerKey` screen handle TOTP 2FA setup
for the Server Key (assisted signing key). When the screen mounts it calls
`SigningServer.register(policy)` to obtain a TOTP secret (`verifier`) and then renders a
QR code and a copyable key for the user to add to their authenticator app.

Two independent failure paths produce the observed bugs:

1. **Silent error swallowing in `SigningServer.ts`**: Every method has the pattern:
```ts
try { res = await RestClient.post(...) }
catch (err) {
if (err.response) throw new Error(err.response.data.err);
if (err.code) throw new Error(err.code);
}
const { ... } = res.data; // ← res is undefined if neither branch threw
```
When an axios error carries neither `.response` nor `.code` (e.g. network timeout,
SSL failure on some iOS versions, request cancellation), the catch block exits silently,
`res` stays `undefined`, and the subsequent `res.data` access throws a secondary
TypeError. The exact error message depends on which property is accessed first; in the
Server Key registration flow this surfaces as the "Cannot read properties of undefined
(reading 'model')" toast observed on iOS.

2. **Unsafe property access + wrong empty check in `SetupSigningServer.tsx`**: The line
`setValidationKey(setupData.verification.verifier)` throws if `setupData.verification`
is `null` or `undefined` (server returned malformed data). The loading guard
`validationKey === ''` doesn't catch `undefined` or `null`, so the QR renderer is
entered with an invalid value when the guard fails, which can cause QR generation
errors.

3. **No QR render-error handling in `KeeperQRCode`**: `react-native-qrcode-svg` accepts an
`onError` callback; without it any generation failure is re-thrown uncaught during
rendering.

## Goals / Non-Goals

**Goals:**
- Fix all catch blocks in `SigningServer.ts` so that errors always rethrow with a useful
message.
- Add an error state to `SetupSigningServer` so the user sees a meaningful message and
a retry button when registration fails instead of a stuck spinner.
- Use safe optional chaining (`?.`) when reading nested API response fields.
- Change the loading guard from `=== ''` to `!validationKey` in both setup screens.
- Forward an `onError` prop through `KeeperQRCode` so callers can handle render failures.

**Non-Goals:**
- Refactoring `SigningServer.ts` to a saga-based pattern.
- Adding retry logic inside `SigningServer.ts` itself.
- Changing the API contract or response shape.
- Fixing unrelated screens or components.

## Decisions

### D1 — Safe rethrow pattern for all axios catch blocks
**Decision**: Replace `if (err.response) throw new Error(err.response.data.err); if (err.code)
throw new Error(err.code);` with
`throw new Error(err.response?.data?.err || err.code || err.message || err.toString())`.

**Rationale**: The stored memory for this codebase documents this exact safe pattern. The
single throw ensures the catch block always propagates an error so that `res.data` is never
reached on an undefined `res`.

**Alternative considered**: Wrap `res.data` in its own null-check. Rejected — it would mask
the underlying error and require duplicating null-checks in every method.

### D2 — Add `registrationError` state to `SetupSigningServer`
**Decision**: Add a boolean `registrationError` state. When `registerSigningServer` throws,
set `registrationError = true` and show an error message with a "Retry" button instead of
the spinner.

**Rationale**: The current UX leaves the user with a permanently spinning loader and a
cryptic toast. A retry button aligns with the pattern used elsewhere in the app.

### D3 — Optional chaining for nested API fields
**Decision**: Use `setupData?.verification?.verifier ?? ''` instead of
`setupData.verification.verifier`.

**Rationale**: Server responses can change; a malformed response should fall back to an
empty string (triggering the spinner/error state) rather than crash the catch block with a
secondary TypeError.

### D4 — `onError` prop in `KeeperQRCode`
**Decision**: Accept `onError?: (error: Error) => void` in `KeeperQRCode` and pass it to
the underlying `<QRCode>` component.

**Rationale**: `react-native-qrcode-svg` already supports `onError`; exposing it at the
`KeeperQRCode` level lets callers handle generation failures (e.g. log and fall back to
manual key only) without crashing the render tree.

## Risks / Trade-offs

- [Risk] Changing all `SigningServer.ts` catch blocks touches many methods in a critical
service file. → **Mitigation**: The change is purely mechanical (same substitution
throughout); every changed line makes the behavior strictly safer.
- [Risk] Adding `onError` to `KeeperQRCode` silently hides QR failures from callers that
do not provide the callback. → **Mitigation**: Without an `onError` the underlying
library already re-throws; we change nothing for existing call sites that don't pass
`onError`.

## Affected Files

| File | Change type |
|---|---|
| `src/services/backend/SigningServer.ts` | Modify catch blocks |
| `src/screens/SigningDevices/SetupSigningServer.tsx` | Add error state, fix guards |
| `src/screens/SigningDevices/SetupAdditionalServerKey.tsx` | Fix `!validationKey` guard |
| `src/components/KeeperQRCode.tsx` | Add `onError` prop |
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
## Why

During Server Key setup on iOS, the "Set up 2FA for Server Key" screen shows an error
toast ("Cannot read properties of undefined (reading 'model')"), the QR code fails to
render or remains blank, and sometimes only the manual key is shown. The root cause is a
combination of broken error handling in `SigningServer.ts` (all catch blocks can silently
swallow errors, causing `res.data` to be accessed on an `undefined` `res`), unsafe nested
property access in `SetupSigningServer.tsx`, no error-state for the loading spinner, and
missing QR code render-error handling.

## What Changes

- **`src/services/backend/SigningServer.ts`**: Apply the safe rethrow pattern to all 12+
method catch blocks (use `err.response?.data?.err || err.code || err.message ||
err.toString()`). This prevents silent swallowing of axios errors and prevents secondary
TypeErrors when `res` is `undefined` after a failed request.
- **`src/screens/SigningDevices/SetupSigningServer.tsx`**: Change `validationKey === ''`
guard to `!validationKey` (handles `undefined`/`null` too); use optional chaining
`setupData?.verification?.verifier` for safe property access; add a loading/error state
so the screen shows meaningful feedback (retry button) when registration fails instead of
permanently showing a spinner.
- **`src/screens/SigningDevices/SetupAdditionalServerKey.tsx`**: Same guard change
(`!validationKey`) to keep parity.
- **`src/components/KeeperQRCode.tsx`**: Accept an optional `onError` callback prop and
pass it through to the underlying `QRCode` component from `react-native-qrcode-svg`, so
QR generation failures on iOS are handled gracefully rather than crashing the component.

## Capabilities

### New Capabilities
_(None — this is a pure bug-fix change.)_

### Modified Capabilities
- `signing-server-2fa-setup`: The 2FA QR-code setup flow now handles registration errors
and QR render failures gracefully instead of crashing or showing a stuck spinner.

## Impact

- **Affected code**: `src/services/backend/SigningServer.ts`,
`src/screens/SigningDevices/SetupSigningServer.tsx`,
`src/screens/SigningDevices/SetupAdditionalServerKey.tsx`,
`src/components/KeeperQRCode.tsx`
- **Environments**: Affects both mainnet and testnet (error handling applies to both).
- **Hardware signers**: N/A — this change is in the assisted-key (Server Key) path only.
- **Subscription tiers**: N/A — the Server Key 2FA setup screen is available to all tiers
that support the Server Key.
- **Security/privacy**: The fix tightens error handling; no key material is exposed by the
changes. Error messages surfaced to the user remain generic.
- **Non-goals**: We are not refactoring the full `SigningServer.ts` API surface; we are
only fixing the broken catch-block pattern. We are not adding analytics/logging beyond
what already exists.
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Signing Server 2FA Setup — Error Handling

## Overview

The Server Key 2FA setup flow (`SetupSigningServer` screen) must handle registration
failures and QR code rendering failures gracefully, providing the user with actionable
feedback and a path to retry.

## Requirements

### REQ-1 — Registration error always propagates
**Given** a call to `SigningServer.register(policy)` fails for any reason (network error,
server error, timeout, SSL failure),
**When** the axios catch block is entered,
**Then** an error with a human-readable message is always rethrown so the caller receives
exactly one, meaningful exception.

### REQ-2 — Screen shows error state on registration failure
**Given** `registerSigningServer()` throws,
**When** the error is caught in the screen,
**Then** the spinner is replaced by an error message, and a "Retry" button allows the user
to re-attempt registration without navigating away.

### REQ-3 — Safe nested property access for API response
**Given** the server returns a registration response where `setupData.verification` may be
absent or `null`,
**When** the `verifier` property is read,
**Then** optional chaining is used so that no secondary TypeError is thrown; missing data
falls back to an empty string, keeping the spinner/error state visible.

### REQ-4 — Loading guard handles all falsy validation keys
**Given** `validationKey` can be `''`, `undefined`, or `null` (before the server response
arrives or when registration fails),
**When** the component renders,
**Then** `!validationKey` is used as the guard condition instead of `validationKey === ''`
so that the QR code is never rendered with a falsy key.

### REQ-5 — QR render errors handled gracefully
**Given** the QR code generation fails at render time (e.g. library issue on iOS),
**When** `KeeperQRCode` is given an `onError` callback,
**Then** the error is forwarded to that callback instead of crashing the component tree.

## Acceptance Scenarios

### SC-1 — Network failure during registration
1. Device has no network connectivity.
2. User navigates to "Set up 2FA for Server Key".
3. **Expected**: Spinner shows briefly, then transitions to an error message ("Unable to
register server key") + "Retry" button. No toast with an internal TypeError message.

### SC-2 — Successful registration
1. Network is available and server returns a valid `verifier`.
2. User navigates to "Set up 2FA for Server Key".
3. **Expected**: Spinner shows briefly, then the TOTP QR code and manual key are both
displayed. No error toast.

### SC-3 — QR code render failure (iOS)
1. Registration succeeds but the QR code library fails to render.
2. **Expected**: Error is caught by `onError`; user still sees the manual key and can
proceed to validate manually. No unhandled crash.
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
## Analysis

Root causes of the 2FA setup regression on iOS:

1. **Silent error swallowing** — All catch blocks in `SigningServer.ts` skip rethrowing
when `err.response` and `err.code` are both absent (network timeout, SSL failure).
`res` stays `undefined`; the next line (`res.data`) throws a secondary TypeError whose
property name depends on the runtime path ("model", "data", etc.).

2. **Unsafe nested access** — `setupData.verification.verifier` throws if
`setupData.verification` is `null`/`undefined`.

3. **Wrong guard condition** — `validationKey === ''` doesn't catch `undefined`/`null`,
causing the QR renderer to run with a falsy key.

4. **No QR render-error handling** — `KeeperQRCode` doesn't forward `onError` to the
underlying `react-native-qrcode-svg`, so iOS render failures crash the component tree.

---

## Tasks

- [x] **T1 — Fix catch blocks in `SigningServer.ts`**
Apply `throw new Error(err.response?.data?.err || err.code || err.message || err.toString())` to every axios catch block in `src/services/backend/SigningServer.ts` (methods: `register`, `validate`, `addSecondaryVerificationOption`, `removeSecondaryVerificationOption`, `fetchSignerSetup`, `updateBackupSetting`, `fetchBackup`, `updatePolicy`, `signPSBT`, `fetchSignedDelayedTransaction`, `cancelDelayedTransaction`, `fetchDelayedPolicyUpdate`, `checkSignerHealth`, `migrateSignerPolicy`).

- [x] **T2 — Add `onError` prop to `KeeperQRCode`**
In `src/components/KeeperQRCode.tsx` accept an optional `onError?: (error: Error) => void` prop and pass it to `<QRCode onError={onError} .../>`.

- [x] **T3 — Fix `SetupSigningServer.tsx`**
- Change `validationKey === ''` guard to `!validationKey`.
- Use `setupData?.verification?.verifier ?? ''` when calling `setValidationKey`.
- Add a `registrationError` boolean state; when `registerSigningServer` fails, set `registrationError = true`.
- When `registrationError` is true, render an error message and a "Retry" button (calls `registerSigningServer` again) instead of the spinner.
- Pass `onError` to `KeeperQRCode` that logs the error (or shows toast) so QR render failures are surfaced.

- [x] **T4 — Fix `SetupAdditionalServerKey.tsx`**
Change `validationKey === ''` guard to `!validationKey` for parity with T3.
3 changes: 3 additions & 0 deletions src/components/KeeperQRCode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ function KeeperQRCode({
ecl = 'L',
logoBackgroundColor,
showLogo = false,
onError,
}: {
qrData: any;
size: number;
ecl?: 'L' | 'M' | 'Q' | 'H';
logoBackgroundColor?: string;
showLogo?: boolean;
onError?: (error: Error) => void;
}) {
const { colorMode } = useColorMode();
const themeMode = useSelector((state: any) => state?.settings?.themeMode);
Expand All @@ -36,6 +38,7 @@ function KeeperQRCode({
ecl={ecl}
logo={showLogo ? (privateTheme ? KeeperPrivateNewLogo : KeeperNewLogo) : undefined}
logoSize={size * 0.2}
onError={onError}
/>
)}
</Box>
Expand Down
2 changes: 1 addition & 1 deletion src/screens/SigningDevices/SetupAdditionalServerKey.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function SetupAdditionalServerKey({ route }: { route }) {
/>
</Box>
<Box>
{validationKey === '' ? (
{!validationKey ? (
<Box height={hp(200)} justifyContent="center">
<ActivityIndicator animating size="small" />
</Box>
Expand Down
28 changes: 24 additions & 4 deletions src/screens/SigningDevices/SetupSigningServer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,24 @@ function SetupSigningServer({ route }: { route }) {
const [setupData, setSetupData] = useState(null);
const [validationKey, setValidationKey] = useState('');
const [isSetupValidated, setIsSetupValidated] = useState(false);
const [registrationError, setRegistrationError] = useState(false);
const { addSignerFlow } = route.params;

const registerSigningServer = async () => {
setRegistrationError(false);
try {
const { policy } = route.params;
const { setupData } = await SigningServer.register(policy);
const verifier = setupData?.verification?.verifier;
if (!verifier) {
setRegistrationError(true);
showToast('Server key registration failed: missing verification key', <ToastErrorIcon />);
return;
}
setSetupData(setupData);
setValidationKey(setupData.verification.verifier);
setValidationKey(verifier);
} catch (err) {
setRegistrationError(true);
showToast(err.message || err.toString(), <ToastErrorIcon />);
}
};
Expand Down Expand Up @@ -183,9 +192,17 @@ function SetupSigningServer({ route }: { route }) {
<WalletHeader title={signingServer.setupServer2FATitle} />
</Box>
<Box>
{validationKey === '' ? (
<Box height={hp(200)} justifyContent="center">
<ActivityIndicator animating size="small" />
{!validationKey ? (
<Box height={hp(200)} justifyContent="center" alignItems="center">
{registrationError ? (
<Buttons
primaryCallback={registerSigningServer}
primaryText={common.retry}
fullWidth
/>
) : (
<ActivityIndicator animating size="small" />
)}
</Box>
) : (
<Box
Expand All @@ -204,6 +221,9 @@ function SetupSigningServer({ route }: { route }) {
logoBackgroundColor="transparent"
size={wp(200)}
showLogo
onError={(error) => {
showToast(error.message || error.toString(), <ToastErrorIcon />);
}}
/>
</Box>
<Box>
Expand Down
Loading