Adjust Sentry Keychain errors logging and handling

[leofelix077]

Coming from #853 discussion

Implementation Ticket: Preserve Keychain Error Messages and Improve Error Handling

Parent SPIKE: #853 — "Failed to set key" errors discard the underlying iOS Keychain reason
Sentry Issue: FREIGHTER-MOBILE-R8 — 56 events / 30 users
Related: #852 (FREIGHTER-MOBILE-R9 — encryption type-safety bug, same import-wallet flow)

---

Context

The SPIKE confirmed that ReactNativeKeychainFacade.setKey swallows the native iOS/Android Keychain error in its catch block, replacing it with "Failed to set key 0.xxx". This single generic message groups all Keychain failures together in Sentry, making diagnosis impossible.

Additionally, auth store error catches pass raw technical error messages directly into user-facing UI state (set({ error: error.message })). After fixing the Facade, those raw messages would become "Failed to write to keychain: User interaction is not allowed." in the user's face — still wrong.

Safety confirmed by full native-layer audit: iOS messageForError and Android CryptoFailedException only surface sanitized system strings; no stored data, passwords, or secrets are ever echoed back.

---

Implementation Path

Change 1 — src/helpers/keyManager/ReactNativeKeychainFacade.ts

1a. setKey — lines 110–112 (the core bug)

Current:

} catch (error) {
  throw new Error(`Failed to set key ${id}`);
}

After:

} catch (error) {
  const cause = error instanceof Error ? error.message : String(error);
  throw new Error(`Keychain write rejected: ${cause}`);
}

Why this way:

• Drops the key ID from the thrown message — the Math.random() suffix (0.195…) made every Sentry event unique, preventing grouping.
• Does NOT call logger.error here. The store catch upstream (useAuthenticationStore.importWallet) already calls logger.error with the rethrown error — that is the single Sentry event. The technical cause is preserved in the error message and goes to Sentry there.
• The numeric error code (OSStatus) is not carried through. The human-readable cause string ("User interaction is not allowed.") is sufficient for Sentry grouping and diagnosis.

1b. getKey — lines 84–95

Already logs and returns null. Update the logger context name for consistency (currently misnamed ReactNativeKeychainKeyStore):

Current:

} catch (error) {
  logger.error(
    "ReactNativeKeychainKeyStore.getKey",
    `Error getting key ${id.slice(0, 8)}...:`,
    error,
  );
  return null;
}

After:

} catch (error) {
  logger.error(
    "ReactNativeKeychainFacade.getKey",
    `Keychain read failed for key ${id.slice(0, 8)}...:`,
    error,
  );
  return null;
}

Note: getKey intentionally does not throw — ReactNativeKeychainKeyStore.loadKey (line 151) converts a null return to Promise.reject(id), so the failure does propagate up. Keeping the non-throwing pattern here is correct.

1c. removeKey — lines 123–131

Already best-effort (no throw). Update the logger context name for consistency:

Current:

} catch (error) {
  logger.error(
    "ReactNativeKeychainKeyStore.removeKey",
    `Error removing key ${id.slice(0, 8)}...:`,
    error,
  );
  // Don't throw here, as the key might not exist
}

After:

} catch (error) {
  logger.error(
    "ReactNativeKeychainFacade.removeKey",
    `Keychain remove failed for key ${id.slice(0, 8)}...:`,
    error,
  );
}

---

Change 2 — src/ducks/auth.ts: Stop passing raw error messages to user-facing state

There are four store catch blocks that follow the same anti-pattern:

error instanceof Error ? error.message : t("authStore.error.someKey")

This means if any error has a .message, it gets shown to the user verbatim. Before Change 1, this only surfaced generic-but-safe strings like "Failed to set key 0.xxx". After Change 1, it would surface "Keychain write rejected: User interaction is not allowed." — still raw and technical.

All four i18n keys already exist in src/i18n/locales/en/translations.json. The fix is to always use them for user-facing state, and rely on the existing logger.error call in each catch to send the technical detail to Sentry.

2a. importWallet — lines 2537–2547

Current:

logger.error(
  "useAuthenticationStore.importWallet",
  "Import wallet failed",
  error,
);
set({
  error:
    error instanceof Error
      ? error.message
      : t("authStore.error.failedToImportWallet"),
  isLoading: false,
});

After:

