From a8a8a954c72b3e21fdc73240cb2b7d7a0a627170 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:57:43 -0700 Subject: [PATCH] fix(mobile): a relay status we could not read is not "offline" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems in the same newly-extracted collector, both in the artifact a user pastes into a bug report. `.catch(() => 'offline' as const)` reported the definite neighbour for a lookup that observed nothing, so a support engineer could not tell a host that reported offline from one that never answered. It now reports `unreadable`. And the collector could reject: `window.api.mobile.getRelayStatus()` throws synchronously when the bridge has no `mobile` — before `.catch` is attached — while both call sites fire this as `void copyRelayDiagnostics()` with the await sitting AHEAD of their try/catch. Measured: the click wrote no clipboard and showed no toast at all, where the pre-extraction inline payload build could not fail. The collector is total now, and the await moved inside the try so a future addition cannot silently kill the toast again. --- .../src/components/mobile/MobilePage.tsx | 10 ++-- .../mobile-relay-diagnostics-payload.test.ts | 55 ++++++++++++++++++- .../mobile-relay-diagnostics-payload.ts | 35 ++++++++++-- .../src/components/settings/MobilePane.tsx | 10 ++-- 4 files changed, 92 insertions(+), 18 deletions(-) diff --git a/src/renderer/src/components/mobile/MobilePage.tsx b/src/renderer/src/components/mobile/MobilePage.tsx index 4087e483efe..312d896746d 100644 --- a/src/renderer/src/components/mobile/MobilePage.tsx +++ b/src/renderer/src/components/mobile/MobilePage.tsx @@ -152,12 +152,12 @@ export default function MobilePage(): React.JSX.Element { if (relayMintFailure == null) { return } - // Why: users share this payload — an address (selected or relay cell) would leak a LAN/Tailscale IP or hostname. - const payload = await collectMobileRelayDiagnosticsPayload({ - connectionMode, - failure: relayMintFailure - }) try { + // Why: users share this payload — an address (selected or relay cell) would leak a LAN/Tailscale IP or hostname. + const payload = await collectMobileRelayDiagnosticsPayload({ + connectionMode, + failure: relayMintFailure + }) await window.api.ui.writeClipboardText(JSON.stringify(payload, null, 2)) if (mountedRef.current) { toast.success( diff --git a/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.test.ts b/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.test.ts index 3bd45ed3368..81c6857e013 100644 --- a/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.test.ts +++ b/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.test.ts @@ -1,5 +1,8 @@ -import { describe, expect, it } from 'vitest' -import { buildMobileRelayDiagnosticsPayload } from './mobile-relay-diagnostics-payload' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + buildMobileRelayDiagnosticsPayload, + collectMobileRelayDiagnosticsPayload +} from './mobile-relay-diagnostics-payload' import type { MobileRelayMintFailure } from '../../../../shared/mobile-relay-mint-failure' const failure: MobileRelayMintFailure = { @@ -47,3 +50,51 @@ describe('buildMobileRelayDiagnosticsPayload', () => { expect(payload).not.toHaveProperty('cellUrl') }) }) + +// Why this matters beyond the field value: the payload is what a user pastes into a bug report, +// and both Copy-diagnostics buttons fire the collector as `void copyRelayDiagnostics()` with the +// await sitting ahead of their try/catch — so a rejection here is a click that writes no +// clipboard and shows no toast at all. +describe('collectMobileRelayDiagnosticsPayload', () => { + afterEach(() => { + Reflect.deleteProperty(globalThis, 'window') + }) + + function stubWindow(mobile: unknown): void { + const api: Record = {} + if (mobile !== undefined) { + api.mobile = mobile + } + Object.defineProperty(globalThis, 'window', { configurable: true, value: { api } }) + } + + it("reports 'unreadable' rather than the definite 'offline' when the status lookup fails", async () => { + stubWindow({ getRelayStatus: vi.fn().mockRejectedValue(new Error('ipc down')) }) + + const payload = await collectMobileRelayDiagnosticsPayload({ + connectionMode: 'automatic', + failure + }) + + expect(payload.relayStatus).toBe('unreadable') + }) + + it('still resolves when the mobile bridge is missing entirely', async () => { + stubWindow(undefined) + + await expect( + collectMobileRelayDiagnosticsPayload({ connectionMode: 'automatic', failure }) + ).resolves.toMatchObject({ relayStatus: 'unreadable' }) + }) + + it('reports a host-answered offline as offline', async () => { + stubWindow({ getRelayStatus: vi.fn().mockResolvedValue({ status: 'offline' }) }) + + const payload = await collectMobileRelayDiagnosticsPayload({ + connectionMode: 'automatic', + failure + }) + + expect(payload.relayStatus).toBe('offline') + }) +}) diff --git a/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.ts b/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.ts index 8ccac2eedc8..07ea129fa05 100644 --- a/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.ts +++ b/src/renderer/src/components/mobile/mobile-relay-diagnostics-payload.ts @@ -3,11 +3,18 @@ import type { MobileRelayMintFailure } from '../../../../shared/mobile-relay-min import type { MobileRelayStatus } from '../../../../shared/mobile-relay-status' import { resolveClientEnvironmentInfo } from '@/lib/client-environment-info' +/** The status lookup's own failure, kept distinct from the host answering 'offline'. */ +export const MOBILE_RELAY_DIAGNOSTICS_STATUS_UNREADABLE = 'unreadable' + +export type MobileRelayDiagnosticsStatus = + | MobileRelayStatus + | typeof MOBILE_RELAY_DIAGNOSTICS_STATUS_UNREADABLE + export type MobileRelayDiagnosticsPayload = { kind: 'mobile_pairing_relay_failure' preferredConnectionMode: MobilePairingConnectionMode failure: MobileRelayMintFailure - relayStatus: MobileRelayStatus + relayStatus: MobileRelayDiagnosticsStatus appVersion: string at: string } @@ -17,7 +24,7 @@ export type MobileRelayDiagnosticsPayload = { export function buildMobileRelayDiagnosticsPayload(args: { connectionMode: MobilePairingConnectionMode failure: MobileRelayMintFailure - relayStatus: MobileRelayStatus + relayStatus: MobileRelayDiagnosticsStatus appVersion: string }): MobileRelayDiagnosticsPayload { return { @@ -30,6 +37,25 @@ export function buildMobileRelayDiagnosticsPayload(args: { } } +/** + * Why not 'offline' on failure: this payload is what a user pastes into a bug report, and + * 'offline' is a claim the broker was down. A status call this renderer could not complete + * observed nothing (docs/reference/ssh-execution-boundary.md), and reporting it as the definite + * neighbour points triage at the relay instead of at the lookup that actually failed. + * + * Why the whole call and not just a `.catch`: both Copy-diagnostics buttons fire this as + * `void copyRelayDiagnostics()`, and the await now sits ahead of their try/catch, so a bridge + * missing `mobile` — which throws where a rejected promise was expected — would leave the click + * with no clipboard write and no toast at all. + */ +async function readRelayStatusForDiagnostics(): Promise { + try { + return (await window.api.mobile.getRelayStatus()).status + } catch { + return MOBILE_RELAY_DIAGNOSTICS_STATUS_UNREADABLE + } +} + // Why here rather than at each call site: both "Copy diagnostics" buttons must // fetch the same two fields the same way, and MobilePane sits at the line ceiling. export async function collectMobileRelayDiagnosticsPayload(args: { @@ -37,10 +63,7 @@ export async function collectMobileRelayDiagnosticsPayload(args: { failure: MobileRelayMintFailure }): Promise { const [relayStatus, environment] = await Promise.all([ - window.api.mobile - .getRelayStatus() - .then((detail) => detail.status) - .catch(() => 'offline' as const), + readRelayStatusForDiagnostics(), resolveClientEnvironmentInfo() ]) return buildMobileRelayDiagnosticsPayload({ diff --git a/src/renderer/src/components/settings/MobilePane.tsx b/src/renderer/src/components/settings/MobilePane.tsx index 814f3beeaa1..78904d81222 100644 --- a/src/renderer/src/components/settings/MobilePane.tsx +++ b/src/renderer/src/components/settings/MobilePane.tsx @@ -306,12 +306,12 @@ export function MobilePane(): React.JSX.Element { if (relayMintFailure == null) { return } - // Why: users share this payload, so it carries no address (selected or relay cell). - const payload = await collectMobileRelayDiagnosticsPayload({ - connectionMode, - failure: relayMintFailure - }) try { + // Why: users share this payload, so it carries no address (selected or relay cell). + const payload = await collectMobileRelayDiagnosticsPayload({ + connectionMode, + failure: relayMintFailure + }) await window.api.ui.writeClipboardText(JSON.stringify(payload, null, 2)) if (mountedRef.current) { toast.success(