fix(correctness): call-invite clock skew, forceState, upload cancel (COR-3/5/6)
COR-3 (CallEmbedProvider): the incoming-call lifetime guard distrusted a caller's sender_ts only when it was >20s AHEAD of the server ts. A caller clock that ran SLOW left sender_ts in the past, so the ring auto-dismissed/never showed for a fresh invite. Trust sender_ts only within ±20s of the server ts, else fall back to it (also fixes a NaN path when sender_ts is missing). COR-6 (CallControl): forceState rebuilt CallControlState with 5 args, silently defaulting screenshareAudioMuted to false; pass this.screenshareAudioMuted. COR-5 (uploadContent + useBindUploadAtom): cancelling during the retry back-off was a no-op (mx.cancelUpload only aborts an in-flight request), so the upload resurrected on the next attempt. Thread an AbortSignal: the back-off sleep resolves early on abort and the loop stops with an abort error; the hook aborts a per-upload AbortController on cancel (alongside mx.cancelUpload for the in-flight case). All verified by two review passes (no double-settle / no resurrection); includes their suggested abort-listener cleanup on normal sleep resolution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -473,8 +473,16 @@ function IncomingCallListener({ callEmbed, joined }: IncomingCallListenerProps)
|
|||||||
|
|
||||||
const sender = event.getSender();
|
const sender = event.getSender();
|
||||||
const content = event.getContent<IRTCNotificationContent>();
|
const content = event.getContent<IRTCNotificationContent>();
|
||||||
|
// Trust the caller's sender_ts only when it's within 20s of the server's
|
||||||
|
// timestamp in EITHER direction. A fast caller clock made a fresh invite
|
||||||
|
// look expired-in-the-future; a SLOW one left sender_ts in the past so
|
||||||
|
// `Date.now() >= senderTs + lifetime` hid/dismissed the ring for a
|
||||||
|
// genuinely fresh invite (COR-3). Fall back to the server ts on large skew.
|
||||||
const senderTs =
|
const senderTs =
|
||||||
content.sender_ts - event.getTs() > 20000 ? event.getTs() : content.sender_ts;
|
typeof content.sender_ts === 'number' &&
|
||||||
|
Math.abs(content.sender_ts - event.getTs()) <= 20000
|
||||||
|
? content.sender_ts
|
||||||
|
: event.getTs();
|
||||||
const lifetime = Math.min(content.lifetime, 120000);
|
const lifetime = Math.min(content.lifetime, 120000);
|
||||||
const notificationType = content.notification_type;
|
const notificationType = content.notification_type;
|
||||||
const relation =
|
const relation =
|
||||||
|
|||||||
@@ -159,6 +159,7 @@ export class CallControl extends EventEmitter implements CallControlState {
|
|||||||
desired.sound,
|
desired.sound,
|
||||||
this.screenshare,
|
this.screenshare,
|
||||||
this.spotlight,
|
this.spotlight,
|
||||||
|
this.screenshareAudioMuted,
|
||||||
);
|
);
|
||||||
await this.applyState();
|
await this.applyState();
|
||||||
// P6-2: CallEmbed calls forceState() only from onCallJoined(), so this is
|
// P6-2: CallEmbed calls forceState() only from onCallJoined(), so this is
|
||||||
|
|||||||
+18
-12
@@ -1,7 +1,7 @@
|
|||||||
import { atom, useAtom } from 'jotai';
|
import { atom, useAtom } from 'jotai';
|
||||||
import { atomFamily } from 'jotai/utils';
|
import { atomFamily } from 'jotai/utils';
|
||||||
import { MatrixClient, UploadResponse, UploadProgress, MatrixError } from 'matrix-js-sdk';
|
import { MatrixClient, UploadResponse, UploadProgress, MatrixError } from 'matrix-js-sdk';
|
||||||
import { useCallback } from 'react';
|
import { useCallback, useRef } from 'react';
|
||||||
import { useThrottle } from '../hooks/useThrottle';
|
import { useThrottle } from '../hooks/useThrottle';
|
||||||
import { uploadContent, TUploadContent } from '../utils/matrix';
|
import { uploadContent, TUploadContent } from '../utils/matrix';
|
||||||
|
|
||||||
@@ -110,19 +110,25 @@ export const useBindUploadAtom = (
|
|||||||
{ immediate: true, wait: 200 },
|
{ immediate: true, wait: 200 },
|
||||||
);
|
);
|
||||||
|
|
||||||
const startUpload = useCallback(
|
const abortRef = useRef<AbortController | undefined>(undefined);
|
||||||
() =>
|
|
||||||
uploadContent(mx, file, {
|
const startUpload = useCallback(() => {
|
||||||
hideFilename,
|
const controller = new AbortController();
|
||||||
onPromise: (promise: Promise<UploadResponse>) => setUpload({ promise }),
|
abortRef.current = controller;
|
||||||
onProgress: handleProgress,
|
return uploadContent(mx, file, {
|
||||||
onSuccess: (mxc) => setUpload({ mxc }),
|
hideFilename,
|
||||||
onError: (error) => setUpload({ error }),
|
signal: controller.signal,
|
||||||
}),
|
onPromise: (promise: Promise<UploadResponse>) => setUpload({ promise }),
|
||||||
[mx, file, hideFilename, setUpload, handleProgress],
|
onProgress: handleProgress,
|
||||||
);
|
onSuccess: (mxc) => setUpload({ mxc }),
|
||||||
|
onError: (error) => setUpload({ error }),
|
||||||
|
});
|
||||||
|
}, [mx, file, hideFilename, setUpload, handleProgress]);
|
||||||
|
|
||||||
const cancelUpload = useCallback(async () => {
|
const cancelUpload = useCallback(async () => {
|
||||||
|
// Abort the retry loop first (covers a cancel during the back-off sleep, when
|
||||||
|
// there is no in-flight request), then abort any in-flight upload request.
|
||||||
|
abortRef.current?.abort();
|
||||||
if (upload.status === UploadStatus.Loading) {
|
if (upload.status === UploadStatus.Loading) {
|
||||||
await mx.cancelUpload(upload.promise);
|
await mx.cancelUpload(upload.promise);
|
||||||
}
|
}
|
||||||
|
|||||||
+33
-3
@@ -144,6 +144,9 @@ export type ContentUploadOptions = {
|
|||||||
onProgress?: (progress: UploadProgress) => void;
|
onProgress?: (progress: UploadProgress) => void;
|
||||||
onSuccess: (mxc: string) => void;
|
onSuccess: (mxc: string) => void;
|
||||||
onError: (error: MatrixError) => void;
|
onError: (error: MatrixError) => void;
|
||||||
|
// Aborts the retry loop, including while it's sleeping between attempts (a
|
||||||
|
// plain mx.cancelUpload only aborts an in-flight request, not the back-off).
|
||||||
|
signal?: AbortSignal;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Build a MatrixError defensively from an unexpected upload response.
|
// Build a MatrixError defensively from an unexpected upload response.
|
||||||
@@ -192,16 +195,38 @@ export const uploadContent = async (
|
|||||||
file: TUploadContent,
|
file: TUploadContent,
|
||||||
options: ContentUploadOptions,
|
options: ContentUploadOptions,
|
||||||
) => {
|
) => {
|
||||||
const { name, fileType, hideFilename, onProgress, onPromise, onSuccess, onError } = options;
|
const { name, fileType, hideFilename, onProgress, onPromise, onSuccess, onError, signal } =
|
||||||
|
options;
|
||||||
|
|
||||||
|
// Resolves after `ms`, or early if `signal` aborts (so a cancel during the
|
||||||
|
// back-off is honored immediately instead of waiting out the delay).
|
||||||
const sleepForMs = (ms: number) =>
|
const sleepForMs = (ms: number) =>
|
||||||
new Promise((resolve) => {
|
new Promise<void>((resolve) => {
|
||||||
setTimeout(resolve, ms);
|
if (signal?.aborted) {
|
||||||
|
resolve();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const onAbort = () => {
|
||||||
|
clearTimeout(timeout);
|
||||||
|
resolve();
|
||||||
|
};
|
||||||
|
const timeout = setTimeout(() => {
|
||||||
|
signal?.removeEventListener('abort', onAbort);
|
||||||
|
resolve();
|
||||||
|
}, ms);
|
||||||
|
signal?.addEventListener('abort', onAbort, { once: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const abortError = () =>
|
||||||
|
matrixErrorFromUnknown(new DOMException('Upload cancelled', 'AbortError'));
|
||||||
|
|
||||||
let lastError: MatrixError | undefined;
|
let lastError: MatrixError | undefined;
|
||||||
|
|
||||||
for (let retryCount = 0; retryCount <= UPLOAD_MAX_RETRY_COUNT; retryCount += 1) {
|
for (let retryCount = 0; retryCount <= UPLOAD_MAX_RETRY_COUNT; retryCount += 1) {
|
||||||
|
if (signal?.aborted) {
|
||||||
|
onError(abortError());
|
||||||
|
return;
|
||||||
|
}
|
||||||
const uploadPromise = mx.uploadContent(file, {
|
const uploadPromise = mx.uploadContent(file, {
|
||||||
name,
|
name,
|
||||||
type: fileType,
|
type: fileType,
|
||||||
@@ -236,6 +261,11 @@ export const uploadContent = async (
|
|||||||
Math.min(1000 * 2 ** retryCount, 30_000);
|
Math.min(1000 * 2 ** retryCount, 30_000);
|
||||||
// eslint-disable-next-line no-await-in-loop
|
// eslint-disable-next-line no-await-in-loop
|
||||||
await sleepForMs(waitMS);
|
await sleepForMs(waitMS);
|
||||||
|
// Cancelled during the back-off — stop instead of resurrecting the upload.
|
||||||
|
if (signal?.aborted) {
|
||||||
|
onError(abortError());
|
||||||
|
return;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user