logger.error(
  "useAuthenticationStore.importWallet",
  "Import wallet failed",
  error,
);
set({
  error: t("authStore.error.failedToImportWallet"),
  isLoading: false,
});

2b. signUp — lines 2110–2118

Current:

error instanceof Error
  ? error.message
  : t("authStore.error.failedToSignUp"),

After:

t("authStore.error.failedToSignUp"),

2c. signIn — lines 2184–2191

Current:

error instanceof Error
  ? error.message
  : t("authStore.error.failedToSignIn"),

After:

t("authStore.error.failedToSignIn"),

2d. logout — lines 2080–2087

Current:

error instanceof Error
  ? error.message
  : t("authStore.error.failedToLogout"),

After:

t("authStore.error.failedToLogout"),

Important: In all four cases, the logger.error call immediately above already sends the technical error (including the now-preserved keychain cause) to Sentry. The ternary was redundant — if a message was available, logger.error already captured it. The i18n string has always been the right user-facing copy.

---

Change 3 — Biometric login: verify existing toast covers blocking keychain errors

The current flow in src/hooks/useAppOpenBiometricsLogin.ts (lines 37–49):

verifyActionWithBiometrics(async (biometricPassword?: string) => {
  if (biometricPassword) {
    await signIn({ password: biometricPassword });
  }
}).catch(() => {
  showToast({
    toastId: "unlock-wallet-error",
    variant: "error",
    title: t("lockScreen.errorUnlockingWalletTitle"),
    message: t("lockScreen.errorUnlockingWalletMessage"),
    duration: 6000,
  });
});

The .catch() already handles any error thrown from signIn — including propagated keychain errors. No change needed here for the happy path.

However, verifyActionWithBiometrics in auth.ts (lines 2506–2512) catches and re-throws with only set({ isLoading: false }) — it does NOT call logger.error, so a keychain read failure during biometric unlock is not captured in Sentry. Add the logger call:

Locate: verifyActionWithBiometrics inner catch in auth.ts (grep for "Biometric authentication failed").

After:

} catch (error) {
  logger.error(
    "verifyActionWithBiometrics",
    "Biometric authentication failed",
    error,
  );
  set({ isLoading: false });
  throw error;
}

(This logger call may already exist — verify before adding.)

---

What Does NOT Change

• hasKey — swallows errors and returns false. This is correct: it's a probe, not a critical read.
• removeKey swallowing errors — intentional, keys may not exist.
• getAllKeys / addToKeyIndex / removeFromKeyIndex error handling — all already log properly.
• The toast in useAppOpenBiometricsLogin — already surfaces errors to users in the biometric flow.
• secureStorage.ts — separate from the keychain facade, out of scope for this ticket.

---

Expected Sentry Outcome After These Changes

Sentry events for FREIGHTER-MOBILE-R8 will split into distinct groups:

New Sentry title	Likely cause
Keychain write rejected: User interaction is not allowed.	Background/locked device race (errSecInteractionNotAllowed)
Keychain write rejected: Failed to allocate memory.	Low-memory device (errSecAllocate)
Keychain write rejected: No keychain is available. You may need to restart your device or enable a passcode.	Device passcode not set or keychain corrupt (errSecNotAvailable)
Keychain write rejected: The user name or passphrase you entered is not correct.	Biometric auth failure during write (errSecAuthFailed)

Each is a separate Sentry group with a separate root cause and separate remediation path. A follow-up issue will be opened once 2–3 weeks of post-fix data accumulates.

---

Acceptance Criteria

• setKey catch preserves the native error message in the thrown error, without the key ID.
• setKey does NOT call logger.error — the single Sentry event fires at the store catch level.
• getKey and removeKey logger context names are corrected to ReactNativeKeychainFacade.*.
• The user-facing error state in importWallet, signIn, signUp, and logout store actions always uses the i18n string — never error.message directly.
• Each store catch still calls logger.error so the technical cause reaches Sentry.
• Sentry events for keychain failures group by native cause, not by Math.random() key ID.
• No PII, stored secrets, or encrypted data appears in error messages or Sentry payloads.
• Biometric login flow still shows a toast on blocking errors (verify no regression).
• logger.error is called in verifyActionWithBiometrics if not already present.

---

Open Decision (need sign-off before implementation)

1. Follow-up trigger — No concrete criteria for when to open the UX unification follow-up. Suggest: re-evaluate after 4 weeks of post-fix Sentry data.