Skip to content

Commit fc7aec7

Browse files
7w17w1
authored
Some potential sync request spam fixes (#736)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> As I haven't really had any issues I'm not 100% sure this fixes anything at all, but it at least doesn't break anything. Increase the timeout for background clients to 60s, separates them into their own store, and makes sliding sync respect the setting instead of the client setting, and hopefully better management. Adds a startup timeout of 45s to the main loading screen to prevent anything from getting stuck behind that which may or may not actually do anything. Replaces the sdk's thread root event fetching on initial load to prevent the useless loading delay and request spam on load. Get's swapped back once the client loads which then functions normally, only loading thread events when a room is opened. Also some random formatting changes that happened in initMatrix.ts when I saved for some reason?? They're more readable imo so I just left them in. Also fixed the 1 lint issue from #730 Maybe related to #710 #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings ### AI disclosure: - [x] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> Bickered with an AI to diagnose some potential syncing issues but none of the changes were directly AI written.
2 parents 7bff2bd + 04c8f40 commit fc7aec7

5 files changed

Lines changed: 179 additions & 28 deletions

File tree

‎.changeset/fix_background_sync.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Some fixes to sync requests being spammed on loading screen and for multi-account background syncing, it should also load faster now!

‎src/app/pages/client/BackgroundNotifications.tsx‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { MatrixClient, MatrixEvent, Room } from '$types/matrix-sdk';
33
import {
44
ClientEvent,
55
createClient,
6+
IndexedDBStore,
67
MatrixEventEvent,
78
RoomEvent,
89
SyncState,
@@ -47,25 +48,47 @@ import { mobileOrTablet } from '$utils/user-agent';
4748

4849
const log = createLogger('BackgroundNotifications');
4950
const debugLog = createDebugLogger('BackgroundNotifications');
51+
52+
const BACKGROUND_SYNC_POLL_TIMEOUT_MS = 60_000;
53+
const BACKGROUND_STAGGER_DELAY_MS = 5_000;
54+
5055
const isClientReadyForNotifications = (state: SyncState | string | null): boolean =>
5156
state === SyncState.Prepared || state === SyncState.Syncing || state === SyncState.Catchup;
5257

5358
const startBackgroundClient = async (
5459
session: Session,
5560
slidingSyncConfig: ReturnType<typeof useClientConfig>['slidingSync']
5661
): Promise<MatrixClient> => {
62+
const storeName = {
63+
sync: `bg-sync${session.userId}`,
64+
crypto: `bg-crypto${session.userId}`,
65+
rustCryptoPrefix: `bg-sync${session.userId}`,
66+
};
67+
68+
const indexedDBStore = new IndexedDBStore({
69+
indexedDB: global.indexedDB,
70+
localStorage: global.localStorage,
71+
dbName: storeName.sync,
72+
});
73+
5774
const mx = createClient({
5875
baseUrl: session.baseUrl,
5976
accessToken: session.accessToken,
6077
userId: session.userId,
6178
deviceId: session.deviceId,
79+
store: indexedDBStore,
6280
timelineSupport: false,
6381
});
64-
await startClient(mx, {
82+
83+
const startOpts = {
6584
baseUrl: session.baseUrl,
66-
slidingSync: slidingSyncConfig,
85+
slidingSync: session.slidingSyncOptIn ? slidingSyncConfig : undefined,
6786
sessionSlidingSyncOptIn: session.slidingSyncOptIn,
68-
});
87+
pollTimeoutMs: BACKGROUND_SYNC_POLL_TIMEOUT_MS,
88+
timelineLimit: 1,
89+
};
90+
91+
await startClient(mx, startOpts);
6992
return mx;
7093
};
7194

@@ -136,9 +159,11 @@ export function BackgroundNotifications() {
136159
// listeners before stopping a background client.
137160
const clientCleanupRef = useRef(new Map());
138161

162+
const activeUserId = activeSessionId ?? sessions[0]?.userId;
163+
139164
const inactiveSessions = useMemo(
140-
() => sessions.filter((s) => s.userId !== (activeSessionId ?? sessions[0]?.userId)),
141-
[sessions, activeSessionId]
165+
() => sessions.filter((s) => s.userId !== activeUserId),
166+
[sessions, activeUserId]
142167
);
143168
// Ref so retry setTimeout callbacks can access the current session list
144169
// without stale closures.
@@ -540,20 +565,30 @@ export function BackgroundNotifications() {
540565
});
541566
};
542567

543-
inactiveSessions.forEach((session) => {
544-
if (!current.has(session.userId)) startSession(session);
568+
const pendingSessions = inactiveSessions.filter((s) => !current.has(s.userId));
569+
const staggerTimers: ReturnType<typeof setTimeout>[] = [];
570+
pendingSessions.forEach((session, idx) => {
571+
if (idx === 0) {
572+
startSession(session);
573+
} else {
574+
staggerTimers.push(
575+
setTimeout(() => startSession(session), idx * BACKGROUND_STAGGER_DELAY_MS)
576+
);
577+
}
545578
});
546579

547-
// Capture the cleanup map for this effect instance so teardown runs against
548-
// the listeners registered while this effect was active.
549580
const cleanupMap = clientCleanupRef.current;
581+
const activeUserIds = new Set(inactiveSessions.map((s) => s.userId));
550582
return () => {
583+
staggerTimers.forEach(clearTimeout);
551584
current.forEach((mx, userId) => {
552-
cleanupMap.get(userId)?.();
553-
cleanupMap.delete(userId);
554-
stopClient(mx);
585+
if (!activeUserIds.has(userId)) {
586+
cleanupMap.get(userId)?.();
587+
cleanupMap.delete(userId);
588+
stopClient(mx);
589+
current.delete(userId);
590+
}
555591
});
556-
current.clear();
557592
};
558593
}, [
559594
clientConfig.slidingSync,

‎src/app/utils/msc4459helper.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ function getImagePackReferencesForMxcInternal(
4747
.map((pack) => {
4848
const img = pack.getImages(imageUsage).find((val) => val.url === mxcUrl);
4949
const room = matrixClient.getRoom(pack.address?.roomId);
50-
if (!room || (isRoomPrivate(matrixClient, room) && !bypassPrivateFilter)) return;
50+
if (!room || (isRoomPrivate(matrixClient, room) && !bypassPrivateFilter)) return undefined;
5151
const viaServers = new SerializableSet<string>();
5252
if (room)
5353
getViaServers(room).forEach((via) => {

‎src/client/initMatrix.ts‎

Lines changed: 124 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { CryptoCallbacks, MatrixClient, ISyncStateData } from '$types/matri
22
import {
33
ClientEvent,
44
createClient,
5+
Filter,
56
IndexedDBStore,
67
IndexedDBCryptoStore,
78
SyncState,
@@ -26,7 +27,7 @@ const classicSyncObserverByClient = new WeakMap<
2627
MatrixClient,
2728
(state: SyncState, prevState: SyncState | null, data?: ISyncStateData) => void
2829
>();
29-
const FAST_SYNC_POLL_TIMEOUT_MS = 10000;
30+
const FAST_SYNC_POLL_TIMEOUT_MS = 30_000;
3031
const SLIDING_SYNC_POLL_TIMEOUT_MS = 20000;
3132
type SyncTransport = 'classic' | 'sliding';
3233
type SyncTransportReason =
@@ -47,8 +48,54 @@ type SyncTransportMeta = {
4748
reason: SyncTransportReason;
4849
};
4950
const syncTransportByClient = new WeakMap<MatrixClient, SyncTransportMeta>();
51+
const fetchRoomEventStartupCleanupByClient = new WeakMap<MatrixClient, () => void>();
5052
const COLD_CACHE_BOOTSTRAP_TIMEOUT_MS = 20000;
5153

54+
type FetchRoomEventResult = Awaited<ReturnType<MatrixClient['fetchRoomEvent']>>;
55+
type MatrixClientWithWritableFetchRoomEvent = MatrixClient & {
56+
fetchRoomEvent: (roomId: string, eventId: string) => Promise<FetchRoomEventResult>;
57+
};
58+
59+
// Replace fetchRoomEvent for first sync so thread roots don't each trigger GET /event
60+
// Uses cached timeline events when present otherwise a stub that fetches when user opens thread
61+
function installStartupFetchRoomEventPatch(mx: MatrixClient): void {
62+
fetchRoomEventStartupCleanupByClient.get(mx)?.();
63+
64+
const mxWritable = mx as MatrixClientWithWritableFetchRoomEvent;
65+
const origFetchRoomEvent = mx.fetchRoomEvent.bind(mx);
66+
let restored = false;
67+
68+
const restore = () => {
69+
if (restored) return;
70+
restored = true;
71+
fetchRoomEventStartupCleanupByClient.delete(mx);
72+
// Put the real fetchRoomEvent back and detach this
73+
mxWritable.fetchRoomEvent = origFetchRoomEvent;
74+
mx.off(ClientEvent.Sync, onSync);
75+
};
76+
77+
const onSync = (state: SyncState) => {
78+
// Initial sync burst is over, let normal server fetches run again
79+
if (state === SyncState.Prepared || state === SyncState.Syncing) {
80+
restore();
81+
}
82+
};
83+
84+
mxWritable.fetchRoomEvent = (roomId: string, eventId: string) => {
85+
if (restored) return origFetchRoomEvent(roomId, eventId);
86+
const cachedEvent = mx.getRoom(roomId)?.findEventById(eventId);
87+
// Reuse sync payload instead of another GET when we already have the root.
88+
const payload: FetchRoomEventResult = cachedEvent?.event ?? {
89+
event_id: eventId,
90+
room_id: roomId,
91+
};
92+
return Promise.resolve(payload);
93+
};
94+
95+
mx.on(ClientEvent.Sync, onSync);
96+
fetchRoomEventStartupCleanupByClient.set(mx, restore);
97+
}
98+
5299
export const resolveSlidingEnabled = (enabled: SlidingSyncConfig['enabled']): boolean => {
53100
if (enabled === undefined) return false;
54101
if (typeof enabled === 'boolean') return enabled;
@@ -315,7 +362,9 @@ export const initClient = async (session: Session): Promise<MatrixClient> => {
315362

316363
const wipeAllStores = async () => {
317364
log.warn('initClient: wiping all stores for', session.userId);
318-
debugLog.warn('sync', 'Wiping all stores due to mismatch', { userId: session.userId });
365+
debugLog.warn('sync', 'Wiping all stores due to mismatch', {
366+
userId: session.userId,
367+
});
319368
Sentry.addBreadcrumb({
320369
category: 'crypto',
321370
message: 'Crypto store mismatch — wiping local stores and retrying',
@@ -353,18 +402,24 @@ export const initClient = async (session: Session): Promise<MatrixClient> => {
353402
}
354403

355404
try {
356-
await mx.initRustCrypto({ cryptoDatabasePrefix: storeName.rustCryptoPrefix });
405+
await mx.initRustCrypto({
406+
cryptoDatabasePrefix: storeName.rustCryptoPrefix,
407+
});
357408
} catch (err) {
358409
if (!isMismatch(err)) {
359410
debugLog.error('sync', 'Failed to initialize crypto', { error: err });
360411
throw err;
361412
}
362413
log.warn('initClient: mismatch on initRustCrypto — wiping and retrying:', err);
363-
debugLog.warn('sync', 'Crypto init mismatch - wiping stores and retrying', { error: err });
414+
debugLog.warn('sync', 'Crypto init mismatch - wiping stores and retrying', {
415+
error: err,
416+
});
364417
mx.stopClient();
365418
await wipeAllStores();
366419
mx = await buildClient(session);
367-
await mx.initRustCrypto({ cryptoDatabasePrefix: storeName.rustCryptoPrefix });
420+
await mx.initRustCrypto({
421+
cryptoDatabasePrefix: storeName.rustCryptoPrefix,
422+
});
368423
}
369424

370425
mx.setMaxListeners(50);
@@ -375,6 +430,8 @@ export type StartClientConfig = {
375430
baseUrl?: string;
376431
slidingSync?: SlidingSyncConfig;
377432
sessionSlidingSyncOptIn?: boolean;
433+
pollTimeoutMs?: number;
434+
timelineLimit?: number;
378435
};
379436

380437
export type ClientSyncDiagnostics = SyncTransportMeta & {
@@ -415,7 +472,12 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig):
415472
hasProxy: hasSlidingProxy,
416473
});
417474

418-
const startClassicSync = async (fallbackFromSliding: boolean, reason: SyncTransportReason) => {
475+
const CLASSIC_SYNC_STARTUP_TIMEOUT_MS = 45_000;
476+
477+
const startClassicSync = async (
478+
fallbackFromSliding: boolean,
479+
reason: SyncTransportReason
480+
): Promise<void> => {
419481
syncTransportByClient.set(mx, {
420482
transport: 'classic',
421483
slidingConfigured: slidingEnabledOnServer,
@@ -426,13 +488,52 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig):
426488
reason,
427489
});
428490
Sentry.metrics.count('sable.sync.transport', 1, {
429-
attributes: { transport: 'classic', reason, fallback: String(fallbackFromSliding) },
491+
attributes: {
492+
transport: 'classic',
493+
reason,
494+
fallback: String(fallbackFromSliding),
495+
},
430496
});
431-
await mx.startClient({
432-
lazyLoadMembers: true,
433-
pollTimeout: FAST_SYNC_POLL_TIMEOUT_MS,
434-
threadSupport: true,
497+
498+
const startupTimeout = new Promise<void>((resolve) => {
499+
window.setTimeout(() => {
500+
debugLog.warn('sync', 'Classic sync startup timed out', {
501+
userId: mx.getUserId(),
502+
timeoutMs: CLASSIC_SYNC_STARTUP_TIMEOUT_MS,
503+
});
504+
resolve();
505+
}, CLASSIC_SYNC_STARTUP_TIMEOUT_MS);
435506
});
507+
508+
const effectivePollTimeout = config?.pollTimeoutMs ?? FAST_SYNC_POLL_TIMEOUT_MS;
509+
const effectiveTimelineLimit = config?.timelineLimit ?? 10;
510+
511+
const classicFilter = new Filter(mx.getUserId() ?? undefined);
512+
classicFilter.setTimelineLimit(effectiveTimelineLimit);
513+
// Ensure lazy loading stays on (carried by buildDefaultFilter but explicit here
514+
// since we replace the filter entirely rather than merging).
515+
const filterDefinition = classicFilter.getDefinition();
516+
if (filterDefinition.room) {
517+
filterDefinition.room.timeline = filterDefinition.room.timeline ?? {};
518+
(filterDefinition.room.timeline as { lazy_load_members?: boolean }).lazy_load_members = true;
519+
}
520+
521+
installStartupFetchRoomEventPatch(mx);
522+
523+
let syncStarted: Promise<void>;
524+
try {
525+
syncStarted = mx.startClient({
526+
lazyLoadMembers: true,
527+
pollTimeout: effectivePollTimeout,
528+
threadSupport: true,
529+
filter: classicFilter,
530+
});
531+
} catch (syncErr) {
532+
fetchRoomEventStartupCleanupByClient.get(mx)?.();
533+
throw syncErr;
534+
}
535+
536+
await Promise.race([syncStarted, startupTimeout]);
436537
// Attach an ongoing classic-sync observer — equivalent to SlidingSyncManager's
437538
// onLifecycle listener. Tracks state transitions, initial-sync timing, and errors.
438539
let classicSyncCount = 0;
@@ -584,16 +685,22 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig):
584685
reason: 'sliding_active',
585686
});
586687
Sentry.metrics.count('sable.sync.transport', 1, {
587-
attributes: { transport: 'sliding', reason: 'sliding_active', fallback: 'false' },
688+
attributes: {
689+
transport: 'sliding',
690+
reason: 'sliding_active',
691+
fallback: 'false',
692+
},
588693
});
589694

590695
try {
696+
installStartupFetchRoomEventPatch(mx);
591697
await mx.startClient({
592698
lazyLoadMembers: true,
593699
slidingSync: manager.slidingSync,
594700
threadSupport: true,
595701
});
596702
} catch (err) {
703+
fetchRoomEventStartupCleanupByClient.get(mx)?.();
597704
debugLog.error('network', 'Failed to start client with sliding sync', {
598705
error: err instanceof Error ? err.message : String(err),
599706
userId: mx.getUserId(),
@@ -608,6 +715,7 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig):
608715
export const stopClient = (mx: MatrixClient): void => {
609716
log.log('stopClient', mx.getUserId());
610717
debugLog.info('sync', 'Stopping client', { userId: mx.getUserId() });
718+
fetchRoomEventStartupCleanupByClient.get(mx)?.();
611719
disposeSlidingSync(mx);
612720
const classicSyncListener = classicSyncObserverByClient.get(mx);
613721
if (classicSyncListener) {
@@ -649,7 +757,10 @@ export const getClientSyncDiagnostics = (mx: MatrixClient): ClientSyncDiagnostic
649757
* so the correct Jotai Provider store is used.
650758
*/
651759
export const logoutClient = async (mx: MatrixClient, session?: Session) => {
652-
log.log('logoutClient', { userId: mx.getUserId(), sessionUserId: session?.userId });
760+
log.log('logoutClient', {
761+
userId: mx.getUserId(),
762+
sessionUserId: session?.userId,
763+
});
653764
debugLog.info('general', 'Logging out client', { userId: mx.getUserId() });
654765
pushSessionToSW();
655766
stopClient(mx);

‎src/types/matrix-sdk.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export * from 'matrix-js-sdk/lib/sliding-sync';
1212
export * from 'matrix-js-sdk/lib/sync-accumulator';
1313
export * from 'matrix-js-sdk/lib/scheduler';
1414
export * from 'matrix-js-sdk/lib/store/memory';
15-
export { createClient } from 'matrix-js-sdk/lib/matrix';
15+
export { createClient, Filter } from 'matrix-js-sdk/lib/matrix';
1616

1717
export * from 'matrix-js-sdk/lib/models/event';
1818
export * from 'matrix-js-sdk/lib/models/room';

0 commit comments

Comments
 (0)