From 4859f31e29ba5188f51c3b8c4523533b7619c85c Mon Sep 17 00:00:00 2001 From: Asher Cohen Date: Fri, 24 Jul 2026 08:55:33 -0700 Subject: [PATCH 1/2] fix(mobile): report handled errors Closes #1795 --- package.json | 3 +- packages/mobile/app/ble-probe.test.tsx | 28 ++- packages/mobile/app/ble-probe.tsx | 16 +- .../app/heart-rate-visualization.test.tsx | 23 +- .../mobile/app/heart-rate-visualization.tsx | 10 +- packages/mobile/app/imu-visualization.tsx | 7 +- packages/mobile/app/login.test.tsx | 22 +- packages/mobile/app/login.tsx | 18 +- .../mobile/app/providers/auth-modals.test.tsx | 59 +++++ packages/mobile/app/providers/auth-modals.tsx | 8 + .../mobile/components/DataExportSection.tsx | 3 +- packages/mobile/lib/apple-health-provider.ts | 6 +- packages/mobile/lib/auth.test.ts | 12 + packages/mobile/lib/auth.ts | 54 ++++- ...nd-watch-inertial-measurement-unit-sync.ts | 3 +- .../lib/background-whoop-ble-sync.test.ts | 24 +- .../mobile/lib/background-whoop-ble-sync.ts | 16 +- packages/mobile/lib/health-kit-sync.test.ts | 17 +- packages/mobile/lib/health-kit-sync.ts | 9 + .../inertial-measurement-unit-service.test.ts | 12 +- .../lib/inertial-measurement-unit-service.ts | 17 +- packages/mobile/lib/open-external-url.test.ts | 12 +- packages/mobile/lib/open-external-url.ts | 6 +- packages/mobile/lib/trpc-fetch.ts | 8 +- packages/mobile/lib/useHaptic.test.ts | 30 ++- packages/mobile/lib/useHaptic.ts | 18 +- packages/mobile/lib/useWhoopBleSync.ts | 4 +- packages/mobile/package.json | 2 +- packages/mobile/test-setup.ts | 2 + scripts/mobile-catch-telemetry-policy.test.ts | 129 ++++++++++ scripts/mobile-catch-telemetry-policy.ts | 225 ++++++++++++++++++ 31 files changed, 714 insertions(+), 89 deletions(-) create mode 100644 packages/mobile/app/providers/auth-modals.test.tsx create mode 100644 scripts/mobile-catch-telemetry-policy.test.ts create mode 100644 scripts/mobile-catch-telemetry-policy.ts diff --git a/package.json b/package.json index 6358ae2af6..c7aa6360d0 100644 --- a/package.json +++ b/package.json @@ -122,13 +122,14 @@ "schema:diagram": "tsx scripts/generate-schema-diagram.ts", "schema:view": "tsx scripts/generate-schema-diagram.ts --open", "typecheck": "tsc --noEmit", - "lint": "biome check . --max-diagnostics=500 && pnpm lint:workflow-downloads && pnpm lint:analytics-sql && pnpm lint:analytics-policy", + "lint": "biome check . --max-diagnostics=500 && pnpm lint:workflow-downloads && pnpm lint:analytics-sql && pnpm lint:analytics-policy && pnpm lint:mobile-telemetry", "lint:fix": "biome check --write .", "lint:openapi": "redocly lint docs/whoop-api.openapi.yaml --extends minimal", "lint:css": "stylelint \"**/*.css\"", "lint:workflow-downloads": "tsx scripts/workflow-download-policy.ts .github/workflows .github/actions", "lint:analytics-sql": "sh -c 'set -a; [ ! -f .env.local ] || . ./.env.local; set +a; cd analytics && UV_PROJECT_ENVIRONMENT=../.venv-analytics uv run --project . sqlfluff lint --ignore parsing models'", "lint:analytics-policy": "sh -c 'files=$(find analytics src/db/clickhouse-sql -path \"analytics/target\" -prune -o -path \"analytics/logs\" -prune -o -path \"analytics/dbt_packages\" -prune -o -name \"*.sql\" -type f -print 2>/dev/null); if [ -z \"$files\" ]; then echo \"No analytics SQL files to lint\"; else tsx scripts/migration-policy.ts $files; fi'", + "lint:mobile-telemetry": "tsx scripts/mobile-catch-telemetry-policy.ts packages/mobile", "lint:migrations": "sh -c 'files=$(git diff --diff-filter=AM --name-only origin/main...HEAD -- \"drizzle/*.sql\" \":(exclude)drizzle/_history/*.sql\"); if [ -z \"$files\" ]; then echo \"No changed migrations to lint\"; else tsx scripts/migration-policy.ts $files; fi'", "sherif": "sherif", "format": "biome format --write .", diff --git a/packages/mobile/app/ble-probe.test.tsx b/packages/mobile/app/ble-probe.test.tsx index 11018bec80..889585bc9b 100644 --- a/packages/mobile/app/ble-probe.test.tsx +++ b/packages/mobile/app/ble-probe.test.tsx @@ -3,7 +3,14 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const { writeRawMock } = vi.hoisted(() => ({ writeRawMock: vi.fn() })); +const { mockCaptureException, writeRawMock } = vi.hoisted(() => ({ + mockCaptureException: vi.fn(), + writeRawMock: vi.fn(), +})); + +vi.mock("../lib/telemetry", () => ({ + captureException: mockCaptureException, +})); vi.mock("../modules/ble-probe", () => ({ addNotificationListener: () => ({ remove: vi.fn() }), @@ -57,6 +64,25 @@ describe("BleProbeScreen", () => { expect(writeRawMock).toHaveBeenCalledTimes(1); }); + it("reports native command failures while keeping them in the on-screen log", async () => { + const writeError = new Error("Native write failed"); + writeRawMock.mockRejectedValueOnce(writeError); + const { default: BleProbeScreen } = await import("./ble-probe"); + render(); + + fireEvent.click( + screen.getByRole("button", { + name: "Send hello and toggle inertial measurement unit (IMU) mode", + }), + ); + + await waitFor(() => { + expect(mockCaptureException).toHaveBeenCalledWith(writeError, { + source: "ble-probe-hello-imu", + }); + }); + }); + it("disables Send when the command is blank", async () => { const { default: BleProbeScreen } = await import("./ble-probe"); render(); diff --git a/packages/mobile/app/ble-probe.tsx b/packages/mobile/app/ble-probe.tsx index c98ccc028a..9778515bd2 100644 --- a/packages/mobile/app/ble-probe.tsx +++ b/packages/mobile/app/ble-probe.tsx @@ -123,6 +123,7 @@ export default function BleProbeScreen() { break; } } catch (error) { + captureException(error, { source: "ble-probe-whoop-discovery" }); addLog(`whoop-ble module error: ${error}`, "error"); } // Fall back to ble-probe module's own scan @@ -150,14 +151,20 @@ export default function BleProbeScreen() { await subscribe("0003"); addLog(" 0003 subscribed"); } catch (error: unknown) { - captureException(error, { context: "ble-probe-subscribe" }); + captureException(error, { + source: "ble-probe-subscribe", + characteristic: "0003", + }); addLog(" 0003 failed"); } try { await subscribe("0005"); addLog(" 0005 subscribed"); } catch (error: unknown) { - captureException(error, { context: "ble-probe-subscribe" }); + captureException(error, { + source: "ble-probe-subscribe", + characteristic: "0005", + }); addLog(" 0005 failed"); } addLog("Ready!"); @@ -263,7 +270,7 @@ export default function BleProbeScreen() { addLog(`whoop-ble isNotifying: ${stats.isNotifying}`); addLog(`whoop-ble buffered: ${whoopBle.getBufferedSampleCount()}`); } catch (error: unknown) { - captureException(error, { context: "ble-probe-whoop" }); + captureException(error, { source: "ble-probe-whoop-status" }); addLog("whoop-ble module not available", "error"); } break; @@ -295,6 +302,7 @@ export default function BleProbeScreen() { addLog(`Unknown command: ${cmd}. Type 'help'.`, "error"); } } catch (error) { + captureException(error, { source: "ble-probe-command" }); addLog(`Error: ${error instanceof Error ? error.message : String(error)}`, "error"); } }, @@ -329,7 +337,7 @@ export default function BleProbeScreen() { await writeRaw("0002", "aa010c000001e74123026a01010000001cc9f7a9", false); addLog("TOGGLE_IMU_MODE sent — watching for response...", "info"); } catch (error: unknown) { - captureException(error, { context: "ble-probe-hello-imu" }); + captureException(error, { source: "ble-probe-hello-imu" }); addLog(`Error: ${error}`, "error"); } finally { helloAndImuBusyRef.current = false; diff --git a/packages/mobile/app/heart-rate-visualization.test.tsx b/packages/mobile/app/heart-rate-visualization.test.tsx index 222a7d47dc..209f212182 100644 --- a/packages/mobile/app/heart-rate-visualization.test.tsx +++ b/packages/mobile/app/heart-rate-visualization.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { act, render, screen } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import React from "react"; import { describe, expect, it, vi } from "vitest"; @@ -98,6 +98,27 @@ describe("HeartRateVisualizationScreen", () => { expect(screen.getByText("Connecting to WHOOP...")).toBeTruthy(); }); + it("reports connection failures while preserving the visible error", async () => { + const whoopBle = await import("../modules/whoop-ble"); + const connectionError = new Error("Bluetooth connection failed"); + vi.spyOn(whoopBle, "findWhoop").mockResolvedValue({ + id: "whoop-1", + name: "WHOOP", + }); + vi.spyOn(whoopBle, "connect").mockRejectedValue(connectionError); + const { captureException } = await import("../lib/telemetry"); + const { default: HeartRateVisualizationScreen } = await import("./heart-rate-visualization"); + + render(); + + await waitFor(() => { + expect(screen.getByText("Bluetooth connection failed")).toBeTruthy(); + }); + expect(captureException).toHaveBeenCalledWith(connectionError, { + source: "heart-rate-visualization-connect", + }); + }); + it("starts in streaming state when BLE is already connected", async () => { const whoopBle = await import("../modules/whoop-ble"); vi.spyOn(whoopBle, "getConnectionState").mockReturnValue("streaming"); diff --git a/packages/mobile/app/heart-rate-visualization.tsx b/packages/mobile/app/heart-rate-visualization.tsx index 89d9ed5e8a..0b0071bc90 100644 --- a/packages/mobile/app/heart-rate-visualization.tsx +++ b/packages/mobile/app/heart-rate-visualization.tsx @@ -87,7 +87,7 @@ export default function HeartRateVisualizationScreen() { setSampleCount((previous) => previous + newSamples.length); setHeartRateHistory((previous) => [...previous, ...newHeartRates].slice(-MAX_SAMPLES)); } catch (pollError) { - captureException(pollError, { context: "heart-rate-visualization-poll" }); + captureException(pollError, { source: "heart-rate-visualization-poll" }); } }, POLL_INTERVAL_MS); }, [stopPolling]); @@ -97,7 +97,10 @@ export default function HeartRateVisualizationScreen() { if (isAlreadyConnected()) { try { await startRealtimeHr(); - } catch { + } catch (startError: unknown) { + captureException(startError, { + source: "heart-rate-visualization-start-realtime", + }); // Best-effort — passive HR data may still flow } setStatus("streaming"); @@ -121,6 +124,9 @@ export default function HeartRateVisualizationScreen() { setStatus("streaming"); startPolling(); } catch (connectionError) { + captureException(connectionError, { + source: "heart-rate-visualization-connect", + }); setError( connectionError instanceof Error ? connectionError.message : String(connectionError), ); diff --git a/packages/mobile/app/imu-visualization.tsx b/packages/mobile/app/imu-visualization.tsx index ba18a47b10..6e9a3837de 100644 --- a/packages/mobile/app/imu-visualization.tsx +++ b/packages/mobile/app/imu-visualization.tsx @@ -2,6 +2,7 @@ import { Stack } from "expo-router"; import { useCallback, useEffect, useRef, useState } from "react"; import { ScrollView, StyleSheet, Text, View } from "react-native"; import { WristModel } from "../components/WristModel"; +import { captureException } from "../lib/telemetry"; import type { OrientationEvent } from "../modules/whoop-ble"; import { addConnectionStateListener, @@ -80,7 +81,10 @@ export default function ImuVisualizationScreen() { // Best-effort: ensure IMU mode is on (background sync should have done this) try { await startImuStreaming(); - } catch { + } catch (startError: unknown) { + captureException(startError, { + source: "imu-visualization-start-streaming", + }); // Ignore — background sync likely already started it } setStatus("streaming"); @@ -102,6 +106,7 @@ export default function ImuVisualizationScreen() { await startImuStreaming(); setStatus("streaming"); } catch (connectionError) { + captureException(connectionError, { source: "imu-visualization-connect" }); setError( connectionError instanceof Error ? connectionError.message : String(connectionError), ); diff --git a/packages/mobile/app/login.test.tsx b/packages/mobile/app/login.test.tsx index b9c17a04e5..b9ff555593 100644 --- a/packages/mobile/app/login.test.tsx +++ b/packages/mobile/app/login.test.tsx @@ -1,6 +1,10 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; +const { mockCaptureException } = vi.hoisted(() => ({ + mockCaptureException: vi.fn(), +})); + // Mock auth module before importing LoginScreen const mockOnLoginSuccess = vi.fn(); const mockFetchConfiguredProviders = vi.fn(); @@ -48,6 +52,10 @@ vi.mock("../components/ProviderLogo", () => ({ ProviderLogo: () => null, })); +vi.mock("../lib/telemetry", () => ({ + captureException: mockCaptureException, +})); + const { default: LoginScreen } = await import("./login"); describe("LoginScreen", () => { @@ -134,12 +142,16 @@ describe("LoginScreen", () => { }); it("shows error message on fetch failure", async () => { - mockFetchConfiguredProviders.mockRejectedValue(new Error("Network error")); + const providerDiscoveryError = new Error("Network error"); + mockFetchConfiguredProviders.mockRejectedValue(providerDiscoveryError); render(); await waitFor(() => { expect(screen.getByText("Network error")).toBeTruthy(); }); + expect(mockCaptureException).toHaveBeenCalledWith(providerDiscoveryError, { + source: "login-screen-configured-providers", + }); }); it("shows empty state when no providers configured", async () => { @@ -344,9 +356,7 @@ describe("LoginScreen", () => { data: [], nativeApple: true, }); - const cancelError = new Error("User canceled"); - Object.assign(cancelError, { code: "ERR_REQUEST_CANCELED" }); - mockStartNativeAppleSignIn.mockRejectedValue(cancelError); + mockStartNativeAppleSignIn.mockResolvedValue(null); render(); @@ -358,6 +368,10 @@ describe("LoginScreen", () => { }); expect(mockStartOAuthLogin).not.toHaveBeenCalled(); expect(screen.queryByText("User canceled")).toBeNull(); + expect(mockCaptureException).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ source: "login-screen-handle-login" }), + ); }); it("requests a password reset from sign-in mode", async () => { diff --git a/packages/mobile/app/login.tsx b/packages/mobile/app/login.tsx index 87b3bdc29f..9d66d1f945 100644 --- a/packages/mobile/app/login.tsx +++ b/packages/mobile/app/login.tsx @@ -27,15 +27,6 @@ import { colors } from "../theme"; type AuthMode = "login" | "register" | "reset"; -function hasCancelCode( - err: unknown, -): err is { code: "ERR_REQUEST_CANCELED" | "ERR_CANCELED"; message?: string } { - if (!err || typeof err !== "object" || !("code" in err)) { - return false; - } - return err.code === "ERR_REQUEST_CANCELED" || err.code === "ERR_CANCELED"; -} - export default function LoginScreen() { const { serverUrl, onLoginSuccess } = useAuth(); const router = useRouter(); @@ -66,6 +57,7 @@ export default function LoginScreen() { fetchConfiguredProviders(serverUrl) .then(setProviders) .catch((err: unknown) => { + captureException(err, { source: "login-screen-configured-providers" }); setError(err instanceof Error ? err.message : "Failed to load providers"); }) .finally(() => setLoading(false)); @@ -93,14 +85,6 @@ export default function LoginScreen() { } } } catch (err: unknown) { - const isCancel = - (err instanceof Error && - (err.message.includes("ERR_CANCELED") || err.message.includes("ERR_REQUEST_CANCELED"))) || - hasCancelCode(err); - - if (isCancel) { - return; - } captureException(err, { source: "login-screen-handle-login" }); setError(err instanceof Error ? err.message : "Login failed"); } finally { diff --git a/packages/mobile/app/providers/auth-modals.test.tsx b/packages/mobile/app/providers/auth-modals.test.tsx new file mode 100644 index 0000000000..391d7f78c4 --- /dev/null +++ b/packages/mobile/app/providers/auth-modals.test.tsx @@ -0,0 +1,59 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { credentialSignIn, mockCaptureException } = vi.hoisted(() => ({ + credentialSignIn: vi.fn(), + mockCaptureException: vi.fn(), +})); + +vi.mock("../../lib/telemetry", () => ({ + captureException: mockCaptureException, +})); + +vi.mock("../../lib/trpc", () => ({ + trpc: { + credentialAuth: { + signIn: { + useMutation: () => ({ mutateAsync: credentialSignIn }), + }, + }, + }, +})); + +import { CredentialAuthModal } from "./auth-modals"; + +describe("CredentialAuthModal", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("reports the original sign-in error while preserving the actionable message", async () => { + const signInError = new Error("Provider rejected these credentials"); + credentialSignIn.mockRejectedValue(signInError); + + render( + , + ); + + fireEvent.change(screen.getByPlaceholderText("Email"), { + target: { value: "athlete@example.com" }, + }); + fireEvent.change(screen.getByPlaceholderText("Password"), { + target: { value: "secret" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Sign in to Wahoo" })); + + await waitFor(() => { + expect(screen.getByText("Provider rejected these credentials")).toBeTruthy(); + }); + expect(mockCaptureException).toHaveBeenCalledWith(signInError, { + source: "provider-credential-auth-sign-in", + providerId: "wahoo", + }); + }); +}); diff --git a/packages/mobile/app/providers/auth-modals.tsx b/packages/mobile/app/providers/auth-modals.tsx index a52dbff42c..4164ac96b5 100644 --- a/packages/mobile/app/providers/auth-modals.tsx +++ b/packages/mobile/app/providers/auth-modals.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { ActivityIndicator, Modal, Text, TextInput, TouchableOpacity, View } from "react-native"; +import { captureException } from "../../lib/telemetry"; import { trpc } from "../../lib/trpc"; import { colors } from "../../theme"; import { styles } from "./styles.ts"; @@ -36,6 +37,10 @@ export function CredentialAuthModal({ await signInMutation.mutateAsync({ providerId, username, password }); onSuccess(); } catch (err: unknown) { + captureException(err, { + source: "provider-credential-auth-sign-in", + providerId, + }); setError(err instanceof Error ? err.message : "Sign in failed"); } finally { setLoading(false); @@ -131,6 +136,7 @@ export function GarminAuthModal({ await signInMutation.mutateAsync({ username, password }); onSuccess(); } catch (error_: unknown) { + captureException(error_, { source: "provider-garmin-auth-sign-in" }); setError(error_ instanceof Error ? error_.message : "Sign in failed"); } finally { setLoading(false); @@ -248,6 +254,7 @@ export function WhoopAuthModal({ onSuccess(); } } catch (error_: unknown) { + captureException(error_, { source: "provider-whoop-auth-sign-in" }); setError(error_ instanceof Error ? error_.message : "Sign in failed"); } finally { setLoading(false); @@ -265,6 +272,7 @@ export function WhoopAuthModal({ onSuccess(); } } catch (error_: unknown) { + captureException(error_, { source: "provider-whoop-auth-verify" }); setError(error_ instanceof Error ? error_.message : "Verification failed"); } finally { setLoading(false); diff --git a/packages/mobile/components/DataExportSection.tsx b/packages/mobile/components/DataExportSection.tsx index 76a7740193..d432fa8c97 100644 --- a/packages/mobile/components/DataExportSection.tsx +++ b/packages/mobile/components/DataExportSection.tsx @@ -72,7 +72,8 @@ async function getResponseErrorMessage(response: Response, fallback: string): Pr if (typeof parsed.data.error === "string") return parsed.data.error; if (parsed.data.error?.message) return parsed.data.error.message; return parsed.data.message ?? fallback; - } catch { + } catch (error: unknown) { + captureException(error, { source: "data-export-response-error-json" }); return fallback; } } diff --git a/packages/mobile/lib/apple-health-provider.ts b/packages/mobile/lib/apple-health-provider.ts index 5ded7aece0..c176dd0f4c 100644 --- a/packages/mobile/lib/apple-health-provider.ts +++ b/packages/mobile/lib/apple-health-provider.ts @@ -272,13 +272,15 @@ export function useAppleHealthProviderModel( } catch (error) { captureException(error, { source: "apple-health-authorization-refresh" }); authorizationErrorHandlerRef.current?.(error); - throw error; + const unknownState = AppleHealthAuthorizationState.unknown(); + setAuthorizationState(unknownState); + return unknownState; } }, [authorizationService, enabled]); useEffect(() => { if (!enabled) return; - void refreshAuthorizationState().catch(() => undefined); + void refreshAuthorizationState(); }, [enabled, refreshAuthorizationState]); const model = useMemo( diff --git a/packages/mobile/lib/auth.test.ts b/packages/mobile/lib/auth.test.ts index 80b857d9f9..b2b8535061 100644 --- a/packages/mobile/lib/auth.test.ts +++ b/packages/mobile/lib/auth.test.ts @@ -312,6 +312,18 @@ describe("startNativeAppleSignIn", () => { expect(fetch).not.toHaveBeenCalled(); }); + it("models user cancellation as a null result without reporting a defect", async () => { + const cancellationError = Object.assign(new Error("User canceled"), { + code: "ERR_REQUEST_CANCELED", + }); + mockSignInAsync.mockRejectedValueOnce(cancellationError); + + await expect(startNativeAppleSignIn("https://srv")).resolves.toBeNull(); + + expect(fetch).not.toHaveBeenCalled(); + expect(mockCaptureException).not.toHaveBeenCalledWith(cancellationError, expect.anything()); + }); + it("throws when server returns an error", async () => { mockSignInAsync.mockResolvedValueOnce({ user: "apple-user-123", diff --git a/packages/mobile/lib/auth.ts b/packages/mobile/lib/auth.ts index 28e30cf4b8..d31c337d1a 100644 --- a/packages/mobile/lib/auth.ts +++ b/packages/mobile/lib/auth.ts @@ -24,6 +24,33 @@ const ErrorResponseSchema = z.object({ error: z.string().min(1) }); const invalidSessionResponseMessage = "The server returned an invalid session response. Please try again."; +function isNativeAppleSignInCancellation( + error: unknown, +): error is { code: "ERR_REQUEST_CANCELED" } { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ERR_REQUEST_CANCELED" + ); +} + +async function requestNativeAppleCredential() { + try { + return await AppleAuthentication.signInAsync({ + requestedScopes: [ + AppleAuthentication.AppleAuthenticationScope.FULL_NAME, + AppleAuthentication.AppleAuthenticationScope.EMAIL, + ], + }); + } catch (error: unknown) { + if (isNativeAppleSignInCancellation(error)) { + return null; + } + throw error; + } +} + // In-memory cache avoids SecureStore reads while iOS has the device locked in background. // Reads still fall back to SecureStore on cold start in the foreground. let cachedSessionToken: string | null | undefined; @@ -70,7 +97,10 @@ export async function fetchCurrentUser(serverUrl: string, token: string): Promis }); if (res.status === 401 || res.status === 403) return null; if (!res.ok) { - const data: unknown = await res.json().catch(() => null); + const data: unknown = await res.json().catch((error: unknown) => { + captureException(error, { source: "auth-bootstrap-error-json" }); + return null; + }); const parsed = ErrorResponseSchema.safeParse(data); throw new Error( parsed.success ? parsed.data.error : `Auth bootstrap failed: ${res.status} ${res.statusText}`, @@ -112,7 +142,13 @@ async function submitPasswordAuth( body: JSON.stringify(body), }); - const data: unknown = await response.json().catch(() => null); + const data: unknown = await response.json().catch((error: unknown) => { + captureException(error, { + source: "password-auth-response-json", + path, + }); + return null; + }); const parsed = z .object({ session: z.string(), @@ -170,7 +206,10 @@ export async function requestPasswordReset( headers: { "Content-Type": "application/json", Accept: "application/json" }, body: JSON.stringify({ email }), }); - const data: unknown = await response.json().catch(() => null); + const data: unknown = await response.json().catch((error: unknown) => { + captureException(error, { source: "password-reset-response-json" }); + return null; + }); const parsed = PasswordResetResponseSchema.safeParse(data); if (!response.ok) { throw new Error( @@ -270,14 +309,9 @@ export async function isNativeAppleSignInAvailable(): Promise { /** Sign in using the native iOS Apple Sign In sheet. Returns auth result or null if cancelled. */ export async function startNativeAppleSignIn(serverUrl: string): Promise { - const credential = await AppleAuthentication.signInAsync({ - requestedScopes: [ - AppleAuthentication.AppleAuthenticationScope.FULL_NAME, - AppleAuthentication.AppleAuthenticationScope.EMAIL, - ], - }); + const credential = await requestNativeAppleCredential(); - if (!credential.authorizationCode) { + if (!credential?.authorizationCode) { return null; } diff --git a/packages/mobile/lib/background-watch-inertial-measurement-unit-sync.ts b/packages/mobile/lib/background-watch-inertial-measurement-unit-sync.ts index d36b2c41d9..7f3f75831d 100644 --- a/packages/mobile/lib/background-watch-inertial-measurement-unit-sync.ts +++ b/packages/mobile/lib/background-watch-inertial-measurement-unit-sync.ts @@ -109,7 +109,8 @@ async function syncAndRecord(trpcClient: WatchSyncTrpcClient): Promise { // Ask the Watch to restart recording and send any new data try { await requestWatchRecording(); - } catch { + } catch (error: unknown) { + captureException(error, { source: `${TAG}:request-recording` }); // Best-effort — Watch may not be reachable } } diff --git a/packages/mobile/lib/background-whoop-ble-sync.test.ts b/packages/mobile/lib/background-whoop-ble-sync.test.ts index 8f25a8437c..fd66f6e3e4 100644 --- a/packages/mobile/lib/background-whoop-ble-sync.test.ts +++ b/packages/mobile/lib/background-whoop-ble-sync.test.ts @@ -661,8 +661,8 @@ describe("background-whoop-ble-sync", () => { expect(whoopDeps.connect).toHaveBeenCalled(); }); - it("calls Sentry.captureException when foreground sync rejects", async () => { - const { captureException: sentryCaptureException } = await import("@sentry/react-native"); + it("calls the canonical telemetry helper when foreground sync rejects", async () => { + const { captureException } = await import("./telemetry"); // Let init succeed normally first await initBackgroundWhoopBleSync(trpcClient, whoopDeps); @@ -675,22 +675,22 @@ describe("background-whoop-ble-sync", () => { appStateCallback?.("active"); await vi.waitFor(() => { - expect(sentryCaptureException).toHaveBeenCalledWith(syncError, { - tags: { source: "whoop-ble-foreground-sync" }, + expect(captureException).toHaveBeenCalledWith(syncError, { + source: "whoop-ble-foreground-sync", }); }); }); - it("calls Sentry.captureException when init sync rejects", async () => { - const { captureException: sentryCaptureException } = await import("@sentry/react-native"); + it("calls the canonical telemetry helper when init sync rejects", async () => { + const { captureException } = await import("./telemetry"); const initError = new Error("init BLE failure"); vi.mocked(whoopDeps.connect).mockRejectedValue(initError); // Init should not throw await initBackgroundWhoopBleSync(trpcClient, whoopDeps); - expect(sentryCaptureException).toHaveBeenCalledWith(initError, { - tags: { source: "whoop-ble-init-sync" }, + expect(captureException).toHaveBeenCalledWith(initError, { + source: "whoop-ble-init-sync", }); }); }); @@ -771,15 +771,15 @@ describe("syncWhoopBle", () => { expect(whoopDeps.connect).not.toHaveBeenCalled(); }); - it("reports errors to Sentry", async () => { - const { captureException: sentryCaptureException } = await import("@sentry/react-native"); + it("reports errors through the canonical telemetry helper", async () => { + const { captureException } = await import("./telemetry"); const bleError = new Error("BLE error"); vi.mocked(whoopDeps.connect).mockRejectedValue(bleError); await syncWhoopBle(trpcClient, whoopDeps); - expect(sentryCaptureException).toHaveBeenCalledWith(bleError, { - tags: { source: "whoop-ble-background-refresh" }, + expect(captureException).toHaveBeenCalledWith(bleError, { + source: "whoop-ble-background-refresh", }); }); diff --git a/packages/mobile/lib/background-whoop-ble-sync.ts b/packages/mobile/lib/background-whoop-ble-sync.ts index f7fe5df9bd..fb25af0671 100644 --- a/packages/mobile/lib/background-whoop-ble-sync.ts +++ b/packages/mobile/lib/background-whoop-ble-sync.ts @@ -153,7 +153,7 @@ export async function initBackgroundWhoopBleSync( syncOnForeground(trpcClient, whoopDeps, realtimeClient, shouldRunForegroundPeriodicDrain) .catch((error: unknown) => { logger.error(LOG_CATEGORY, `foreground sync error: ${error}`); - Sentry.captureException(error, { tags: { source: "whoop-ble-foreground-sync" } }); + captureException(error, { source: "whoop-ble-foreground-sync" }); }) .finally(() => { syncing = false; @@ -170,7 +170,7 @@ export async function initBackgroundWhoopBleSync( logger.info(LOG_CATEGORY, "initial sync complete"); } catch (error: unknown) { logger.error(LOG_CATEGORY, `initial sync error: ${error}`); - Sentry.captureException(error, { tags: { source: "whoop-ble-init-sync" } }); + captureException(error, { source: "whoop-ble-init-sync" }); } // Periodically drain the buffer while the app is active so samples @@ -210,7 +210,7 @@ function startPeriodicDrainTimer( drainBuffer(trpcClient, whoopDeps, realtimeClient, shouldRunForegroundPeriodicDrain) .catch((error: unknown) => { logger.error(LOG_CATEGORY, `periodic drain error: ${error}`); - Sentry.captureException(error, { tags: { source: "whoop-ble-periodic-drain" } }); + captureException(error, { source: "whoop-ble-periodic-drain" }); }) .finally(() => { syncing = false; @@ -236,7 +236,7 @@ export async function syncWhoopBle( logger.info(LOG_CATEGORY, "background refresh — sync complete"); } catch (error: unknown) { logger.error(LOG_CATEGORY, `background refresh sync error: ${error}`); - Sentry.captureException(error, { tags: { source: "whoop-ble-background-refresh" } }); + captureException(error, { source: "whoop-ble-background-refresh" }); } } @@ -285,6 +285,7 @@ async function syncOnForeground( await whoopDeps.startImuStreaming(); logger.info(LOG_CATEGORY, "TOGGLE_IMU_MODE sent"); } catch (error: unknown) { + captureException(error, { source: "whoop-ble-start-streaming" }); // Best-effort — passive data may still flow without the command logger.warn(LOG_CATEGORY, `startImuStreaming failed (passive data may still work): ${error}`); } @@ -307,8 +308,8 @@ async function syncOnForeground( const stats = bleModule.getDataPathStats(); logger.info(LOG_CATEGORY, `data path stats: ${JSON.stringify(stats)}`); } - } catch { - // Diagnostic-only, ignore errors + } catch (error: unknown) { + captureException(error, { source: "whoop-ble-data-path-stats-connect" }); } await drainBuffer(trpcClient, whoopDeps, realtimeClient, shouldContinueUploading); @@ -474,7 +475,8 @@ export function teardownBackgroundWhoopBleSync(): void { currentDeps.stopImuStreaming().catch((error: unknown) => { captureException(error, { source: "whoop-ble-teardown" }); }); - } catch { + } catch (error: unknown) { + captureException(error, { source: "whoop-ble-teardown-sync" }); // Best-effort cleanup } currentDeps.disconnect(); diff --git a/packages/mobile/lib/health-kit-sync.test.ts b/packages/mobile/lib/health-kit-sync.test.ts index f419a787b6..65717e54c3 100644 --- a/packages/mobile/lib/health-kit-sync.test.ts +++ b/packages/mobile/lib/health-kit-sync.test.ts @@ -4,6 +4,9 @@ import { NON_ADDITIVE_QUANTITY_TYPES, syncHealthKitToServer, } from "./health-kit-sync"; +import { captureException } from "./telemetry"; + +vi.mock("./telemetry", () => ({ captureException: vi.fn() })); describe("syncHealthKitToServer", () => { function createMockClient() { @@ -302,7 +305,8 @@ describe("syncHealthKitToServer", () => { it("records route query errors as non-fatal without aborting sync", async () => { const client = createMockClient(); const healthKit = createMockHealthKit(); - healthKit.queryWorkoutRoutes.mockRejectedValue(new Error("Route permission denied")); + const routeQueryError = new Error("Route permission denied"); + healthKit.queryWorkoutRoutes.mockRejectedValue(routeQueryError); const result = await syncHealthKitToServer({ trpcClient: client, @@ -315,6 +319,10 @@ describe("syncHealthKitToServer", () => { expect(result.errors.some((error) => error.includes("Route query"))).toBe(true); // Route push should not have been attempted expect(client.healthKitSync.pushWorkoutRoutes.mutate).not.toHaveBeenCalled(); + expect(captureException).toHaveBeenCalledWith(routeQueryError, { + source: "health-kit-workout-route-query", + workoutUuid: "workout-1", + }); }); it("records route push errors as non-fatal", async () => { @@ -323,7 +331,8 @@ describe("syncHealthKitToServer", () => { healthKit.queryWorkoutRoutes.mockResolvedValue([ { date: "2026-03-21T07:00:00Z", lat: 40.7128, lng: -74.006 }, ]); - client.healthKitSync.pushWorkoutRoutes.mutate.mockRejectedValue(new Error("Server error")); + const routePushError = new Error("Server error"); + client.healthKitSync.pushWorkoutRoutes.mutate.mockRejectedValue(routePushError); const result = await syncHealthKitToServer({ trpcClient: client, @@ -333,6 +342,10 @@ describe("syncHealthKitToServer", () => { expect(result.inserted).toBeGreaterThan(0); expect(result.errors.some((error) => error.includes("Push workout routes"))).toBe(true); + expect(captureException).toHaveBeenCalledWith(routePushError, { + source: "health-kit-workout-route-push", + routeCount: 1, + }); }); it("does not query routes when there are no workouts", async () => { diff --git a/packages/mobile/lib/health-kit-sync.ts b/packages/mobile/lib/health-kit-sync.ts index ce36fc7781..686c3d957b 100644 --- a/packages/mobile/lib/health-kit-sync.ts +++ b/packages/mobile/lib/health-kit-sync.ts @@ -5,6 +5,7 @@ import type { SleepSample, WorkoutSample, } from "../modules/health-kit"; +import { captureException } from "./telemetry"; // Additive types use HKStatisticsCollectionQuery for proper source deduplication. // Without this, overlapping samples from iPhone + Apple Watch get summed, roughly @@ -231,6 +232,10 @@ export async function syncHealthKitToServer(options: SyncOptions): Promise { }); it("does not throw when CoreMotion fails", async () => { - vi.mocked(deps.coreMotion.startRecording).mockRejectedValue(new Error("CoreMotion error")); + const recordingError = new Error("CoreMotion error"); + vi.mocked(deps.coreMotion.startRecording).mockRejectedValue(recordingError); await expect(service.ensureRecording()).resolves.toBeUndefined(); + expect(captureException).toHaveBeenCalledWith(recordingError, { + source: "activity-recording-core-motion-start", + }); }); it("does not throw when Watch sync fails", async () => { @@ -157,9 +161,13 @@ describe("InertialMeasurementUnitService", () => { }); it("does not throw when CoreMotion query fails", async () => { - vi.mocked(deps.coreMotion.queryRecordedData).mockRejectedValue(new Error("Query failed")); + const queryError = new Error("Query failed"); + vi.mocked(deps.coreMotion.queryRecordedData).mockRejectedValue(queryError); await expect(service.syncForTimeRange(startedAt, endedAt)).resolves.toBeUndefined(); + expect(captureException).toHaveBeenCalledWith(queryError, { + source: "activity-save-core-motion-sync", + }); }); it("does not throw when upload fails", async () => { diff --git a/packages/mobile/lib/inertial-measurement-unit-service.ts b/packages/mobile/lib/inertial-measurement-unit-service.ts index ee0d73da2f..e5de79021a 100644 --- a/packages/mobile/lib/inertial-measurement-unit-service.ts +++ b/packages/mobile/lib/inertial-measurement-unit-service.ts @@ -94,7 +94,10 @@ export function createInertialMeasurementUnitService( if (coreMotion.isAccelerometerRecordingAvailable()) { try { await coreMotion.startRecording(TWELVE_HOURS_SECONDS); - } catch { + } catch (error: unknown) { + captureException(error, { + source: "activity-recording-core-motion-start", + }); // Best-effort — don't block activity recording } } @@ -103,7 +106,8 @@ export function createInertialMeasurementUnitService( if (watch.isAvailable()) { try { await watch.requestSync(); - } catch { + } catch (error: unknown) { + captureException(error, { source: "activity-recording-watch-sync" }); // Best-effort — Watch may not be reachable } } @@ -115,7 +119,8 @@ export function createInertialMeasurementUnitService( if (connected) { await whoopBle.startStreaming(); } - } catch { + } catch (error: unknown) { + captureException(error, { source: "activity-recording-whoop-connect" }); // Best-effort — WHOOP may not be nearby or BLE unavailable } } @@ -129,7 +134,8 @@ export function createInertialMeasurementUnitService( if (phoneSamples.length > 0) { await uploadBatched(deviceId, "iphone", phoneSamples); } - } catch { + } catch (error: unknown) { + captureException(error, { source: "activity-save-core-motion-sync" }); // Best-effort — don't fail activity save } } @@ -142,7 +148,8 @@ export function createInertialMeasurementUnitService( await uploadBatched("Apple Watch", "apple_watch", watchSamples); watch.acknowledgeSamples(); } - } catch { + } catch (error: unknown) { + captureException(error, { source: "activity-save-watch-sync" }); // Best-effort — don't fail activity save } } diff --git a/packages/mobile/lib/open-external-url.test.ts b/packages/mobile/lib/open-external-url.test.ts index 615845d0ac..f26a2c481e 100644 --- a/packages/mobile/lib/open-external-url.test.ts +++ b/packages/mobile/lib/open-external-url.test.ts @@ -1,8 +1,9 @@ import { Linking } from "react-native"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { logger } from "./telemetry"; +import { captureException, logger } from "./telemetry"; vi.mock("./telemetry", () => ({ + captureException: vi.fn(), logger: { warn: vi.fn(), }, @@ -25,9 +26,8 @@ describe("openExternalUrl", () => { }); it("logs a warning and returns false when Linking fails", async () => { - vi.mocked(Linking.openURL).mockRejectedValue( - new Error("Unable to open URL: https://www.fatsecret.com/"), - ); + const openError = new Error("Unable to open URL: https://www.fatsecret.com/"); + vi.mocked(Linking.openURL).mockRejectedValue(openError); const { openExternalUrl } = await import("./open-external-url"); const opened = await openExternalUrl("https://www.fatsecret.com/", "food"); @@ -37,5 +37,9 @@ describe("openExternalUrl", () => { url: "https://www.fatsecret.com/", message: "Unable to open URL: https://www.fatsecret.com/", }); + expect(captureException).toHaveBeenCalledWith(openError, { + source: "open-external-url", + caller: "food", + }); }); }); diff --git a/packages/mobile/lib/open-external-url.ts b/packages/mobile/lib/open-external-url.ts index 48488a3a87..a7ba9543e8 100644 --- a/packages/mobile/lib/open-external-url.ts +++ b/packages/mobile/lib/open-external-url.ts @@ -1,11 +1,15 @@ import { Linking } from "react-native"; -import { logger } from "./telemetry"; +import { captureException, logger } from "./telemetry"; export async function openExternalUrl(url: string, source: string): Promise { try { await Linking.openURL(url); return true; } catch (error) { + captureException(error, { + source: "open-external-url", + caller: source, + }); logger.warn(source, "Unable to open external URL", { url, message: error instanceof Error ? error.message : String(error), diff --git a/packages/mobile/lib/trpc-fetch.ts b/packages/mobile/lib/trpc-fetch.ts index 54ce0196f8..f94ce4b187 100644 --- a/packages/mobile/lib/trpc-fetch.ts +++ b/packages/mobile/lib/trpc-fetch.ts @@ -25,7 +25,10 @@ function getTrpcPath(input: Parameters[0]): string { const url = new URL(rawUrl, "https://dofek.local"); const pathIndex = url.pathname.indexOf(pathPrefix); return pathIndex >= 0 ? url.pathname.slice(pathIndex + pathPrefix.length) : url.pathname; - } catch { + } catch (error: unknown) { + captureException(error, { + source: "trpc-request-url-parse", + }); return rawUrl; } } @@ -39,7 +42,8 @@ function buildStatusLabel(response: Response): string { async function readBodyPreview(response: Response): Promise { try { return (await response.clone().text()).slice(0, BODY_PREVIEW_LIMIT); - } catch { + } catch (error: unknown) { + captureException(error, { source: "trpc-response-body-preview" }); return "body preview unavailable"; } } diff --git a/packages/mobile/lib/useHaptic.test.ts b/packages/mobile/lib/useHaptic.test.ts index 0b6296451f..4af7edfcce 100644 --- a/packages/mobile/lib/useHaptic.test.ts +++ b/packages/mobile/lib/useHaptic.test.ts @@ -1,7 +1,11 @@ import { renderHook } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; -const mockSelection = vi.fn(() => Promise.resolve()); +const { mockCaptureException } = vi.hoisted(() => ({ + mockCaptureException: vi.fn(), +})); + +const mockSelection = vi.fn<() => Promise>(() => Promise.resolve()); const mockImpact = vi.fn(() => Promise.resolve()); const mockNotification = vi.fn(() => Promise.resolve()); @@ -13,7 +17,16 @@ vi.mock("expo-haptics", () => ({ NotificationFeedbackType: { Success: "success", Warning: "warning", Error: "error" }, })); +vi.mock("./telemetry", () => ({ + captureException: mockCaptureException, +})); + describe("useHaptic", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockSelection.mockResolvedValue(undefined); + }); + it("exports selection, impact, and notification functions", async () => { const { useHaptic } = await import("./useHaptic"); const { result } = renderHook(() => useHaptic()); @@ -30,4 +43,17 @@ describe("useHaptic", () => { result.current.selection(); expect(mockSelection).toHaveBeenCalled(); }); + + it("models unavailable optional haptics without reporting an operational defect", async () => { + mockSelection.mockRejectedValue(new Error("Haptics unavailable")); + const { useHaptic } = await import("./useHaptic"); + const { result } = renderHook(() => useHaptic()); + + result.current.selection(); + + await vi.waitFor(() => { + expect(mockSelection).toHaveBeenCalled(); + }); + expect(mockCaptureException).not.toHaveBeenCalled(); + }); }); diff --git a/packages/mobile/lib/useHaptic.ts b/packages/mobile/lib/useHaptic.ts index 651700160f..e6711df134 100644 --- a/packages/mobile/lib/useHaptic.ts +++ b/packages/mobile/lib/useHaptic.ts @@ -9,6 +9,19 @@ import { useCallback, useRef } from "react"; const THROTTLE_MS = 150; +type OptionalHapticResult = { status: "performed" } | { status: "unavailable"; cause: unknown }; + +async function performOptionalHaptic( + operation: () => Promise, +): Promise { + try { + await operation(); + return { status: "performed" }; + } catch (cause: unknown) { + return { status: "unavailable", cause }; + } +} + /** * Haptic feedback hook with built-in throttling. * @@ -22,10 +35,7 @@ export function useHaptic() { const now = Date.now(); if (now - lastFired.current < THROTTLE_MS) return; lastFired.current = now; - fn().catch((_error: unknown) => { - // Haptics unavailable — intentionally ignored (simulator, low power mode). - // This is non-critical UI feedback; logging would just create noise. - }); + void performOptionalHaptic(fn); }, []); const selection = useCallback(() => { diff --git a/packages/mobile/lib/useWhoopBleSync.ts b/packages/mobile/lib/useWhoopBleSync.ts index f3ac71eedb..81cd15e1ae 100644 --- a/packages/mobile/lib/useWhoopBleSync.ts +++ b/packages/mobile/lib/useWhoopBleSync.ts @@ -1,4 +1,3 @@ -import * as Sentry from "@sentry/react-native"; import { useEffect } from "react"; import { initBackgroundWhoopBleSync, @@ -7,6 +6,7 @@ import { type WhoopBleSyncDeps, } from "./background-whoop-ble-sync"; import type { InertialMeasurementUnitUploadClient } from "./inertial-measurement-unit-service"; +import { captureException } from "./telemetry"; /** * Hook that starts WHOOP BLE sync for all available data streams: @@ -27,7 +27,7 @@ export function useWhoopBleSync( ): void { useEffect(() => { initBackgroundWhoopBleSync(uploadClient, whoopDeps, realtimeClient).catch((error: unknown) => { - Sentry.captureException(error, { tags: { source: "whoop-ble-sync-init" } }); + captureException(error, { source: "whoop-ble-sync-init" }); }); return () => { diff --git a/packages/mobile/package.json b/packages/mobile/package.json index 621434ec53..0e87bed664 100644 --- a/packages/mobile/package.json +++ b/packages/mobile/package.json @@ -22,7 +22,7 @@ "ios": "expo run:ios", "ios:device": "expo run:ios --device", "prebuild": "expo prebuild --platform ios --clean", - "lint": "biome check .", + "lint": "biome check . && pnpm --dir ../.. lint:mobile-telemetry", "lint:fix": "biome check --write .", "typecheck": "tsc --noEmit" }, diff --git a/packages/mobile/test-setup.ts b/packages/mobile/test-setup.ts index 36c4503864..c857b621e1 100644 --- a/packages/mobile/test-setup.ts +++ b/packages/mobile/test-setup.ts @@ -201,6 +201,7 @@ vi.mock("react-native", () => { ); Image.displayName = "Image"; const FlatList = createMockComponent("FlatList"); + const Modal = createMockComponent("Modal"); const ActivityIndicator = ({ color, style, ...props }: Record) => React.createElement("activityindicator", { ...props, @@ -339,6 +340,7 @@ vi.mock("react-native", () => { TextInput, Image, FlatList, + Modal, ActivityIndicator, RefreshControl, Switch, diff --git a/scripts/mobile-catch-telemetry-policy.test.ts b/scripts/mobile-catch-telemetry-policy.test.ts new file mode 100644 index 0000000000..3efbde4aba --- /dev/null +++ b/scripts/mobile-catch-telemetry-policy.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from "vitest"; +import { + findHandledMobileErrorViolations, + scanMobileProductionFiles, +} from "./mobile-catch-telemetry-policy.ts"; + +describe("findHandledMobileErrorViolations", () => { + it.each([ + [ + "logs and consumes a catch clause", + ` + try { + await connect(); + } catch (error) { + console.error(error); + } + `, + ], + [ + "aggregates and consumes a catch clause", + ` + try { + await sync(); + } catch (error) { + errors.push(String(error)); + } + `, + ], + [ + "returns fallback data from a catch clause", + ` + try { + return await response.json(); + } catch { + return null; + } + `, + ], + [ + "updates UI state from a Promise catch", + ` + loadProviders().catch((error) => { + setError(String(error)); + }); + `, + ], + ])("rejects a handler that %s", (_description, sourceText) => { + expect(findHandledMobileErrorViolations("fixture.ts", sourceText)).toHaveLength(1); + }); + + it.each([ + [ + "reports the original error through the canonical helper", + ` + try { + await connect(); + } catch (error) { + captureException(error, { source: "connect" }); + setError(String(error)); + } + `, + ], + [ + "rethrows the original error", + ` + try { + await connect(); + } catch (error) { + throw error; + } + `, + ], + [ + "models user cancellation and rethrows unexpected errors", + ` + try { + return await signIn(); + } catch (error) { + if (isUserCancellation(error)) { + return { status: "cancelled" }; + } + throw error; + } + `, + ], + [ + "returns the explicit optional-haptic unavailable result", + ` + type OptionalHapticResult = + | { status: "performed" } + | { status: "unavailable"; cause: unknown }; + + async function performOptionalHaptic( + operation: () => Promise, + ): Promise { + try { + await operation(); + return { status: "performed" }; + } catch (cause: unknown) { + return { status: "unavailable", cause }; + } + } + `, + ], + ])("accepts a handler that %s", (_description, sourceText) => { + expect(findHandledMobileErrorViolations("fixture.ts", sourceText)).toEqual([]); + }); + + it("does not accept an exemption based only on haptic-looking names or comments", () => { + const sourceText = ` + async function runHaptic() { + try { + await vibrate(); + } catch (error) { + // Optional haptic is unavailable. + return { status: "ignored", cause: error }; + } + } + `; + + expect(findHandledMobileErrorViolations("fixture.ts", sourceText)).toHaveLength(1); + }); +}); + +describe("scanMobileProductionFiles", () => { + it("reports no caught-and-consumed unexpected errors in production mobile sources", () => { + expect(scanMobileProductionFiles("packages/mobile")).toEqual([]); + }); +}); diff --git a/scripts/mobile-catch-telemetry-policy.ts b/scripts/mobile-catch-telemetry-policy.ts new file mode 100644 index 0000000000..4916f29dd8 --- /dev/null +++ b/scripts/mobile-catch-telemetry-policy.ts @@ -0,0 +1,225 @@ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import ts from "typescript"; + +export interface HandledMobileErrorViolation { + column: number; + filePath: string; + kind: "catch-clause" | "promise-catch"; + line: number; +} + +function containsCanonicalCaptureOrThrow(node: ts.Node): boolean { + let handled = false; + + function visit(currentNode: ts.Node): void { + if (handled) { + return; + } + if (ts.isThrowStatement(currentNode)) { + handled = true; + return; + } + if ( + ts.isCallExpression(currentNode) && + ts.isIdentifier(currentNode.expression) && + currentNode.expression.text === "captureException" + ) { + handled = true; + return; + } + ts.forEachChild(currentNode, visit); + } + + visit(node); + return handled; +} + +function hasOptionalHapticResultReturnType( + catchClause: ts.CatchClause, + sourceFile: ts.SourceFile, +): boolean { + let currentNode: ts.Node | undefined = catchClause.parent; + while (currentNode && !ts.isFunctionLike(currentNode)) { + currentNode = currentNode.parent; + } + if (!currentNode || !currentNode.type) { + return false; + } + return ( + currentNode.type.getText(sourceFile).replace(/\s+/g, "") === "Promise" + ); +} + +function isExplicitOptionalHapticUnavailableResult( + catchClause: ts.CatchClause, + sourceFile: ts.SourceFile, +): boolean { + const catchVariable = catchClause.variableDeclaration?.name; + if ( + !catchVariable || + !ts.isIdentifier(catchVariable) || + catchClause.block.statements.length !== 1 + ) { + return false; + } + if (!hasOptionalHapticResultReturnType(catchClause, sourceFile)) { + return false; + } + + const statement = catchClause.block.statements[0]; + if (!statement || !ts.isReturnStatement(statement) || !statement.expression) { + return false; + } + if (!ts.isObjectLiteralExpression(statement.expression)) { + return false; + } + + let unavailableStatus = false; + let originalCause = false; + for (const property of statement.expression.properties) { + if ( + ts.isPropertyAssignment(property) && + property.name.getText(sourceFile) === "status" && + ts.isStringLiteral(property.initializer) && + property.initializer.text === "unavailable" + ) { + unavailableStatus = true; + } + if ( + ts.isShorthandPropertyAssignment(property) && + property.name.text === catchVariable.text && + property.name.text === "cause" + ) { + originalCause = true; + } + if ( + ts.isPropertyAssignment(property) && + property.name.getText(sourceFile) === "cause" && + ts.isIdentifier(property.initializer) && + property.initializer.text === catchVariable.text + ) { + originalCause = true; + } + } + + return unavailableStatus && originalCause; +} + +function makeViolation( + filePath: string, + kind: HandledMobileErrorViolation["kind"], + node: ts.Node, + sourceFile: ts.SourceFile, +): HandledMobileErrorViolation { + const position = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)); + return { + column: position.character + 1, + filePath, + kind, + line: position.line + 1, + }; +} + +export function findHandledMobileErrorViolations( + filePath: string, + sourceText: string, +): HandledMobileErrorViolation[] { + const scriptKind = filePath.endsWith(".tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS; + const sourceFile = ts.createSourceFile( + filePath, + sourceText, + ts.ScriptTarget.Latest, + true, + scriptKind, + ); + const violations: HandledMobileErrorViolation[] = []; + + function visit(node: ts.Node): void { + if ( + ts.isCatchClause(node) && + !containsCanonicalCaptureOrThrow(node.block) && + !isExplicitOptionalHapticUnavailableResult(node, sourceFile) + ) { + violations.push(makeViolation(filePath, "catch-clause", node, sourceFile)); + } + + if ( + ts.isCallExpression(node) && + ts.isPropertyAccessExpression(node.expression) && + node.expression.name.text === "catch" + ) { + const handler = node.arguments[0]; + if (handler && !containsCanonicalCaptureOrThrow(handler)) { + violations.push(makeViolation(filePath, "promise-catch", node, sourceFile)); + } + } + + ts.forEachChild(node, visit); + } + + visit(sourceFile); + return violations; +} + +function isProductionTypeScriptFile(filePath: string): boolean { + const filename = path.basename(filePath); + return ( + /\.(ts|tsx)$/.test(filename) && + !/\.(test|stories)\.(ts|tsx)$/.test(filename) && + !filePath.split(path.sep).some((segment) => segment.startsWith(".")) + ); +} + +function listProductionTypeScriptFiles(inputPath: string): string[] { + if (!statSync(inputPath).isDirectory()) { + return isProductionTypeScriptFile(inputPath) ? [inputPath] : []; + } + + const files: string[] = []; + for (const directoryEntry of readdirSync(inputPath, { withFileTypes: true })) { + if ( + directoryEntry.name === "node_modules" || + (directoryEntry.isDirectory() && directoryEntry.name.startsWith(".")) + ) { + continue; + } + const entryPath = path.join(inputPath, directoryEntry.name); + if (directoryEntry.isDirectory()) { + files.push(...listProductionTypeScriptFiles(entryPath)); + } else if (isProductionTypeScriptFile(entryPath)) { + files.push(entryPath); + } + } + return files; +} + +export function scanMobileProductionFiles(inputPath: string): HandledMobileErrorViolation[] { + return listProductionTypeScriptFiles(inputPath) + .sort() + .flatMap((filePath) => + findHandledMobileErrorViolations(filePath, readFileSync(filePath, "utf8")), + ); +} + +function runCommandLine(): void { + const inputPath = process.argv[2] ?? "packages/mobile"; + const violations = scanMobileProductionFiles(inputPath); + if (violations.length === 0) { + console.log("Mobile handled-error telemetry policy passed."); + return; + } + + for (const violation of violations) { + console.error( + `${violation.filePath}:${violation.line}:${violation.column} ${violation.kind} consumes an unexpected error without canonical captureException() or rethrowing`, + ); + } + process.exitCode = 1; +} + +const commandPath = process.argv[1]; +if (commandPath && pathToFileURL(path.resolve(commandPath)).href === import.meta.url) { + runCommandLine(); +} From 0be7fbb77f2c3292167447b1827f4cbc8ce8cdcc Mon Sep 17 00:00:00 2001 From: Asher Cohen Date: Fri, 24 Jul 2026 09:24:31 -0700 Subject: [PATCH 2/2] fix(mobile): address telemetry review --- .../mobile/app/imu-visualization.test.tsx | 109 +++++++++++++ .../mobile/app/providers/auth-modals.test.tsx | 123 +++++++++++++- packages/mobile/app/providers/auth-modals.tsx | 15 +- .../inertial-measurement-unit-service.test.ts | 19 ++- .../lib/inertial-measurement-unit-service.ts | 16 +- packages/mobile/lib/telemetry.test.ts | 13 +- packages/mobile/lib/telemetry.ts | 6 +- packages/mobile/lib/useHaptic.test.ts | 20 ++- packages/mobile/lib/useHaptic.ts | 19 ++- scripts/mobile-catch-telemetry-policy.test.ts | 63 +++++-- scripts/mobile-catch-telemetry-policy.ts | 154 +++++++++--------- 11 files changed, 446 insertions(+), 111 deletions(-) create mode 100644 packages/mobile/app/imu-visualization.test.tsx diff --git a/packages/mobile/app/imu-visualization.test.tsx b/packages/mobile/app/imu-visualization.test.tsx new file mode 100644 index 0000000000..1c367b930e --- /dev/null +++ b/packages/mobile/app/imu-visualization.test.tsx @@ -0,0 +1,109 @@ +// @vitest-environment jsdom + +import { render, screen, waitFor } from "@testing-library/react"; +import React from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const whoopBleMocks = vi.hoisted(() => ({ + addConnectionStateListener: vi.fn(() => ({ remove: vi.fn() })), + addOrientationListener: vi.fn(() => ({ remove: vi.fn() })), + connect: vi.fn(() => Promise.resolve(true)), + findWhoop: vi.fn(() => Promise.resolve({ id: "whoop-1", name: "WHOOP" })), + getConnectionState: vi.fn(() => "idle"), + startImuStreaming: vi.fn(() => Promise.resolve(true)), +})); +const mockCaptureException = vi.hoisted(() => vi.fn()); + +function stripStyle({ + style: _style, + contentContainerStyle: _contentStyle, + ...rest +}: Record) { + return rest; +} + +vi.mock("react-native", () => ({ + View: ({ children, ...props }: Record) => + React.createElement("div", stripStyle(props), ...(children != null ? [children] : [])), + Text: ({ children, ...props }: Record) => + React.createElement("span", stripStyle(props), ...(children != null ? [children] : [])), + ScrollView: ({ children, ...props }: Record) => + React.createElement("div", stripStyle(props), ...(children != null ? [children] : [])), + StyleSheet: { + create: >(styles: Styles): Styles => styles, + }, +})); + +vi.mock("expo-router", () => ({ + Stack: { + Screen: () => null, + }, +})); + +vi.mock("../components/WristModel", () => ({ + WristModel: () => null, +})); + +vi.mock("../modules/whoop-ble", () => whoopBleMocks); + +vi.mock("../lib/telemetry", () => ({ + captureException: mockCaptureException, +})); + +vi.mock("../theme", () => ({ + colors: { + background: "#000", + surface: "#111", + text: "#fff", + textSecondary: "#aaa", + positive: "#0f0", + danger: "#f00", + green: "#0f0", + blue: "#00f", + }, +})); + +vi.mock("./_layout-options", () => ({ + rootStackScreenOptions: {}, +})); + +import ImuVisualizationScreen from "./imu-visualization"; + +describe("ImuVisualizationScreen", () => { + beforeEach(() => { + vi.clearAllMocks(); + whoopBleMocks.getConnectionState.mockReturnValue("idle"); + whoopBleMocks.findWhoop.mockResolvedValue({ id: "whoop-1", name: "WHOOP" }); + whoopBleMocks.connect.mockResolvedValue(true); + whoopBleMocks.startImuStreaming.mockResolvedValue(true); + }); + + it("reports a best-effort streaming-start failure for an existing connection", async () => { + const streamingError = new Error("Native streaming failed"); + whoopBleMocks.getConnectionState.mockReturnValue("streaming"); + whoopBleMocks.startImuStreaming.mockRejectedValue(streamingError); + + render(); + + await waitFor(() => { + expect(mockCaptureException).toHaveBeenCalledWith(streamingError, { + source: "imu-visualization-start-streaming", + }); + }); + expect(screen.getByText("streaming")).toBeTruthy(); + }); + + it("reports connection failures while preserving the visible error", async () => { + const connectionError = new Error("Bluetooth connection failed"); + whoopBleMocks.connect.mockRejectedValue(connectionError); + + render(); + + await waitFor(() => { + expect(screen.getByText("Bluetooth connection failed")).toBeTruthy(); + }); + expect(mockCaptureException).toHaveBeenCalledWith(connectionError, { + source: "imu-visualization-connect", + }); + }); +}); diff --git a/packages/mobile/app/providers/auth-modals.test.tsx b/packages/mobile/app/providers/auth-modals.test.tsx index 391d7f78c4..d288840997 100644 --- a/packages/mobile/app/providers/auth-modals.test.tsx +++ b/packages/mobile/app/providers/auth-modals.test.tsx @@ -1,9 +1,20 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const { credentialSignIn, mockCaptureException } = vi.hoisted(() => ({ +const { + credentialSignIn, + garminSignIn, + mockCaptureException, + whoopSaveTokens, + whoopSignIn, + whoopVerifyCode, +} = vi.hoisted(() => ({ credentialSignIn: vi.fn(), + garminSignIn: vi.fn(), mockCaptureException: vi.fn(), + whoopSaveTokens: vi.fn(), + whoopSignIn: vi.fn(), + whoopVerifyCode: vi.fn(), })); vi.mock("../../lib/telemetry", () => ({ @@ -17,17 +28,38 @@ vi.mock("../../lib/trpc", () => ({ useMutation: () => ({ mutateAsync: credentialSignIn }), }, }, + garminAuth: { + signIn: { + useMutation: () => ({ mutateAsync: garminSignIn }), + }, + }, + whoopAuth: { + signIn: { + useMutation: () => ({ mutateAsync: whoopSignIn }), + }, + verifyCode: { + useMutation: () => ({ mutateAsync: whoopVerifyCode }), + }, + saveTokens: { + useMutation: () => ({ mutateAsync: whoopSaveTokens }), + }, + }, }, })); -import { CredentialAuthModal } from "./auth-modals"; +import { CredentialAuthModal, GarminAuthModal, WhoopAuthModal } from "./auth-modals"; -describe("CredentialAuthModal", () => { +describe("provider auth modals", () => { beforeEach(() => { - vi.clearAllMocks(); + credentialSignIn.mockReset(); + garminSignIn.mockReset(); + mockCaptureException.mockReset(); + whoopSaveTokens.mockReset(); + whoopSignIn.mockReset(); + whoopVerifyCode.mockReset(); }); - it("reports the original sign-in error while preserving the actionable message", async () => { + it("reports a credential provider sign-in error while preserving its message", async () => { const signInError = new Error("Provider rejected these credentials"); credentialSignIn.mockRejectedValue(signInError); @@ -56,4 +88,85 @@ describe("CredentialAuthModal", () => { providerId: "wahoo", }); }); + + it("reports a Garmin sign-in error while preserving its message", async () => { + const signInError = new Error("Garmin rejected these credentials"); + garminSignIn.mockRejectedValue(signInError); + + render(); + + fireEvent.change(screen.getByPlaceholderText("Email"), { + target: { value: "athlete@example.com" }, + }); + fireEvent.change(screen.getByPlaceholderText("Password"), { + target: { value: "secret" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Sign in to Garmin" })); + + await waitFor(() => { + expect(screen.getByText("Garmin rejected these credentials")).toBeTruthy(); + }); + expect(mockCaptureException).toHaveBeenCalledWith(signInError, { + source: "provider-garmin-auth-sign-in", + providerId: "garmin", + }); + }); + + it("reports a WHOOP sign-in error while preserving its message", async () => { + const signInError = new Error("WHOOP rejected these credentials"); + whoopSignIn.mockRejectedValue(signInError); + + render(); + + fireEvent.change(screen.getByPlaceholderText("Email"), { + target: { value: "athlete@example.com" }, + }); + fireEvent.change(screen.getByPlaceholderText("Password"), { + target: { value: "secret" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Sign in to WHOOP" })); + + await waitFor(() => { + expect(screen.getByText("WHOOP rejected these credentials")).toBeTruthy(); + }); + expect(mockCaptureException).toHaveBeenCalledWith(signInError, { + source: "provider-whoop-auth-sign-in", + providerId: "whoop", + }); + }); + + it("reports a WHOOP verification error while preserving its message", async () => { + const verificationError = new Error("Verification code expired"); + whoopSignIn.mockResolvedValue({ + status: "verification_required", + challengeId: "challenge-1", + }); + whoopVerifyCode.mockRejectedValue(verificationError); + + render(); + + fireEvent.change(screen.getByPlaceholderText("Email"), { + target: { value: "athlete@example.com" }, + }); + fireEvent.change(screen.getByPlaceholderText("Password"), { + target: { value: "secret" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Sign in to WHOOP" })); + + await waitFor(() => { + expect(screen.getByPlaceholderText("Verification code")).toBeTruthy(); + }); + fireEvent.change(screen.getByPlaceholderText("Verification code"), { + target: { value: "123456" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Verify WHOOP code" })); + + await waitFor(() => { + expect(screen.getByText("Verification code expired")).toBeTruthy(); + }); + expect(mockCaptureException).toHaveBeenCalledWith(verificationError, { + source: "provider-whoop-auth-verify", + providerId: "whoop", + }); + }); }); diff --git a/packages/mobile/app/providers/auth-modals.tsx b/packages/mobile/app/providers/auth-modals.tsx index 4164ac96b5..1feab0452c 100644 --- a/packages/mobile/app/providers/auth-modals.tsx +++ b/packages/mobile/app/providers/auth-modals.tsx @@ -136,7 +136,10 @@ export function GarminAuthModal({ await signInMutation.mutateAsync({ username, password }); onSuccess(); } catch (error_: unknown) { - captureException(error_, { source: "provider-garmin-auth-sign-in" }); + captureException(error_, { + source: "provider-garmin-auth-sign-in", + providerId: "garmin", + }); setError(error_ instanceof Error ? error_.message : "Sign in failed"); } finally { setLoading(false); @@ -254,7 +257,10 @@ export function WhoopAuthModal({ onSuccess(); } } catch (error_: unknown) { - captureException(error_, { source: "provider-whoop-auth-sign-in" }); + captureException(error_, { + source: "provider-whoop-auth-sign-in", + providerId: "whoop", + }); setError(error_ instanceof Error ? error_.message : "Sign in failed"); } finally { setLoading(false); @@ -272,7 +278,10 @@ export function WhoopAuthModal({ onSuccess(); } } catch (error_: unknown) { - captureException(error_, { source: "provider-whoop-auth-verify" }); + captureException(error_, { + source: "provider-whoop-auth-verify", + providerId: "whoop", + }); setError(error_ instanceof Error ? error_.message : "Verification failed"); } finally { setLoading(false); diff --git a/packages/mobile/lib/inertial-measurement-unit-service.test.ts b/packages/mobile/lib/inertial-measurement-unit-service.test.ts index 442bd845c0..254091c116 100644 --- a/packages/mobile/lib/inertial-measurement-unit-service.test.ts +++ b/packages/mobile/lib/inertial-measurement-unit-service.test.ts @@ -50,6 +50,7 @@ describe("InertialMeasurementUnitService", () => { let service: InertialMeasurementUnitService; beforeEach(() => { + vi.clearAllMocks(); deps = makeMockDeps(); service = createInertialMeasurementUnitService(deps); }); @@ -240,9 +241,25 @@ describe("InertialMeasurementUnitService", () => { it("does not throw when WHOOP connection fails", async () => { const whoopBle = deps.whoopBle; if (!whoopBle) throw new Error("whoopBle not initialized"); - vi.mocked(whoopBle.findAndConnect).mockRejectedValue(new Error("BLE error")); + const connectionError = new Error("BLE error"); + vi.mocked(whoopBle.findAndConnect).mockRejectedValue(connectionError); await expect(service.ensureRecording()).resolves.toBeUndefined(); + expect(captureException).toHaveBeenCalledWith(connectionError, { + source: "activity-recording-whoop-connect", + }); + }); + + it("reports WHOOP streaming-start failures with a distinct source", async () => { + const whoopBle = deps.whoopBle; + if (!whoopBle) throw new Error("whoopBle not initialized"); + const streamingError = new Error("Streaming failed"); + vi.mocked(whoopBle.startStreaming).mockRejectedValue(streamingError); + + await expect(service.ensureRecording()).resolves.toBeUndefined(); + expect(captureException).toHaveBeenCalledWith(streamingError, { + source: "activity-recording-whoop-start-streaming", + }); }); it("does not start streaming when connection fails", async () => { diff --git a/packages/mobile/lib/inertial-measurement-unit-service.ts b/packages/mobile/lib/inertial-measurement-unit-service.ts index e5de79021a..53821d612a 100644 --- a/packages/mobile/lib/inertial-measurement-unit-service.ts +++ b/packages/mobile/lib/inertial-measurement-unit-service.ts @@ -114,15 +114,23 @@ export function createInertialMeasurementUnitService( // Connect to WHOOP strap and start IMU streaming (best-effort) if (whoopBle?.isAvailable()) { + let connected = false; try { - const connected = await whoopBle.findAndConnect(); - if (connected) { - await whoopBle.startStreaming(); - } + connected = await whoopBle.findAndConnect(); } catch (error: unknown) { captureException(error, { source: "activity-recording-whoop-connect" }); // Best-effort — WHOOP may not be nearby or BLE unavailable } + if (connected) { + try { + await whoopBle.startStreaming(); + } catch (error: unknown) { + captureException(error, { + source: "activity-recording-whoop-start-streaming", + }); + // Best-effort — WHOOP streaming may be unavailable + } + } } }, diff --git a/packages/mobile/lib/telemetry.test.ts b/packages/mobile/lib/telemetry.test.ts index 72502f59ab..437552db7a 100644 --- a/packages/mobile/lib/telemetry.test.ts +++ b/packages/mobile/lib/telemetry.test.ts @@ -111,17 +111,24 @@ describe("ios telemetry", () => { expect(mocks.mockCaptureMessage).not.toHaveBeenCalled(); }); - it("delegates captureException to Sentry with extra context", async () => { + it("delegates captureException to Sentry with a searchable source tag and extra context", async () => { process.env.EXPO_PUBLIC_SENTRY_DSN = "https://key@sentry.example/789"; const mod = await import("./telemetry"); mod.initTelemetry(); const error = new Error("test error"); - mod.captureException(error, { "error.source": "react-native.global" }); + mod.captureException(error, { + source: "react-native-global", + operation: "global-handler", + }); expect(mocks.mockCaptureException).toHaveBeenCalledWith(error, { - extra: { "error.source": "react-native.global" }, + tags: { source: "react-native-global" }, + extra: { + source: "react-native-global", + operation: "global-handler", + }, }); }); diff --git a/packages/mobile/lib/telemetry.ts b/packages/mobile/lib/telemetry.ts index 7677287047..10ecb1a3b1 100644 --- a/packages/mobile/lib/telemetry.ts +++ b/packages/mobile/lib/telemetry.ts @@ -83,7 +83,11 @@ export function initTelemetry() { } export function captureException(error: unknown, context: Record = {}) { - Sentry.captureException(error, { extra: context }); + const source = typeof context.source === "string" ? context.source : undefined; + Sentry.captureException(error, { + ...(source ? { tags: { source } } : {}), + extra: context, + }); const errorMessage = error instanceof Error ? error.message : typeof error === "string" ? error : "Unknown error"; const attributes = diff --git a/packages/mobile/lib/useHaptic.test.ts b/packages/mobile/lib/useHaptic.test.ts index 4af7edfcce..3dd9cff713 100644 --- a/packages/mobile/lib/useHaptic.test.ts +++ b/packages/mobile/lib/useHaptic.test.ts @@ -45,7 +45,10 @@ describe("useHaptic", () => { }); it("models unavailable optional haptics without reporting an operational defect", async () => { - mockSelection.mockRejectedValue(new Error("Haptics unavailable")); + const unavailableError = Object.assign(new Error("Haptics unavailable"), { + code: "ERR_UNAVAILABLE", + }); + mockSelection.mockRejectedValue(unavailableError); const { useHaptic } = await import("./useHaptic"); const { result } = renderHook(() => useHaptic()); @@ -56,4 +59,19 @@ describe("useHaptic", () => { }); expect(mockCaptureException).not.toHaveBeenCalled(); }); + + it("reports unexpected haptic failures", async () => { + const hapticError = new Error("Native bridge failed"); + mockSelection.mockRejectedValue(hapticError); + const { useHaptic } = await import("./useHaptic"); + const { result } = renderHook(() => useHaptic()); + + result.current.selection(); + + await vi.waitFor(() => { + expect(mockCaptureException).toHaveBeenCalledWith(hapticError, { + source: "optional-haptic-feedback", + }); + }); + }); }); diff --git a/packages/mobile/lib/useHaptic.ts b/packages/mobile/lib/useHaptic.ts index e6711df134..d49562fcaf 100644 --- a/packages/mobile/lib/useHaptic.ts +++ b/packages/mobile/lib/useHaptic.ts @@ -6,11 +6,21 @@ import { selectionAsync, } from "expo-haptics"; import { useCallback, useRef } from "react"; +import { captureException } from "./telemetry"; const THROTTLE_MS = 150; type OptionalHapticResult = { status: "performed" } | { status: "unavailable"; cause: unknown }; +function isHapticUnavailableError(error: unknown): error is { code: "ERR_UNAVAILABLE" } { + return ( + typeof error === "object" && + error !== null && + "code" in error && + error.code === "ERR_UNAVAILABLE" + ); +} + async function performOptionalHaptic( operation: () => Promise, ): Promise { @@ -18,7 +28,10 @@ async function performOptionalHaptic( await operation(); return { status: "performed" }; } catch (cause: unknown) { - return { status: "unavailable", cause }; + if (isHapticUnavailableError(cause)) { + return { status: "unavailable", cause }; + } + throw cause; } } @@ -35,7 +48,9 @@ export function useHaptic() { const now = Date.now(); if (now - lastFired.current < THROTTLE_MS) return; lastFired.current = now; - void performOptionalHaptic(fn); + void performOptionalHaptic(fn).catch((error: unknown) => { + captureException(error, { source: "optional-haptic-feedback" }); + }); }, []); const selection = useCallback(() => { diff --git a/scripts/mobile-catch-telemetry-policy.test.ts b/scripts/mobile-catch-telemetry-policy.test.ts index 3efbde4aba..6ce47023bc 100644 --- a/scripts/mobile-catch-telemetry-policy.test.ts +++ b/scripts/mobile-catch-telemetry-policy.test.ts @@ -44,6 +44,32 @@ describe("findHandledMobileErrorViolations", () => { }); `, ], + [ + "hides a throw in an unreachable branch", + ` + try { + await sync(); + } catch (error) { + if (false) { + throw error; + } + console.error(error); + } + `, + ], + [ + "hides captureException in an unreachable nested function", + ` + try { + await sync(); + } catch (error) { + function reportLater() { + captureException(error); + } + console.error(error); + } + `, + ], ])("rejects a handler that %s", (_description, sourceText) => { expect(findHandledMobileErrorViolations("fixture.ts", sourceText)).toHaveLength(1); }); @@ -84,21 +110,15 @@ describe("findHandledMobileErrorViolations", () => { `, ], [ - "returns the explicit optional-haptic unavailable result", + "rethrows errors outside an explicit expected-error classifier", ` - type OptionalHapticResult = - | { status: "performed" } - | { status: "unavailable"; cause: unknown }; - - async function performOptionalHaptic( - operation: () => Promise, - ): Promise { - try { - await operation(); - return { status: "performed" }; - } catch (cause: unknown) { - return { status: "unavailable", cause }; + try { + return await readHealthData(); + } catch (error) { + if (!isAuthorizationNotDetermined(error)) { + throw error; } + return []; } `, ], @@ -120,6 +140,23 @@ describe("findHandledMobileErrorViolations", () => { expect(findHandledMobileErrorViolations("fixture.ts", sourceText)).toHaveLength(1); }); + + it("does not accept a same-named optional haptic result alias as an exemption", () => { + const sourceText = ` + type OptionalHapticResult = { status: "ignored"; cause: unknown }; + + async function performOptionalHaptic(): Promise { + try { + await vibrate(); + return { status: "ignored", cause: null }; + } catch (cause) { + return { status: "unavailable", cause }; + } + } + `; + + expect(findHandledMobileErrorViolations("fixture.ts", sourceText)).toHaveLength(1); + }); }); describe("scanMobileProductionFiles", () => { diff --git a/scripts/mobile-catch-telemetry-policy.ts b/scripts/mobile-catch-telemetry-policy.ts index 4916f29dd8..df1e18e682 100644 --- a/scripts/mobile-catch-telemetry-policy.ts +++ b/scripts/mobile-catch-telemetry-policy.ts @@ -10,101 +10,96 @@ export interface HandledMobileErrorViolation { line: number; } -function containsCanonicalCaptureOrThrow(node: ts.Node): boolean { - let handled = false; - - function visit(currentNode: ts.Node): void { - if (handled) { - return; - } - if (ts.isThrowStatement(currentNode)) { - handled = true; - return; - } - if ( - ts.isCallExpression(currentNode) && - ts.isIdentifier(currentNode.expression) && - currentNode.expression.text === "captureException" - ) { - handled = true; - return; - } - ts.forEachChild(currentNode, visit); - } - - visit(node); - return handled; +function isCanonicalCaptureCall(node: ts.Node): boolean { + return ( + ts.isCallExpression(node) && + ts.isIdentifier(node.expression) && + node.expression.text === "captureException" + ); } -function hasOptionalHapticResultReturnType( - catchClause: ts.CatchClause, - sourceFile: ts.SourceFile, +function statementAlwaysHandles( + statement: ts.Statement, + caughtIdentifier: string | undefined, ): boolean { - let currentNode: ts.Node | undefined = catchClause.parent; - while (currentNode && !ts.isFunctionLike(currentNode)) { - currentNode = currentNode.parent; + if (ts.isThrowStatement(statement)) { + return true; } - if (!currentNode || !currentNode.type) { - return false; + if (ts.isExpressionStatement(statement)) { + return isCanonicalCaptureCall(statement.expression); } - return ( - currentNode.type.getText(sourceFile).replace(/\s+/g, "") === "Promise" - ); + if (ts.isBlock(statement)) { + return statementsContainReachableHandler(statement.statements, caughtIdentifier); + } + if (ts.isIfStatement(statement) && statement.elseStatement) { + return ( + statementAlwaysHandles(statement.thenStatement, caughtIdentifier) && + statementAlwaysHandles(statement.elseStatement, caughtIdentifier) + ); + } + return false; } -function isExplicitOptionalHapticUnavailableResult( - catchClause: ts.CatchClause, - sourceFile: ts.SourceFile, +function isExpectedErrorGuard( + statement: ts.Statement, + caughtIdentifier: string | undefined, ): boolean { - const catchVariable = catchClause.variableDeclaration?.name; if ( - !catchVariable || - !ts.isIdentifier(catchVariable) || - catchClause.block.statements.length !== 1 + !caughtIdentifier || + !ts.isIfStatement(statement) || + statement.elseStatement || + !statementAlwaysHandles(statement.thenStatement, caughtIdentifier) || + !ts.isPrefixUnaryExpression(statement.expression) || + statement.expression.operator !== ts.SyntaxKind.ExclamationToken || + !ts.isCallExpression(statement.expression.operand) || + !ts.isIdentifier(statement.expression.operand.expression) || + !statement.expression.operand.expression.text.startsWith("is") ) { return false; } - if (!hasOptionalHapticResultReturnType(catchClause, sourceFile)) { - return false; - } - - const statement = catchClause.block.statements[0]; - if (!statement || !ts.isReturnStatement(statement) || !statement.expression) { - return false; - } - if (!ts.isObjectLiteralExpression(statement.expression)) { - return false; - } + return statement.expression.operand.arguments.some( + (argument) => ts.isIdentifier(argument) && argument.text === caughtIdentifier, + ); +} - let unavailableStatus = false; - let originalCause = false; - for (const property of statement.expression.properties) { - if ( - ts.isPropertyAssignment(property) && - property.name.getText(sourceFile) === "status" && - ts.isStringLiteral(property.initializer) && - property.initializer.text === "unavailable" - ) { - unavailableStatus = true; - } +function statementsContainReachableHandler( + statements: ts.NodeArray, + caughtIdentifier: string | undefined, +): boolean { + for (const statement of statements) { if ( - ts.isShorthandPropertyAssignment(property) && - property.name.text === catchVariable.text && - property.name.text === "cause" + statementAlwaysHandles(statement, caughtIdentifier) || + isExpectedErrorGuard(statement, caughtIdentifier) ) { - originalCause = true; + return true; } - if ( - ts.isPropertyAssignment(property) && - property.name.getText(sourceFile) === "cause" && - ts.isIdentifier(property.initializer) && - property.initializer.text === catchVariable.text - ) { - originalCause = true; + if (ts.isReturnStatement(statement)) { + return false; } } + return false; +} - return unavailableStatus && originalCause; +function containsCanonicalCaptureOrThrow(node: ts.Node, caughtIdentifier?: string): boolean { + if (isCanonicalCaptureCall(node)) { + return true; + } + if (ts.isIdentifier(node) && node.text === "captureException") { + return true; + } + if (ts.isBlock(node)) { + return statementsContainReachableHandler(node.statements, caughtIdentifier); + } + if (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) { + const handlerParameter = node.parameters[0]?.name; + const handlerIdentifier = ts.isIdentifier(handlerParameter) + ? handlerParameter.text + : caughtIdentifier; + return ts.isBlock(node.body) + ? statementsContainReachableHandler(node.body.statements, handlerIdentifier) + : isCanonicalCaptureCall(node.body); + } + return false; } function makeViolation( @@ -137,10 +132,13 @@ export function findHandledMobileErrorViolations( const violations: HandledMobileErrorViolation[] = []; function visit(node: ts.Node): void { + const catchVariable = ts.isCatchClause(node) ? node.variableDeclaration?.name : undefined; if ( ts.isCatchClause(node) && - !containsCanonicalCaptureOrThrow(node.block) && - !isExplicitOptionalHapticUnavailableResult(node, sourceFile) + !containsCanonicalCaptureOrThrow( + node.block, + catchVariable && ts.isIdentifier(catchVariable) ? catchVariable.text : undefined, + ) ) { violations.push(makeViolation(filePath, "catch-clause", node, sourceFile)); }