From 0460b128f32d1cf67ee4d03e19515f106fd6f6b1 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Wed, 2 Sep 2026 09:20:17 +0200 Subject: [PATCH] fix(crypto): recover device verification and quieten resume disconnects --- src/app/components/DeviceVerification.tsx | 1 + .../ReceiveSelfDeviceVerification.test.tsx | 22 +++++++++++- .../components/modal-overlay/ModalOverlay.tsx | 10 ++++-- src/app/crypto/engineCrypto/EngineCrypto.ts | 16 +++++++++ .../incomingVerificationRequest.test.ts | 35 +++++++++++++++++++ src/app/hooks/useNetworkRecovery.ts | 26 ++++++++------ src/app/pages/client/SyncStatus.test.ts | 20 +++++++++++ src/app/pages/client/SyncStatus.tsx | 33 +++++++++++++++-- 8 files changed, 147 insertions(+), 16 deletions(-) diff --git a/src/app/components/DeviceVerification.tsx b/src/app/components/DeviceVerification.tsx index 91c8db0fd6..c28a1cc27b 100644 --- a/src/app/components/DeviceVerification.tsx +++ b/src/app/components/DeviceVerification.tsx @@ -272,6 +272,7 @@ export function DeviceVerification({ request, onExit }: DeviceVerificationProps) requestClose={handleCancel} dismissOnClickOutside={false} escapeDeactivates={false} + deactivateCloses={false} >
diff --git a/src/app/components/ReceiveSelfDeviceVerification.test.tsx b/src/app/components/ReceiveSelfDeviceVerification.test.tsx index 799d122fa3..ff53a014c1 100644 --- a/src/app/components/ReceiveSelfDeviceVerification.test.tsx +++ b/src/app/components/ReceiveSelfDeviceVerification.test.tsx @@ -18,7 +18,13 @@ vi.mock('$hooks/useMatrixClient', () => ({ })); vi.mock('$components/modal-overlay/ModalOverlay', () => ({ - ModalOverlay: ({ children }: { children: React.ReactNode }) =>
{children}
, + ModalOverlay: ({ + children, + deactivateCloses, + }: { + children: React.ReactNode; + deactivateCloses?: boolean; + }) =>
{children}
, })); const pendingRequest = { @@ -51,6 +57,20 @@ describe('ReceiveSelfDeviceVerification', () => { await waitFor(() => expect(screen.getByText('Device Verification')).toBeInTheDocument()); }); + it('does not treat unmounting as the user cancelling', async () => { + const cancel = vi.fn<() => Promise>(async () => undefined); + getVerificationRequestsToDeviceInProgress.mockReturnValue([{ ...pendingRequest, cancel }]); + + const { unmount } = renderReceiver(); + await waitFor(() => expect(screen.getByText('Device Verification')).toBeInTheDocument()); + expect( + screen.getByText('Device Verification').closest('[data-deactivate-closes]') + ).toHaveAttribute('data-deactivate-closes', 'false'); + + unmount(); + expect(cancel).not.toHaveBeenCalled(); + }); + it('ignores a request this device started', async () => { getVerificationRequestsToDeviceInProgress.mockReturnValue([ { ...pendingRequest, initiatedByMe: true }, diff --git a/src/app/components/modal-overlay/ModalOverlay.tsx b/src/app/components/modal-overlay/ModalOverlay.tsx index a86b170fef..c726023cbf 100644 --- a/src/app/components/modal-overlay/ModalOverlay.tsx +++ b/src/app/components/modal-overlay/ModalOverlay.tsx @@ -39,6 +39,8 @@ type ModalOverlayProps = { background?: string; /** Set false for full-bleed viewers that inset their own controls. */ respectSafeArea?: boolean; + /** Set false where unmounting must not count as the user dismissing the overlay. */ + deactivateCloses?: boolean; children: ReactNode; }; @@ -52,8 +54,10 @@ export function ModalOverlay({ escapeDeactivates = stopPropagation, background, respectSafeArea = true, + deactivateCloses = true, children, }: ModalOverlayProps) { + const onDeactivate = deactivateCloses ? requestClose : undefined; // Null outside a provider, where desktop is the safe assumption. const isMobile = useScreenSizeOptionally() === ScreenSize.Mobile; const ownedModalRef = useRef(null); @@ -73,7 +77,7 @@ export function ModalOverlay({ initialFocus: false, fallbackFocus: () => contentRef?.current ?? document.body, escapeDeactivates, - onDeactivate: requestClose, + onDeactivate, }} >
document.body, - onDeactivate: requestClose, + onDeactivate, clickOutsideDeactivates: dismissOnClickOutside, escapeDeactivates, }; @@ -128,7 +132,7 @@ export function ModalOverlay({ fallbackFocus: () => (size ? ownedModalRef.current : contentRef?.current) ?? document.body, clickOutsideDeactivates: dismissOnClickOutside, - onDeactivate: requestClose, + onDeactivate, escapeDeactivates, }} > diff --git a/src/app/crypto/engineCrypto/EngineCrypto.ts b/src/app/crypto/engineCrypto/EngineCrypto.ts index d1a909e1e3..a072222a67 100644 --- a/src/app/crypto/engineCrypto/EngineCrypto.ts +++ b/src/app/crypto/engineCrypto/EngineCrypto.ts @@ -2005,7 +2005,22 @@ export class EngineCrypto return request; } + async #cancelStaleRequests(userId: string): Promise { + const stale = [...this.#verificationRequests.values()].filter( + (request) => request.otherUserId === userId && request.pending + ); + for (const request of stale) { + traceVerification('Cancelling a stale verification request', { + flowId: request.transactionId ?? null, + }); + // eslint-disable-next-line no-await-in-loop + await request.cancel().catch(() => undefined); + if (request.transactionId) this.#verificationRequests.delete(request.transactionId); + } + } + async requestOwnUserVerification(): Promise { + await this.#cancelStaleRequests(this.#identity.userId); return this.#startVerification('userIdentity.requestVerification', { userId: this.#identity.userId, methods: SUPPORTED_VERIFICATION_METHOD_CODES, @@ -2013,6 +2028,7 @@ export class EngineCrypto } async requestDeviceVerification(userId: string, deviceId: string): Promise { + await this.#cancelStaleRequests(userId); await this.#sendTracked(await this.#call('queryKeysForUsers', { users: [userId] })); await this.#flushOutgoingRequests(); return this.#startVerification('device.requestVerification', { diff --git a/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts b/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts index bfc43fd901..ddaf69b072 100644 --- a/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts +++ b/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts @@ -102,6 +102,41 @@ describe('sending a verification request', () => { }); }); +describe('stale verification requests', () => { + beforeEach(() => { + mockInvoke.mockReset(); + vi.mocked(traceVerification).mockClear(); + }); + + it('cancels an existing pending flow before starting a new one', async () => { + const { mx } = clientSpy(); + const invoked: string[] = []; + mockInvoke.mockImplementation(async (_identity, method) => { + invoked.push(method as string); + if (method === 'receiveSyncChanges') { + return [{ type: 3, rawEvent: JSON.stringify(REQUEST_EVENT) }]; + } + if (method === 'getVerificationRequest') return requestState; + if (method === 'queryKeysForUsers') { + return { id: 'q1', type: 1, className: 'KeysQueryRequest', body: '{}' }; + } + if (method === 'device.requestVerification') { + return { request: { ...requestState, flowId: '$new' }, outgoingRequest: null }; + } + return []; + }); + + const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' }); + await crypto.preprocessToDeviceMessages([REQUEST_EVENT as never]); + await crypto.requestDeviceVerification('@me:e.org', 'OTHER'); + + expect(invoked).toContain('verificationRequest.cancel'); + expect(invoked.indexOf('verificationRequest.cancel')).toBeLessThan( + invoked.indexOf('device.requestVerification') + ); + }); +}); + describe('pending verification request sweep', () => { beforeEach(() => mockInvoke.mockReset()); diff --git a/src/app/hooks/useNetworkRecovery.ts b/src/app/hooks/useNetworkRecovery.ts index 22ddc8a09d..ba4445166e 100644 --- a/src/app/hooks/useNetworkRecovery.ts +++ b/src/app/hooks/useNetworkRecovery.ts @@ -3,6 +3,7 @@ import { onlineManager } from '@tanstack/react-query'; import { TauriEvent, listen } from '@tauri-apps/api/event'; import { isTauri } from '@tauri-apps/api/core'; import type { MatrixClient } from '$types/matrix-sdk'; +import { SyncState } from '$types/matrix-sdk'; import type { NudgeReason } from '$client/reconnect'; import { abortClassicSyncPoll, nudgeReconnect } from '$client/reconnect'; import { useSyncState } from './useSyncState'; @@ -19,6 +20,7 @@ const WEDGED_NUDGE_ATTEMPTS = 3; export const useNetworkRecovery = (mx: MatrixClient | undefined): void => { const lastSyncAtRef = useRef(Date.now()); + const syncStateRef = useRef(null); const verifyTimerRef = useRef(undefined); const deadNudgesRef = useRef(0); @@ -39,11 +41,15 @@ export const useNetworkRecovery = (mx: MatrixClient | undefined): void => { useSyncState( mx, - useCallback(() => { - lastSyncAtRef.current = Date.now(); - deadNudgesRef.current = 0; - cancelVerify(); - }, [cancelVerify]) + useCallback( + (current) => { + syncStateRef.current = current; + lastSyncAtRef.current = Date.now(); + deadNudgesRef.current = 0; + cancelVerify(); + }, + [cancelVerify] + ) ); // Foreground nudges (resume / visible-stale / online) get one sync verification: @@ -70,11 +76,11 @@ export const useNetworkRecovery = (mx: MatrixClient | undefined): void => { const onOnline = () => nudgeForeground('online'); const onVisible = () => { - if ( - document.visibilityState === 'visible' && - Date.now() - lastSyncAtRef.current >= VISIBLE_STALE_MS - ) { - nudgeForeground('visible'); + if (document.visibilityState !== 'visible') return; + const degraded = + syncStateRef.current === SyncState.Error || syncStateRef.current === SyncState.Reconnecting; + if (degraded || Date.now() - lastSyncAtRef.current >= VISIBLE_STALE_MS) { + nudgeForeground('visible', degraded ? { force: true } : undefined); } }; diff --git a/src/app/pages/client/SyncStatus.test.ts b/src/app/pages/client/SyncStatus.test.ts index f934cf96ac..a570d4a562 100644 --- a/src/app/pages/client/SyncStatus.test.ts +++ b/src/app/pages/client/SyncStatus.test.ts @@ -80,6 +80,26 @@ describe('useStickyDisconnected (hysteresis)', () => { expect(result.current).toBeNull(); }); + it('waits longer before the banner when the app has just been resumed', () => { + const { result, rerender } = renderHook(({ state }) => useStickyDisconnected(state), { + initialProps: { state: SyncState.Syncing as SyncState | null }, + }); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + + rerender({ state: SyncState.Error }); + act(() => { + vi.advanceTimersByTime(3000); + }); + expect(result.current).toBeNull(); + + act(() => { + vi.advanceTimersByTime(6000); + }); + expect(result.current).toBe(SyncState.Error); + }); + it('shows banner after degraded for >2s', () => { const { result, rerender } = renderHook(({ state }) => useStickyDisconnected(state), { initialProps: { state: SyncState.Syncing as SyncState | null }, diff --git a/src/app/pages/client/SyncStatus.tsx b/src/app/pages/client/SyncStatus.tsx index 964267a867..23fdaf2756 100644 --- a/src/app/pages/client/SyncStatus.tsx +++ b/src/app/pages/client/SyncStatus.tsx @@ -9,9 +9,16 @@ import { type TitlebarStatusView, titlebarStatusAtom } from '$state/titlebarStat import { SyncConnectionStatusBanner } from '$components/SyncConnectionStatus'; import { useDesktopSetting } from '$state/hooks/desktopSettings'; import { hasCustomDesktopTitlebar } from '$utils/tauriTitlebar'; +import { createDebugLogger } from '$utils/debugLogger'; + +const syncLog = createDebugLogger('sync-status'); const DISCONNECTED_SHOW_DELAY_MS = 2000; const DISCONNECTED_HIDE_DELAY_MS = 3000; +// Coming back from the background kills the poll; the SDK reconnects on its own well +// inside this window, so waiting spares a banner for something already being fixed. +const RESUME_SHOW_DELAY_MS = 8000; +const RESUME_WINDOW_MS = 10000; type StateData = { current: SyncState | null; @@ -37,8 +44,17 @@ export const useStickyDisconnected = (syncCurrent: SyncState | null): SyncState const degraded = syncCurrent === SyncState.Reconnecting || syncCurrent === SyncState.Error ? syncCurrent : null; const showStartedAtRef = useRef(null); + const becameVisibleAtRef = useRef(0); const [stickyDisconnected, setStickyDisconnected] = useState(null); + useEffect(() => { + const onVisibility = () => { + if (document.visibilityState === 'visible') becameVisibleAtRef.current = Date.now(); + }; + document.addEventListener('visibilitychange', onVisibility); + return () => document.removeEventListener('visibilitychange', onVisibility); + }, []); + useEffect(() => { if (degraded) { if (stickyDisconnected) { @@ -49,7 +65,9 @@ export const useStickyDisconnected = (syncCurrent: SyncState | null): SyncState const startedAt = showStartedAtRef.current ?? Date.now(); showStartedAtRef.current = startedAt; - const remaining = Math.max(0, DISCONNECTED_SHOW_DELAY_MS - (Date.now() - startedAt)); + const justResumed = startedAt - becameVisibleAtRef.current < RESUME_WINDOW_MS; + const showDelay = justResumed ? RESUME_SHOW_DELAY_MS : DISCONNECTED_SHOW_DELAY_MS; + const remaining = Math.max(0, showDelay - (Date.now() - startedAt)); const id = setTimeout(() => { showStartedAtRef.current = null; setStickyDisconnected(degraded); @@ -83,7 +101,7 @@ export function SyncStatus({ mx }: SyncStatusProps) { useSyncState( mx, - useCallback((current, previous) => { + useCallback((current, previous, data) => { const showConnecting = shouldShowConnecting(hasConnectedRef.current, current, previous); if (current === SyncState.Syncing) hasConnectedRef.current = true; @@ -99,6 +117,17 @@ export function SyncStatus({ mx }: SyncStatusProps) { }); if (current === SyncState.Reconnecting || current === SyncState.Error) { + const error = data?.error as + | { name?: string; message?: string; errcode?: string; httpStatus?: number } + | undefined; + syncLog.warn('network', 'Sync degraded', { + state: current, + previous: previous ?? 'none', + name: error?.name ?? 'none', + message: error?.message ?? 'none', + errcode: error?.errcode ?? 'none', + httpStatus: error?.httpStatus ?? -1, + }); Sentry.addBreadcrumb({ category: 'sync', message: `Sync state changed to ${current}`,