From d2946c00ce2f4f3d9fa5583563e8a40b0e6a73d8 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Wed, 24 Jun 2026 08:22:00 -0400 Subject: [PATCH] fix(matrix): upload retry/backoff, robust MatrixError, typed m.direct, reliable presence on unload - uploadContent: bounded retry (max 3) reusing rateLimitedActions' capped exponential backoff; retries only transient failures (network/429/5xx), never 4xx. - Robust MatrixError construction from UploadResponse / unknown error shapes. - addRoomIdToMDirect/removeRoomIdFromMDirect: drop `as any`, use typed EventType.Direct + MDirectContent. - usePresenceUpdater: keep fetch({keepalive}) for the unload offline-presence update (sendBeacon can't set the auth header) and log redacted warnings instead of silently swallowing presence errors. Co-Authored-By: Claude Opus 4.8 --- src/app/hooks/usePresenceUpdater.ts | 22 +++++- src/app/utils/matrix.ts | 117 ++++++++++++++++++++++------ 2 files changed, 110 insertions(+), 29 deletions(-) diff --git a/src/app/hooks/usePresenceUpdater.ts b/src/app/hooks/usePresenceUpdater.ts index 62ff28b07..e71e630fb 100644 --- a/src/app/hooks/usePresenceUpdater.ts +++ b/src/app/hooks/usePresenceUpdater.ts @@ -23,6 +23,13 @@ export function usePresenceUpdater() { const readStatus = () => userId ? (localStorage.getItem(`lotus-status-msg-${userId}`) ?? '') : ''; + // Log presence failures without leaking PII (user id, token, status message). + const warnPresenceFailure = (presence: string, err: unknown) => { + const reason = + err instanceof Error ? err.message : typeof err === 'string' ? err : 'unknown error'; + console.warn(`Failed to set presence to "${presence}":`, reason); + }; + const setOnline = () => { const status = readStatus(); return mx @@ -30,7 +37,7 @@ export function usePresenceUpdater() { presence: 'online', ...(status ? { status_msg: status } : {}), }) - .catch(() => undefined); + .catch((err) => warnPresenceFailure('online', err)); }; const setUnavailable = (statusMsg?: string) => { const status = readStatus(); @@ -39,10 +46,12 @@ export function usePresenceUpdater() { presence: 'unavailable', ...(statusMsg ? { status_msg: statusMsg } : status ? { status_msg: status } : {}), }) - .catch(() => undefined); + .catch((err) => warnPresenceFailure('unavailable', err)); }; const setOffline = () => - mx.setPresence({ presence: 'offline', status_msg: '' }).catch(() => undefined); + mx + .setPresence({ presence: 'offline', status_msg: '' }) + .catch((err) => warnPresenceFailure('offline', err)); // Manual presence overrides — no activity tracking needed. if (hidePresence || presenceStatus === 'invisible') { @@ -100,6 +109,11 @@ export function usePresenceUpdater() { const baseUrl = mx.getHomeserverUrl(); if (!userId || !token || !baseUrl) return; + // Reliable delivery during page teardown: navigator.sendBeacon cannot set the + // Authorization header required by the authenticated Matrix presence endpoint, so + // it isn't usable here. fetch(..., { keepalive: true }) lets the request outlive the + // page and is the correct mechanism for an authed endpoint. (keepalive bodies are + // capped at 64KB, which this tiny payload is well under.) fetch(`${baseUrl}/_matrix/client/v3/presence/${encodeURIComponent(userId)}/status`, { method: 'PUT', headers: { @@ -108,7 +122,7 @@ export function usePresenceUpdater() { }, body: JSON.stringify({ presence: 'offline' }), keepalive: true, - }).catch(() => undefined); + }).catch((err) => warnPresenceFailure('offline (pagehide)', err)); }; setOnline(); diff --git a/src/app/utils/matrix.ts b/src/app/utils/matrix.ts index 34bb84a53..13d18b5a3 100644 --- a/src/app/utils/matrix.ts +++ b/src/app/utils/matrix.ts @@ -5,6 +5,7 @@ import { } from 'browser-encrypt-attachment'; import { EventTimeline, + EventType, MatrixClient, MatrixError, MatrixEvent, @@ -15,7 +16,7 @@ import { } from 'matrix-js-sdk'; import to from 'await-to-js'; import { IImageInfo, IThumbnailContent, IVideoInfo } from '../../types/matrix/common'; -import { AccountDataEvent } from '../../types/matrix/accountData'; +import { MDirectContent } from '../../types/matrix/accountData'; import { getStateEvent } from './room'; import { Membership, StateEvent } from '../../types/matrix/room'; @@ -145,6 +146,42 @@ export type ContentUploadOptions = { onError: (error: MatrixError) => void; }; +// Build a MatrixError defensively from an unexpected upload response. +// MatrixError's constructor expects an IErrorJson ({ errcode?, error?, ... }), not a +// raw UploadResponse, so we guard the shape and only forward string fields it understands. +const matrixErrorFromUploadResponse = (data: UploadResponse): MatrixError => { + const errorJson: { errcode?: string; error?: string } = {}; + const maybe = data as Partial<{ errcode: unknown; error: unknown }>; + if (typeof maybe.errcode === 'string') errorJson.errcode = maybe.errcode; + if (typeof maybe.error === 'string') errorJson.error = maybe.error; + if (!errorJson.error) errorJson.error = 'Upload failed: missing content_uri in response'; + return new MatrixError(errorJson); +}; + +const matrixErrorFromUnknown = (e: unknown): MatrixError => { + if (e instanceof MatrixError) return e; + const err = e as Partial<{ message: unknown; errcode: unknown }> | null | undefined; + const error = typeof err?.message === 'string' ? err.message : 'Upload failed'; + const errcode = typeof err?.errcode === 'string' ? err.errcode : undefined; + return new MatrixError({ error, errcode }); +}; + +// HTTP statuses that should not be retried — client errors are deterministic +// (e.g. 413 payload too large, 400 bad request, 401/403 auth) and won't succeed on retry. +const isRetryableUploadError = (e: unknown): boolean => { + if (e instanceof MatrixError) { + const status = e.httpStatus; + // No status => network/transport failure (transient): retry. + if (typeof status !== 'number') return true; + // Retry on rate-limiting and server-side (5xx) errors only. + return status === 429 || status >= 500; + } + // Non-Matrix errors are typically network/transport failures: retry. + return true; +}; + +const UPLOAD_MAX_RETRY_COUNT = 3; + export const uploadContent = async ( mx: MatrixClient, file: TUploadContent, @@ -152,23 +189,53 @@ export const uploadContent = async ( ) => { const { name, fileType, hideFilename, onProgress, onPromise, onSuccess, onError } = options; - const uploadPromise = mx.uploadContent(file, { - name, - type: fileType, - includeFilename: !hideFilename, - progressHandler: onProgress, - }); - onPromise?.(uploadPromise); - try { - const data = await uploadPromise; - const mxc = data.content_uri; - if (mxc) onSuccess(mxc); - else onError(new MatrixError(data)); - } catch (e: any) { - const error = typeof e?.message === 'string' ? e.message : undefined; - const errcode = typeof e?.name === 'string' ? e.message : undefined; - onError(new MatrixError({ error, errcode })); + const sleepForMs = (ms: number) => + new Promise((resolve) => { + setTimeout(resolve, ms); + }); + + let lastError: MatrixError | undefined; + + for (let retryCount = 0; retryCount <= UPLOAD_MAX_RETRY_COUNT; retryCount += 1) { + const uploadPromise = mx.uploadContent(file, { + name, + type: fileType, + includeFilename: !hideFilename, + progressHandler: onProgress, + }); + onPromise?.(uploadPromise); + + try { + // eslint-disable-next-line no-await-in-loop + const data = await uploadPromise; + const mxc = data.content_uri; + if (mxc) { + onSuccess(mxc); + return; + } + // Missing content_uri is not a transient failure — fail immediately. + onError(matrixErrorFromUploadResponse(data)); + return; + } catch (e: unknown) { + lastError = matrixErrorFromUnknown(e); + + if (retryCount === UPLOAD_MAX_RETRY_COUNT || !isRetryableUploadError(e)) { + onError(lastError); + return; + } + + // Respect server Retry-After header; fall back to capped exponential backoff, + // mirroring rateLimitedActions (min(1000 * 2^retryCount, 30_000)ms). + const waitMS = + (e instanceof MatrixError ? e.getRetryAfterMs() : null) ?? + Math.min(1000 * 2 ** retryCount, 30_000); + // eslint-disable-next-line no-await-in-loop + await sleepForMs(waitMS); + } } + + // Unreachable in practice, but keeps onError guaranteed if the loop exits. + if (lastError) onError(lastError); }; export const matrixEventByRecency = (m1: MatrixEvent, m2: MatrixEvent) => m2.getTs() - m1.getTs(); @@ -230,11 +297,11 @@ export const addRoomIdToMDirect = async ( roomId: string, userId: string, ): Promise => { - const mDirectsEvent = mx.getAccountData(AccountDataEvent.Direct as any); - let userIdToRoomIds: Record = {}; + const mDirectsEvent = mx.getAccountData(EventType.Direct); + let userIdToRoomIds: MDirectContent = {}; if (typeof mDirectsEvent !== 'undefined') - userIdToRoomIds = structuredClone(mDirectsEvent.getContent()); + userIdToRoomIds = structuredClone(mDirectsEvent.getContent()); // remove it from the lists of any others users // (it can only be a DM room for one person) @@ -255,15 +322,15 @@ export const addRoomIdToMDirect = async ( } userIdToRoomIds[userId] = roomIds; - await mx.setAccountData(AccountDataEvent.Direct as any, userIdToRoomIds as any); + await mx.setAccountData(EventType.Direct, userIdToRoomIds); }; export const removeRoomIdFromMDirect = async (mx: MatrixClient, roomId: string): Promise => { - const mDirectsEvent = mx.getAccountData(AccountDataEvent.Direct as any); - let userIdToRoomIds: Record = {}; + const mDirectsEvent = mx.getAccountData(EventType.Direct); + let userIdToRoomIds: MDirectContent = {}; if (typeof mDirectsEvent !== 'undefined') - userIdToRoomIds = structuredClone(mDirectsEvent.getContent()); + userIdToRoomIds = structuredClone(mDirectsEvent.getContent()); Object.keys(userIdToRoomIds).forEach((targetUserId) => { const roomIds = userIdToRoomIds[targetUserId]; @@ -273,7 +340,7 @@ export const removeRoomIdFromMDirect = async (mx: MatrixClient, roomId: string): } }); - await mx.setAccountData(AccountDataEvent.Direct as any, userIdToRoomIds as any); + await mx.setAccountData(EventType.Direct, userIdToRoomIds); }; export const mxcUrlToHttp = (