From 9a04cc455654e8cf2782ee42478536aefa354556 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Mon, 24 Aug 2026 12:16:43 +0200 Subject: [PATCH 1/9] feat: llc driven connection recovery initial pass --- AGENTS.md | 12 ++- package/src/components/Channel/Channel.tsx | 82 ++++++++++++----- .../Channel/__tests__/Channel.test.tsx | 88 ++++++++++++++++++- .../__tests__/ChannelList.test.tsx | 19 ++-- .../ChannelList/hooks/usePaginatedChannels.ts | 32 ++----- package/src/components/Chat/Chat.tsx | 3 - 6 files changed, 172 insertions(+), 64 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8097495dec..33eeec42b8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,7 +130,17 @@ yarn workspace sampleapp android └─ ``` -`` is the entry point. It sets SDK metadata on the `stream-chat` client (identifier, device info), disables the JS client's `recoverStateOnReconnect` (the SDK handles recovery itself), registers subscriptions for threads/polls/reminders (cleaned up on unmount), initializes `OfflineDB` when `enableOfflineSupport` is set, and wraps children in `ChatProvider` → `TranslationProvider` → `ThemeProvider` → `ChannelsStateProvider`. +`` is the entry point. It sets SDK metadata on the `stream-chat` client (identifier, device info), registers subscriptions for threads/polls/reminders (cleaned up on unmount), initializes `OfflineDB` when `enableOfflineSupport` is set, and wraps children in `ChatProvider` → `TranslationProvider` → `ThemeProvider`. + +**Reconnect recovery is owned by the JS client, not this SDK.** `client.connectionRecovery` re-runs +each channel list's own first-page query and reloads whichever channels are `active` (`` +marks its channel active on mount), then dispatches `connection.recovered`. So do not add a +`connection.changed` listener that re-queries a list or re-watches a channel — that duplicates it. +`recoverStateOnReconnect` is left at its default `true`; it used to be switched off here because the +client's old recovery was a single 30-channel query that ignored the lists' own filters. Two things +deliberately stay UI-side: mark-read after the reload (`` listens for +`connection.recovered`), and the open thread's reply refresh (`resyncThread` in `Channel.tsx`, since +nothing in the client recovers an open thread's replies yet). ### Context three-layer pattern diff --git a/package/src/components/Channel/Channel.tsx b/package/src/components/Channel/Channel.tsx index 6cb8123fac..f78a16a958 100644 --- a/package/src/components/Channel/Channel.tsx +++ b/package/src/components/Channel/Channel.tsx @@ -350,6 +350,10 @@ const availableCommandsSelector = (state: ChannelConfig) => ({ availableCommands: state.availableCommands, }); +const reloadErrorSelector = (state: { lastReloadError?: Error }) => ({ + lastReloadError: state.lastReloadError, +}); + const messageFocusSignalSelector = (state: { signal: { messageId?: string } | null }) => ({ highlightedMessageId: state.signal?.messageId, }); @@ -501,6 +505,12 @@ const ChannelWithContext = (props: PropsWithChildren) = messageFocusSignalSelector, ); + // A failed reconnect reload is published on the channel rather than thrown at us, because the reload + // is issued by `client.connectionRecovery` inside a `Promise.allSettled`. Reading it here is what + // keeps the "could not load messages" indicator working now that this component no longer owns the + // call. ORed with the local `error` below, which still covers the mount-time `watch()` failure. + const { lastReloadError } = useStateStore(channel?.state, reloadErrorSelector) ?? {}; + /** * This ref keeps track of message IDs which have already been optimistically updated. * We need it to make sure we don't react on message.new/notification.message_new events @@ -684,7 +694,16 @@ const ChannelWithContext = (props: PropsWithChildren) = // instance via useMarkRead(channel). Channel still needs it internally (mark-read-on-mount + resync). const markRead = useMarkRead(channel); - const resyncChannel = useStableCallback(async () => { + /** + * Reconnect refresh for an open THREAD's replies. + * + * The channel half of this used to live here too; it is now owned by `client.connectionRecovery`, + * which reloads whichever channels are active. Threads still need their own pass: the LLC's + * `ThreadManager` recovery refreshes the thread *list*, and `Thread`'s own stale-state recovery keys + * off `user.watching.stop` rather than a reconnect — so nothing else brings an open thread's replies + * back. Keep this until thread recovery moves down as its own piece of work. + */ + const resyncThread = useStableCallback(async () => { if (!channel || syncingChannelRef.current || (!channel.initialized && !channel.offlineMode)) { return; } @@ -711,19 +730,7 @@ const ChannelWithContext = (props: PropsWithChildren) = .map(parseMessage); try { - if (!thread) { - // The LLC owns the reconnect refresh now: channel.reload() re-watches, folds the newest page, - // and reconciles messages hard-deleted while offline — capturing the pre-fetch snapshot + the - // requested limit itself, so this no longer passes them (see Channel.reload / - // MessagePaginator.mergeNewestPage). - await channel.reload(); - // Only mark read when the refreshed window is at the newest (hasMoreHead false); if the user - // has paginated up into older history, leave their read state untouched. - const atLatest = !channel.messagePaginator.hasMoreHead; - if (atLatest) { - await markRead(); - } - } else if (threadInstance) { + if (threadInstance) { await threadInstance.reload(); const currentThreadMessages = @@ -746,14 +753,22 @@ const ChannelWithContext = (props: PropsWithChildren) = syncingChannelRef.current = false; }); - // resync channel is added to ref so that it can be used in useEffect without adding it as a dependency - const resyncChannelRef = useRef(resyncChannel); - resyncChannelRef.current = resyncChannel; + // held in a ref so the effect below need not take it as a dependency + const resyncThreadRef = useRef(resyncThread); + resyncThreadRef.current = resyncThread; + // Trigger for the thread resync above. Deliberately unchanged: with offline support enabled it hangs + // off the offline DB's sync-status change rather than `connection.changed`, because that is published + // only after pending-task replay and `sync()` have run — the reply refresh has to land after them, + // not race them. useEffect(() => { + if (!thread) { + return; + } + const connectionChangedHandler = () => { if (shouldSyncChannel) { - resyncChannelRef.current(); + resyncThreadRef.current(); } }; let connectionChangedSubscription: ReturnType; @@ -776,7 +791,27 @@ const ChannelWithContext = (props: PropsWithChildren) = return () => { connectionChangedSubscription.unsubscribe(); }; - }, [enableOfflineSupport, client, shouldSyncChannel]); + }, [enableOfflineSupport, client, shouldSyncChannel, thread]); + + // Mark-read after the LLC's reconnect reload. `connection.recovered` is dispatched by + // `client.connectionRecovery` once that reload has landed, so `hasMoreHead` read here reflects the + // refreshed window — which is why this cannot hang off `connection.changed`. Only the reload moved + // into the LLC; whether a caught-up channel is marked read stays a UI decision (see `useMarkRead`). + useEffect(() => { + if (thread || !shouldSyncChannel) { + return; + } + + const { unsubscribe } = client.on('connection.recovered', () => { + // Only when the refreshed window is at the newest. If the user has paginated up into older + // history, leave their read state alone. + if (!channel.messagePaginator.hasMoreHead) { + markRead(); + } + }); + + return unsubscribe; + }, [channel, client, markRead, shouldSyncChannel, thread]); /** * Channel configs for use in disabling local functionality. @@ -839,10 +874,11 @@ const ChannelWithContext = (props: PropsWithChildren) = // (see the useChannelRequestHandlers call below) so it runs INSIDE the stream-chat send pipeline — // after the LLC's optimistic ingest (message already shows pending), before the POST — awaiting // `client.uploadManager` to finish the in-flight uploads and swapping local preview URLs for the - // returned CDN URLs. This deliberately stays RN-side: native image compression (`compressedImageURI`) + // returned CDN URLs. It lives here for now because native image compression (`compressedImageURI`) // and a custom uploader registered through `client.config` must remain reachable, and the - // sendMessageRequest seam lets it run in the right place without a pre-ingest or any LLC change. It is NOT slated to move - // into the LLC — this handler is its intended home. + // sendMessageRequest seam lets it run in the right place without a pre-ingest or any LLC change. + // It IS slated to move into the LLC, just not yet — and that move is what lets the + // `doSendMessageRequest` prop and its wrapper in `useChannelRequestHandlers` go. const uploadPendingAttachments = useStableCallback(async (message: LocalMessage) => { if (!message.attachments?.length || !channel?.cid) { return; @@ -988,7 +1024,7 @@ const ChannelWithContext = (props: PropsWithChildren) = disabled: !!channel?.data?.frozen, enableMessageGroupingByUser, enforceUniqueReaction, - error, + error: error || lastReloadError || false, hideDateSeparators, hideStickyDateHeader, highlightedMessageId, diff --git a/package/src/components/Channel/__tests__/Channel.test.tsx b/package/src/components/Channel/__tests__/Channel.test.tsx index 4da6d8905a..9fdda2cea3 100644 --- a/package/src/components/Channel/__tests__/Channel.test.tsx +++ b/package/src/components/Channel/__tests__/Channel.test.tsx @@ -3,7 +3,7 @@ import { View } from 'react-native'; import { act, cleanup, render, waitFor } from '@testing-library/react-native'; import type { Channel as ChannelType, StreamChat as StreamChatType } from 'stream-chat'; -import { StreamChat } from 'stream-chat'; +import { StreamChat, Thread } from 'stream-chat'; import type { ChannelContextValue } from '../../../contexts/channelContext/ChannelContext'; import { ChannelContext, ChannelProvider } from '../../../contexts/channelContext/ChannelContext'; @@ -562,7 +562,10 @@ describe('Channel initial load useEffect', () => { }); }); - it('should call resyncChannel when connection changed event is triggered', async () => { + it('reloads the channel on reconnect while preserving failed messages', async () => { + // The reload is issued by `client.connectionRecovery` now, not by this component — `` + // only marks the channel active. Asserted end to end on purpose: what matters is that a reconnect + // still refreshes the open channel and still does not lose locally-unsent messages. // Deterministic timestamps so the 10 loaded messages and the 10 offline-failed messages occupy // adjacent, ordered positions in the paginator's active window. const baseTime = 1600000000000; @@ -602,11 +605,88 @@ describe('Channel initial load useEffect', () => { act(() => dispatchConnectionChanged(chatClient)); }); - // resyncChannel re-watches via channel.reload() and reconciles, but preserves the failed - // (locally-unsent) messages — the 10 originals + 10 failed remain. + // The reload re-watches and reconciles, but preserves the failed (locally-unsent) messages — + // the 10 originals + 10 failed remain. await waitFor(() => { expect(reloadSpy).toHaveBeenCalled(); expect(channel.messagePaginator.headItems.length).toBe(20); }); }); + + // Regression guard for the reconnect refresh of an OPEN THREAD's replies. + // + // The channel half of the old `resyncChannel` moved into `client.connectionRecovery`; the thread half + // deliberately did NOT, because nothing else brings an open thread's replies back — `ThreadManager` + // recovery refreshes the thread *list*, and `Thread`'s own stale-state recovery keys off + // `user.watching.stop` rather than a reconnect. Deleting this branch while moving the rest is an easy + // mistake to make and produces no other failure, so it is pinned here. + it('reloads an open thread on reconnect', async () => { + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + await testChannel.watch(); + + const parentMessage = generateMessage({ user }); + const threadInstance = new Thread({ + channel: testChannel, + client: chatClient, + parentMessage: testChannel.state.formatMessage(parentMessage), + }); + const reload = jest.spyOn(threadInstance, 'reload').mockResolvedValue(undefined); + + render( + + the one that owns the thread resync + // (`shouldSyncChannel`); without it the channel view would claim it instead. + threadList + thread={{ thread: testChannel.state.formatMessage(parentMessage), threadInstance }} + /> + , + ); + + act(() => dispatchConnectionChanged(chatClient, false)); + act(() => dispatchConnectionChanged(chatClient)); + + await waitFor(() => expect(reload).toHaveBeenCalled()); + }); + + it('surfaces a failed reconnect reload on the channel context', async () => { + // The reload is issued by `client.connectionRecovery` inside a `Promise.allSettled`, so a failure + // never reaches this component as a throw. It arrives on `channel.state.lastReloadError` instead, + // and has to reach `ChannelContext.error` — that is what drives the "Error loading messages for + // this channel…" indicator. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + await testChannel.watch(); + + // Rendered inline rather than through this describe's `renderComponent`, which does not take a + // context probe. + let contextError: unknown; + render( + + + { + contextError = (ctx as { error: unknown }).error; + }} + context={ChannelContext as React.Context} + /> + + , + ); + // Let the mount settle with a healthy channel first, so the error asserted below can only have + // come from the reconnect reload and not from the mount-time watch. + await waitFor(() => expect(contextError).toBe(false)); + + const failure = new Error('reload failed'); + jest.spyOn(testChannel, 'watch').mockRejectedValue(failure); + + act(() => dispatchConnectionChanged(chatClient, false)); + act(() => dispatchConnectionChanged(chatClient)); + + await waitFor(() => expect(contextError).toBe(failure)); + }); }); diff --git a/package/src/components/ChannelList/__tests__/ChannelList.test.tsx b/package/src/components/ChannelList/__tests__/ChannelList.test.tsx index e4d4fde547..fc4f58b893 100644 --- a/package/src/components/ChannelList/__tests__/ChannelList.test.tsx +++ b/package/src/components/ChannelList/__tests__/ChannelList.test.tsx @@ -836,11 +836,16 @@ describe('ChannelList', () => { }); describe('connection.changed', () => { - it('should force reconnection refreshes past the pull-to-refresh debounce while keeping them out of the refreshing UI', async () => { - // Regression guard: a reconnect is the sole trigger that re-watches channels on the fresh - // socket, so it must bypass the 5s pull-to-refresh throttle (`force`). Without the bypass a - // second reconnect landing inside the debounce window is dropped and its channels stay - // un-watched (frozen last message / unread) until the next reconnect > 5s later. + it('refreshes on every reconnect, however close together, without surfacing in the refreshing UI', async () => { + // Regression guard for a shipped freeze bug: a reconnect is the trigger that re-watches + // channels on the fresh socket, so dropping one leaves its channels un-watched and their + // per-channel state (last message / unread) frozen until the next reconnect or an app reload. + // The list reorders anyway off member-level `notification.message_new`, which is what made it + // look like the connection was fine. + // + // Recovery is now owned by `client.connectionRecovery`, which has no throttle at all — the + // 5s window this used to have to be forced past belongs to pull-to-refresh only and is no + // longer on the reconnect path. The behaviour asserted here is unchanged. useMockedApis(chatClient, [queryChannelsApi([testChannel1])]); // Freeze the clock at t=0 for the whole mount so `lastRefresh` is seeded to 0 regardless of // how many `Date.now()` calls the render makes. @@ -881,8 +886,8 @@ describe('ChannelList', () => { ).toBe(false); }); - // Reconnect #2 at t=6000, i.e. 0ms after reconnect #1 → inside the debounce window. It fires a - // fresh query only because reconnection refreshes are forced past the throttle. + // Reconnect #2 at t=6000, i.e. 0ms after reconnect #1 — well inside what used to be the + // debounce window. It must still fire a fresh query. act(() => dispatchConnectionChangedEvent(chatClient, false)); act(() => dispatchConnectionChangedEvent(chatClient, true)); await waitFor(() => { diff --git a/package/src/components/ChannelList/hooks/usePaginatedChannels.ts b/package/src/components/ChannelList/hooks/usePaginatedChannels.ts index a6381de068..27b41acbba 100644 --- a/package/src/components/ChannelList/hooks/usePaginatedChannels.ts +++ b/package/src/components/ChannelList/hooks/usePaginatedChannels.ts @@ -169,19 +169,13 @@ export const usePaginatedChannels = ({ ); const refreshList = useStableCallback( - async ({ - force = false, - isBackground = false, - }: { force?: boolean; isBackground?: boolean } = {}) => { + async ({ isBackground = false }: { isBackground?: boolean } = {}) => { const now = Date.now(); - // Only allow pull-to-refresh 5 seconds after the last successful refresh. A reconnect (`force`) - // must bypass this throttle: it is the sole trigger that re-establishes channel watches after the - // socket reopens (the JS client's own recovery is disabled via `recoverStateOnReconnect = false`), - // so debouncing it leaves the channels un-watched — the list still reorders on member-level - // `notification.message_new`, but per-channel state (last message / unread) stays frozen until the - // next reconnect > 5s later or an app reload. This bites both a reconnect < 5s after launch - // (`lastRefresh` is seeded to mount time) and two reconnects < 5s apart. - if (!force && now - lastRefresh.current < RETRY_INTERVAL_IN_MS && error === undefined) { + // Only allow pull-to-refresh 5 seconds after the last successful refresh. This throttle is now + // purely about the user pulling repeatedly: reconnect refreshes no longer come through here at + // all — `client.connectionRecovery` owns them and re-runs each list's own query directly on the + // paginator, so they can never be swallowed by this window. + if (now - lastRefresh.current < RETRY_INTERVAL_IN_MS && error === undefined) { return; } @@ -214,20 +208,6 @@ export const usePaginatedChannels = ({ } reloadList(); - - const listener: ReturnType = client.on( - 'connection.changed', - async (event) => { - if (event.online) { - // Reconnection refreshes stay silent (`isBackground`) but must NOT be throttled by the - // pull-to-refresh debounce (`force`) — this is the query that re-watches the channels on the - // fresh socket. See the `force` note in `refreshList`. - await refreshList({ force: true, isBackground: true }); - } - }, - ); - - return () => listener?.unsubscribe?.(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [filterStr, optionsStr, sortStr, paginator]); diff --git a/package/src/components/Chat/Chat.tsx b/package/src/components/Chat/Chat.tsx index f7c39d819f..1711a33582 100644 --- a/package/src/components/Chat/Chat.tsx +++ b/package/src/components/Chat/Chat.tsx @@ -281,9 +281,6 @@ const ChatWithContext = (props: PropsWithChildren) => { version, }; client.deviceIdentifier = { os: `${Platform.OS} ${Platform.Version}` }; - // This is to disable recovery related logic in js client, since we handle it in this SDK - client.recoverStateOnReconnect = false; - client.preventThreadCleanup = true; client.persistUserOnConnectionFailure = enableOfflineSupport; } From 70213f7569512d0aee34a9740a47e0f416f726d9 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Mon, 24 Aug 2026 18:23:32 +0200 Subject: [PATCH 2/9] fix: clear errored out channel screen --- package/src/components/Channel/Channel.tsx | 7 ++++ .../Channel/__tests__/Channel.test.tsx | 40 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/package/src/components/Channel/Channel.tsx b/package/src/components/Channel/Channel.tsx index f78a16a958..c1fe697431 100644 --- a/package/src/components/Channel/Channel.tsx +++ b/package/src/components/Channel/Channel.tsx @@ -803,6 +803,13 @@ const ChannelWithContext = (props: PropsWithChildren) = } const { unsubscribe } = client.on('connection.recovered', () => { + // Drop an error latched while there was no connection — opening a channel offline leaves + // `watch()` throwing, which sets it below. The old `resyncChannel` cleared it at the top of + // every reconnect; nothing else does now that the channel reload lives in the LLC. Safe to + // clear unconditionally: a reload that actually failed reports itself on + // `channel.lastReloadError`, which is OR'd into the context error separately. + setError(false); + // Only when the refreshed window is at the newest. If the user has paginated up into older // history, leave their read state alone. if (!channel.messagePaginator.hasMoreHead) { diff --git a/package/src/components/Channel/__tests__/Channel.test.tsx b/package/src/components/Channel/__tests__/Channel.test.tsx index 9fdda2cea3..3977006a02 100644 --- a/package/src/components/Channel/__tests__/Channel.test.tsx +++ b/package/src/components/Channel/__tests__/Channel.test.tsx @@ -652,6 +652,46 @@ describe('Channel initial load useEffect', () => { await waitFor(() => expect(reload).toHaveBeenCalled()); }); + it('clears an error latched by opening the channel offline once recovery lands', async () => { + // Opening a channel with no connection leaves `watch()` throwing, which latches `error` on the + // component (and `offlineMode` on the channel). The old `resyncChannel` cleared that error at the + // top of every reconnect; with the reload moved into `client.connectionRecovery` there is no + // longer a channel-branch resync to do it, so it has to be cleared on `connection.recovered` or + // the "Error loading messages for this channel..." indicator outlives the recovery it describes. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + + // Not initialized and `watch()` rejecting = the offline-open path. + const watchSpy = jest + .spyOn(testChannel, 'watch') + .mockRejectedValue(new Error('offline: watch failed')); + + let contextError: unknown; + render( + + + { + contextError = (ctx as { error: unknown }).error; + }} + context={ChannelContext as React.Context} + /> + + , + ); + + await waitFor(() => expect(contextError).toBe(true)); + + // Connection is back: the reload the LLC issues now succeeds. + watchSpy.mockRestore(); + + act(() => dispatchConnectionChanged(chatClient, false)); + act(() => dispatchConnectionChanged(chatClient)); + + await waitFor(() => expect(contextError).toBe(false)); + }); + it('surfaces a failed reconnect reload on the channel context', async () => { // The reload is issued by `client.connectionRecovery` inside a `Promise.allSettled`, so a failure // never reaches this component as a throw. It arrives on `channel.state.lastReloadError` instead, From 8a805ee95ce425253a392b8dd9fb9ab0f8e67d48 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Mon, 24 Aug 2026 22:26:59 +0200 Subject: [PATCH 3/9] feat: move threads reconciliation to llc --- package/src/components/Channel/Channel.tsx | 168 ++++------------- .../Channel/__tests__/Channel.test.tsx | 171 +++++++++++++++++- 2 files changed, 203 insertions(+), 136 deletions(-) diff --git a/package/src/components/Channel/Channel.tsx b/package/src/components/Channel/Channel.tsx index c1fe697431..7c26693e4d 100644 --- a/package/src/components/Channel/Channel.tsx +++ b/package/src/components/Channel/Channel.tsx @@ -2,12 +2,10 @@ import React, { PropsWithChildren, useCallback, useEffect, useMemo, useRef, useS import { StyleSheet, Text, View } from 'react-native'; import { - Channel as ChannelType, ChannelConfig, EventHandler, LocalMessage, MessageComposerConfig, - MessageResponse, SendMessageAPIResponse, SendMessageOptions, Event as StreamEvent, @@ -84,12 +82,7 @@ import { primitives } from '../../theme'; import { FileTypes } from '../../types/types'; import { compressedImageURI } from '../../utils/compressImage'; import { patchMessageTextCommand } from '../../utils/patchMessageTextCommand'; -import { - getFileNameFromPath, - isLocalUrl, - MessageStatusTypes, - ReactionData, -} from '../../utils/utils'; +import { getFileNameFromPath, isLocalUrl, ReactionData } from '../../utils/utils'; import { NotificationAnnouncer } from '../Accessibility/NotificationAnnouncer'; import { AttachmentPicker } from '../AttachmentPicker/AttachmentPicker'; import type { KeyboardCompatibleViewProps } from '../KeyboardCompatibleView/KeyboardCompatibleView'; @@ -498,18 +491,18 @@ const ChannelWithContext = (props: PropsWithChildren) = const [messageInputHeightStore] = useState(() => new MessageInputHeightStore()); const { bottomSheetRef, closePicker, openPicker } = useAttachmentPickerBottomSheet(); - const syncingChannelRef = useRef(false); - const { highlightedMessageId } = useStateStore( (threadInstance ?? channel).messagePaginator.messageFocusSignal, messageFocusSignalSelector, ); - // A failed reconnect reload is published on the channel rather than thrown at us, because the reload - // is issued by `client.connectionRecovery` inside a `Promise.allSettled`. Reading it here is what - // keeps the "could not load messages" indicator working now that this component no longer owns the + // A failed reconnect reload is published on the channel/thread rather than thrown at us, because the + // reload is issued by `client.connectionRecovery` inside a `Promise.allSettled`. Reading it here is + // what keeps the "could not load messages" indicator working now that this component owns neither // call. ORed with the local `error` below, which still covers the mount-time `watch()` failure. const { lastReloadError } = useStateStore(channel?.state, reloadErrorSelector) ?? {}; + const { lastReloadError: threadReloadError } = + useStateStore(threadInstance?.state, reloadErrorSelector) ?? {}; /** * This ref keeps track of message IDs which have already been optimistically updated. @@ -694,130 +687,49 @@ const ChannelWithContext = (props: PropsWithChildren) = // instance via useMarkRead(channel). Channel still needs it internally (mark-read-on-mount + resync). const markRead = useMarkRead(channel); - /** - * Reconnect refresh for an open THREAD's replies. - * - * The channel half of this used to live here too; it is now owned by `client.connectionRecovery`, - * which reloads whichever channels are active. Threads still need their own pass: the LLC's - * `ThreadManager` recovery refreshes the thread *list*, and `Thread`'s own stale-state recovery keys - * off `user.watching.stop` rather than a reconnect — so nothing else brings an open thread's replies - * back. Keep this until thread recovery moves down as its own piece of work. - */ - const resyncThread = useStableCallback(async () => { - if (!channel || syncingChannelRef.current || (!channel.initialized && !channel.offlineMode)) { - return; - } - syncingChannelRef.current = true; - setError(false); - - const parseMessage = (message: LocalMessage) => - ({ - ...message, - created_at: message.created_at.toString(), - pinned_at: message.pinned_at?.toString(), - updated_at: message.updated_at?.toString(), - }) as unknown as MessageResponse; - - const getRecoverableFailedMessages = (messages: LocalMessage[] = []) => - messages - .filter( - (message) => - message.status === MessageStatusTypes.FAILED && - !(message.parent_id - ? threadInstance?.messagePaginator.getItem(message.id) - : channel.messagePaginator.getItem(message.id)), - ) - .map(parseMessage); - - try { - if (threadInstance) { - await threadInstance.reload(); - - const currentThreadMessages = - threadInstance.messagePaginator.state.getLatestValue().items ?? []; - const failedThreadMessages = getRecoverableFailedMessages(currentThreadMessages); - if (failedThreadMessages.length) { - failedThreadMessages.forEach((m) => - threadInstance.messagePaginator.ingestItem(channel.state.formatMessage(m)), - ); - } - } - } catch (err) { - if (err instanceof Error) { - setError(err); - } else { - setError(true); - } - } - - syncingChannelRef.current = false; - }); - - // held in a ref so the effect below need not take it as a dependency - const resyncThreadRef = useRef(resyncThread); - resyncThreadRef.current = resyncThread; - - // Trigger for the thread resync above. Deliberately unchanged: with offline support enabled it hangs - // off the offline DB's sync-status change rather than `connection.changed`, because that is published - // only after pending-task replay and `sync()` have run — the reply refresh has to land after them, - // not race them. - useEffect(() => { - if (!thread) { - return; - } - - const connectionChangedHandler = () => { - if (shouldSyncChannel) { - resyncThreadRef.current(); - } - }; - let connectionChangedSubscription: ReturnType; - - if (enableOfflineSupport && client.offlineDb) { - connectionChangedSubscription = client.offlineDb.syncManager.onSyncStatusChange( - (statusChanged) => { - if (statusChanged) { - connectionChangedHandler(); - } - }, - ); - } else { - connectionChangedSubscription = client.on('connection.changed', (event) => { - if (event.online) { - connectionChangedHandler(); - } - }); - } - return () => { - connectionChangedSubscription.unsubscribe(); - }; - }, [enableOfflineSupport, client, shouldSyncChannel, thread]); - // Mark-read after the LLC's reconnect reload. `connection.recovered` is dispatched by // `client.connectionRecovery` once that reload has landed, so `hasMoreHead` read here reflects the // refreshed window — which is why this cannot hang off `connection.changed`. Only the reload moved // into the LLC; whether a caught-up channel is marked read stays a UI decision (see `useMarkRead`). useEffect(() => { - if (thread || !shouldSyncChannel) { + if (!shouldSyncChannel) { return; } - const { unsubscribe } = client.on('connection.recovered', () => { - // Drop an error latched while there was no connection — opening a channel offline leaves - // `watch()` throwing, which sets it below. The old `resyncChannel` cleared it at the top of - // every reconnect; nothing else does now that the channel reload lives in the LLC. Safe to - // clear unconditionally: a reload that actually failed reports itself on - // `channel.lastReloadError`, which is OR'd into the context error separately. - setError(false); - - // Only when the refreshed window is at the newest. If the user has paginated up into older - // history, leave their read state alone. - if (!channel.messagePaginator.hasMoreHead) { - markRead(); + // Drop an error latched while there was no connection — opening a channel offline leaves + // `watch()` throwing, which sets it below. The old `resyncChannel`/`resyncThread` cleared it at + // the top of every reconnect; nothing else does now that both reloads live in the LLC. + // + // Deliberately on `connection.changed`, NOT on `connection.recovered`: `NetworkDownIndicator` + // masks `error` behind `!isOnline`, and `isOnline` flips on exactly this event — so clearing any + // later leaves the banner reading "Error loading messages for this channel..." over perfectly + // good (often offline-cached) content for the whole length of the recovery. Same event as + // `useIsOnline`'s, so the two state updates batch and the banner never flips through the error + // text at all. Clearing early cannot hide a real failure: a reload that fails republishes on + // `channel.lastReloadError` / `thread.state.lastReloadError`, both ORed into the context error. + // + // Not gated on `thread` — a thread screen opened offline latches the same error. + const clearLatchedError = client.on('connection.changed', (event) => { + if (event.online) { + setError(false); } - }); + }).unsubscribe; + + // Mark-read, by contrast, HAS to wait for `connection.recovered`: it is dispatched once the + // reloads have landed, so `hasMoreHead` read here reflects the refreshed window. Channel view + // only, and only when that window is at the newest — if the user has paginated up into older + // history, leave their read state alone. + const markReadOnRecovery = client.on('connection.recovered', () => { + if (thread || channel.messagePaginator.hasMoreHead) { + return; + } + markRead(); + }).unsubscribe; - return unsubscribe; + return () => { + clearLatchedError(); + markReadOnRecovery(); + }; }, [channel, client, markRead, shouldSyncChannel, thread]); /** @@ -1031,7 +943,7 @@ const ChannelWithContext = (props: PropsWithChildren) = disabled: !!channel?.data?.frozen, enableMessageGroupingByUser, enforceUniqueReaction, - error: error || lastReloadError || false, + error: error || lastReloadError || threadReloadError || false, hideDateSeparators, hideStickyDateHeader, highlightedMessageId, diff --git a/package/src/components/Channel/__tests__/Channel.test.tsx b/package/src/components/Channel/__tests__/Channel.test.tsx index 3977006a02..8c11ce9486 100644 --- a/package/src/components/Channel/__tests__/Channel.test.tsx +++ b/package/src/components/Channel/__tests__/Channel.test.tsx @@ -26,6 +26,7 @@ import { generateUser } from '../../../mock-builders/generator/user'; import { getTestClientWithUser } from '../../../mock-builders/mock'; import { Attachment } from '../../Attachment/Attachment'; import { Chat } from '../../Chat/Chat'; +import { Thread as ThreadComponent } from '../../Thread/Thread'; import { Channel } from '../Channel'; import * as MessageListPaginationHooks from '../hooks/useMessageListPagination'; @@ -613,13 +614,13 @@ describe('Channel initial load useEffect', () => { }); }); - // Regression guard for the reconnect refresh of an OPEN THREAD's replies. + // Regression guard for the reconnect refresh of an OPEN THREAD's replies, which now runs entirely in + // `client.connectionRecovery` — this component's only part is marking the thread active. // - // The channel half of the old `resyncChannel` moved into `client.connectionRecovery`; the thread half - // deliberately did NOT, because nothing else brings an open thread's replies back — `ThreadManager` - // recovery refreshes the thread *list*, and `Thread`'s own stale-state recovery keys off - // `user.watching.stop` rather than a reconnect. Deleting this branch while moving the rest is an easy - // mistake to make and produces no other failure, so it is pinned here. + // Asserted end to end on purpose: the LLC can only reach the thread through `client.activeThreads`, + // and a thread resolved as `threadsById[id] ?? new Thread(...)` (the common path — see the + // `threadInstance` memo) is in no other registry. Drop the `threadInstance.activate()` effect and + // recovery silently skips the thread with nothing else failing, so it is pinned here. it('reloads an open thread on reconnect', async () => { const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); @@ -633,25 +634,179 @@ describe('Channel initial load useEffect', () => { parentMessage: testChannel.state.formatMessage(parentMessage), }); const reload = jest.spyOn(threadInstance, 'reload').mockResolvedValue(undefined); + // Recovery finds threads through `client.threads.threadsById`, and only adopts an + // unmanaged instance into the manager once its reply paginator has loaded (Thread.tsx:126, gated + // on `items !== undefined`). Seed loaded-but-empty replies so that adoption actually happens — + // without it this test exercises the documented gap (active but unadopted → skipped) rather than + // the path it means to cover. + act(() => threadInstance.messagePaginator.state.partialNext({ items: [], isLoading: false })); render( the one that owns the thread resync + // `threadList` is what makes this the one that owns the thread view // (`shouldSyncChannel`); without it the channel view would claim it instead. threadList thread={{ thread: testChannel.state.formatMessage(parentMessage), threadInstance }} - /> + > + {/* The real is what calls `threadInstance.activate()`, which is the ONLY thing + that puts the instance in `client.activeThreads` for recovery to find. Rendering it is + the point of the test — a bare would not activate anything. */} + + , ); + // Wait for to activate AND adopt the instance — both are preconditions for recovery to + // see it at all. (With replies seeded above, Thread.tsx's mount metadata-reload is skipped, so + // the spy is clean; cleared anyway so this can only pass on a reconnect-driven call.) + await waitFor(() => { + expect(chatClient.threads.threadsById[threadInstance.id]).toBeDefined(); + expect(threadInstance.state.getLatestValue().active).toBe(true); + }); + reload.mockClear(); + act(() => dispatchConnectionChanged(chatClient, false)); act(() => dispatchConnectionChanged(chatClient)); await waitFor(() => expect(reload).toHaveBeenCalled()); }); + it('clears a latched error on a thread view too, once recovery lands', async () => { + // Sibling of the channel-view case below. `resyncThread` used to clear this at the top of every + // reconnect; when it moved into the LLC the clearing had to move to `connection.recovered` — and + // that handler must NOT be gated on `thread`, or a thread screen opened offline keeps the "Error + // loading messages for this channel..." banner forever after a perfectly good recovery. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + + const parentMessage = generateMessage({ user }); + const watchSpy = jest + .spyOn(testChannel, 'watch') + .mockRejectedValue(new Error('offline: watch failed')); + const threadInstance = new Thread({ + channel: testChannel, + client: chatClient, + parentMessage: testChannel.state.formatMessage(parentMessage), + }); + jest.spyOn(threadInstance, 'reload').mockResolvedValue(undefined); + + let contextError: unknown; + render( + + + {/* activates the instance; without it recovery never sees the thread. */} + + { + contextError = (ctx as { error: unknown }).error; + }} + context={ChannelContext as React.Context} + /> + + , + ); + + await waitFor(() => expect(contextError).toBe(true)); + + watchSpy.mockRestore(); + act(() => dispatchConnectionChanged(chatClient, false)); + act(() => dispatchConnectionChanged(chatClient)); + + await waitFor(() => expect(contextError).toBe(false)); + }); + + it('clears the latched error as soon as the connection is back, not when recovery finishes', async () => { + // Timing, not just outcome. `NetworkDownIndicator` masks `error` behind `!isOnline`, and + // `isOnline` flips on `connection.changed`. If the error were only cleared on + // `connection.recovered` — dispatched after offline replay, sync and every reload — the banner + // would read "Error loading messages for this channel..." over good offline-cached content for + // the whole length of the recovery. So the reload is held open here and the error must ALREADY + // be gone. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + + const watchSpy = jest + .spyOn(testChannel, 'watch') + .mockRejectedValue(new Error('offline: watch failed')); + + let contextError: unknown; + render( + + + { + contextError = (ctx as { error: unknown }).error; + }} + context={ChannelContext as React.Context} + /> + + , + ); + + await waitFor(() => expect(contextError).toBe(true)); + + // Recovery is in flight and never settles, so `connection.recovered` cannot fire. + watchSpy.mockRestore(); + jest.spyOn(testChannel, 'reload').mockReturnValue(new Promise(() => {})); + + act(() => dispatchConnectionChanged(chatClient, false)); + act(() => dispatchConnectionChanged(chatClient)); + + await waitFor(() => expect(contextError).toBe(false)); + }); + + it('surfaces a failed thread reload on the channel context', async () => { + // Recovery runs `thread.reload()` inside a `Promise.allSettled`, so a failure cannot reach this + // component as a throw — it arrives on `thread.state.lastReloadError` and has to be ORed into + // `ChannelContext.error`. That wiring is what this pins; the "recovery actually calls reload" + // half is covered by "reloads an open thread on reconnect" above, so the failure is published + // directly here rather than driven through a reconnect. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + await testChannel.watch(); + + const parentMessage = generateMessage({ user }); + const threadInstance = new Thread({ + channel: testChannel, + client: chatClient, + parentMessage: testChannel.state.formatMessage(parentMessage), + }); + + let contextError: unknown; + render( + + + { + contextError = (ctx as { error: unknown }).error; + }} + context={ChannelContext as React.Context} + /> + + , + ); + + await waitFor(() => expect(contextError).toBe(false)); + + const failure = new Error('thread reload failed'); + act(() => threadInstance.state.partialNext({ lastReloadError: failure })); + + await waitFor(() => expect(contextError).toBe(failure)); + }); + it('clears an error latched by opening the channel offline once recovery lands', async () => { // Opening a channel with no connection leaves `watch()` throwing, which latches `error` on the // component (and `offlineMode` on the channel). The old `resyncChannel` cleared that error at the From 4d0e493fa21047a68022c1bb9508f76e79c5c037 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Tue, 25 Aug 2026 03:14:40 +0200 Subject: [PATCH 4/9] fix: errors on thread open and bump mig guide --- ai-docs/ai-migration-v9-to-v10.md | 231 +++++++++++++++++- .../Channel/__tests__/Channel.test.tsx | 112 +++++++++ package/src/components/Thread/Thread.tsx | 8 +- 3 files changed, 345 insertions(+), 6 deletions(-) diff --git a/ai-docs/ai-migration-v9-to-v10.md b/ai-docs/ai-migration-v9-to-v10.md index e98c71d142..31d30a74f4 100644 --- a/ai-docs/ai-migration-v9-to-v10.md +++ b/ai-docs/ai-migration-v9-to-v10.md @@ -104,6 +104,10 @@ rg '\b(onAddedToChannel|onRemovedFromChannel|onChannelDeleted|onChannelHidden|on rg 'useChatContext\(\)' -A6 src/ | rg '\bchannelManager\b' rg '` / `useChatContext().channelManager` | `client.channelManager` | §18.4 | +| `client.recoverState()` | `client.connectionRecovery.recover()` | §L.6 | +| `` refreshing the channel / open thread on reconnect | `client.connectionRecovery`; `` only marks them active | §L.4 | +| a custom channel/thread view built on the contexts | call `channel.activate()` / `thread.activate()`, balanced with `deactivate()`, or recovery cannot see them (`` and `` do it for you) | §L.4 | +| `` setting `client.recoverStateOnReconnect = false` for you | it no longer does; the option is now the kill switch for `client.connectionRecovery` | §L.1 | +| a `connection.changed` listener re-querying a list / re-watching a channel | delete it — `client.connectionRecovery` owns reconnect | §L | +| `connection.recovered` as "the `_reconnect()` path fired" | now dispatched on **every** reconnect path, after the recovery reload | §L.2 | | `useStateStore(channel.state.readStore / typingStore / membersStore / watcherStore / ownCapabilitiesStore, sel)` | `useStateStore(channel.state, sel)` — drop the `.Store`, keep the selector | §K.1 | | `channel.state.mutedUsersStore` | `client.mutedUsersStore` (via `useMutedUsers()`) | §K.2 | | in-place `channel.data.member_count = n` / `.own_capabilities = […]` | reassign `channel.data = { …channel.data, member_count: n }` | §K.5 | @@ -1105,10 +1115,13 @@ Configure the shared manager through its own API (`client.channelManager.setEven now relocates by its sort key (e.g. `last_message_at`) instead of jumping to the very top. This also means a new message no longer overrides a pinned-first `sort` (pinned channels stay pinned). To keep the old jump-to-top, boost it yourself: `paginator.boost(channel.cid)`. -- **Watch-on-notification narrowed.** On `notification.*` events (e.g. added to a channel) v10 watches - only channels it does not already know; v9 re-watched unconditionally. Deliberate change — the SDK's - own reconnect/query flow re-establishes watches, and blanket auto-watch risks the watch limit. To - watch a specific channel, call `channel.watch()`. +- **Watch-on-notification narrowed — but a watch that was *lost* comes back.** On `notification.*` + events (e.g. added to a channel) v9 re-watched the routed channel unconditionally. v10 does two + narrower things: it watches a channel it does not already know, and it restores a watch this client + previously held and lost to a dropped socket (`watchStatus === 'wasWatching'` — see **§L.3**). A + channel that was never watched, or that you explicitly `stopWatching()`d, stays unwatched, so the + client's watch count can never exceed what it already had, and blanket auto-watch never risks the + watch limit. To watch a specific channel yourself, call `channel.watch()`. --- @@ -1264,6 +1277,204 @@ const { aiState } = useStateStore(channel.state, (s) => ({ aiState: s.aiState }) --- +# Part L — Connection recovery (`client.connectionRecovery`) + +> **Who this affects.** Every app inherits the new behaviour, but almost none needs a code change. +> Reconnect recovery moved out of the UI SDK into `stream-chat`, so `` / `` / +> `` get it for free. You have work to do only if you set `recoverStateOnReconnect: false` +> and hand-rolled recovery, called `client.recoverState()` yourself, or listen for +> `connection.recovered`. + +In v9 the RN SDK reimplemented reconnect recovery in three uncoordinated places, because the +client's own recovery was unusable and `` switched it off (`client.recoverStateOnReconnect = +false`). v10 gives the client a `ConnectionRecoveryManager` — reachable as +`client.connectionRecovery` — that owns the whole flow, and the SDK's own listeners were deleted in +favour of it. + +**What recovery does once the socket is back:** + +1. **Every loaded channel list re-runs its own first-page query** — `client.channelManager.recover()`, + i.e. `paginator.toTail({ keepPreviousItems: true, reset: 'yes' })` for each initialized paginator. + Each list re-asserts its *own* `filters` / `sort` / `pageSize`, and since `queryChannels` watches + by default, that page's watches come back as a side effect. Non-destructive — the list never + blanks, unlike `channelManager.reload()`. +2. **Every `active` channel reloads itself** — `channel.reload()`. A list page carries far fewer + messages per channel than an open channel's loaded window, so the open channel cannot be served by + the list query. `` marks its channel `active` on mount (§K.4). +3. **Every active thread reloads its replies** — `thread.reload()`, for threads in + `client.threads.threadsById` whose `state.active` is set. Nothing else covers them: a channel + reload refreshes the main message list, and `ThreadManager`'s own recovery refreshes the thread + *list*, reusing thread instances without rehydrating them unless something separately marked them + stale — which only `user.watching.stop` does, never a reconnect. Note the current limitation: a + thread opened from a message list only enters `threadsById` once `` adopts it (after its + replies load), so a reconnect before that — or after `ThreadManager.reload()` evicts it — skips it. +4. **Every other previously-watched channel self-heals on demand** — §L.3. + +It is deliberately **not** a sweep over `client.activeChannels`: watches are a bounded server +resource, and after any scrolling that cache holds far more channels than a query page. + +With offline support enabled, active-channel recovery is triggered off the offline DB's sync-status +edge, so the ordering `executePendingTasks()` → `sync()` → reload holds on every reconnect path — +including mobile backgrounding (`closeConnection()` → `openConnection()`), which v9's client-side +recovery never covered at all. + +**No app code is required for any of this.** Do not add a `connection.changed` listener that +re-queries a list or re-watches a channel — that now duplicates the client. + +## L.1 `recoverStateOnReconnect` — same name, new meaning (behavioral) + +The option and its `true` default are unchanged, but what it gates is not. + +| | v9 | v10 | +|---|---|---| +| Gates | one `queryChannels({ cid: { $in: Object.keys(activeChannels) } }, [{ last_message_at: -1 }], { limit: 30 })` | the whole `client.connectionRecovery` flow above | +| Query shape | invented, unrelated to any list's `filters` / `sort` | each list's own first-page query | +| Coverage | the 30 most recently active channels, silently truncated | every loaded list page + every `active` channel | +| Fires on | `StableWSConnection._reconnect()` only | every reconnect path, backgrounding included | + +If you set `recoverStateOnReconnect: false` and recover state yourself, nothing changes — it is still +the kill switch. **The RN SDK no longer sets it to `false`**, so if you were relying on `` +disabling client recovery for you, it no longer does. + +```ts +// still the escape hatch, still opt-out +const client = new StreamChat(apiKey, { recoverStateOnReconnect: false }); +``` + +## L.2 `connection.recovered` now fires on every reconnect path (behavioral) + +Same event, same (empty) payload, still exactly one dispatcher — but the dispatcher moved from +`client.recoverState()` to `ConnectionRecoveryManager`, and it is dispatched *after* the recovery +reload. In v9 `recoverState()` was called only by `StableWSConnection._reconnect()`, so a +`closeConnection()` → `openConnection()` cycle — mobile backgrounding, the dominant path on React +Native — produced no `connection.recovered` at all. + +Consequences: + +- A listener that was effectively dead on backgrounding now runs there. If it is expensive or not + idempotent, check it. +- It is now a valid "recovery finished" hook: the channel reloads have settled by the time it fires. + That is how the SDK sequences its own mark-read-on-catch-up (§L.4). + +## L.3 A lost watch is restored on demand (behavioral — amends §18.5) + +The server keys watches by connection ID, so a dropped socket ends every watch it held. Recovery +re-watches the first page of each list plus the active channel; everything else — pages 2+ of a +scrolled list, channels visited and navigated away from, channels matching no mounted list — is left +marked `ChannelWatchStatus.WasWatching` (§K.3). + +`ChannelManager`'s default event pipeline now re-watches such a channel the moment an event routes it +into a list. Without this, a channel that lost its watch still receives member-level events (e.g. +`notification.message_new`) — enough to relocate its row, but carrying no message body — so its +preview would sit frozen until the channel was opened. + +This is **narrower than v9, not a revert of §18.5's second bullet**: v9 re-watched any routed channel +unconditionally, whereas v10 only restores a watch this client previously *held and lost*. Skipped +for `channel.hidden` events, for a channel pending disposal, and for `NotWatching` — a channel never +watched, or one you explicitly `stopWatching()`d, stays unwatched. The client's watch count can +therefore never exceed what it already had. The re-watch runs after routing and is not awaited, so +the row still relocates immediately off the event; it is idempotent, and concurrent watches for the +same cid are deduped. + +> Applies to `stream-chat-react` as well — this is a default `ChannelManager` handler, not RN code. + +## L.4 `` no longer runs any reconnect resync (behavioral) + +`` used to reload the channel on reconnect, and — when a thread was open — reload that +thread's replies. **Both are gone**; the component's only remaining part is declaring what is on +screen: + +```tsx +// what does now — the rest is client.connectionRecovery's job +useEffect(() => { channel?.activate?.(); return () => channel?.deactivate?.(); }, [channel]); +``` + +The open thread is declared the same way, but by **``**, which already did this before v10 — +`threadInstance.activate()` on mount, `deactivate()` on unmount. `` does not need to (and +does not) activate it as well. + +Neither reconnect handler was ever exported, so there is no symbol to migrate — this is here because +the behaviour relocated, and because the *capability* is unchanged: an integrator who relied on +`` refreshing the channel (or the open thread) on reconnect still gets both, one layer down +and on more reconnect paths than before. + +**If you render `` and `` you get this for free.** If you built your own views on the +contexts, call `channel.activate()` / `thread.activate()` (each balanced with `deactivate()`) or those +surfaces will not be recovered — being active is the one thing recovery cannot infer. + +Two things stay deliberately UI-side, on **different** events: + +- **Mark-read after the reload**, on `connection.recovered` — it has to be post-reload so the + "is the window at the newest?" check reflects the refreshed window. Read policy is a UI decision + (`useMarkRead`, §5); refreshing state is not. +- **Clearing a connection error**, on `connection.changed { online: true }` — deliberately the earlier + event. The built-in `NetworkDownIndicator` masks the channel error behind `!isOnline`, so clearing + any later would leave "Error loading messages for this channel…" showing over good (often + offline-cached) content for the whole length of the recovery. Clearing early cannot hide a real + failure: a reload that fails republishes on `channel.lastReloadError` / + `thread.state.lastReloadError`. If you render your own error UI from `useChannelContext().error`, + the same reasoning applies. Read policy is a UI decision; refreshing state is not. + +``'s own reconnect listener is gone for the same reason — the list re-query is item 1. + +## L.5 Additive surface + +- **`client.connectionRecovery`** — the `ConnectionRecoveryManager` instance. Public method: + `recover()`, which runs a full recovery immediately without waiting for a connection event. +- **`client.channelManager.recover()`** — re-runs every initialized list's first-page query, + non-destructively. Sibling of the destructive `reload()`, which is unchanged. +- **`thread.activate()` / `deactivate()` are now refcounted**, matching `channel.activate()`, so a + thread held by more than one mount stays active until the last holder releases it. `active` is what + recovery filters on, so an unbalanced `deactivate()` now costs a missed reload as well as a missed + auto-read. +- **`thread.state.lastReloadError`** — the thread twin of the channel field below, same contract. +- **`channel.state.lastReloadError`** (+ the `channel.lastReloadError` getter) — mirrors + `BasePaginator.lastQueryError`. Cleared when `reload()` starts, set in its catch, and **rethrown**, + so a failed reconnect refresh stays visible to the UI even though the manager runs the reloads + inside `Promise.allSettled`. The SDK ORs it into `useChannelContext().error`, which is what keeps + the "Error loading messages for this channel…" indicator working. + +```tsx +// surface a failed reconnect refresh in your own UI +const { lastReloadError } = useStateStore(channel.state, (s) => ({ + lastReloadError: s.lastReloadError, +})); +``` + +## L.6 `client.recoverState()` removed (breaking — `stream-chat`) + +Removed outright rather than left as a no-op, so a caller fails loudly instead of silently getting no +recovery. To force a recovery by hand: + +```ts +// Before (v9) +await client.recoverState(); +// After (v10) +await client.connectionRecovery.recover(); +``` + +The two are not equivalent in shape — `recoverState()` ran the 30-channel bulk query described in +§L.1 — but `recover()` is the v10 way to say "bring everything I am reading back in line with the +server now". Most apps never called it: it was invoked automatically on reconnect, and it still is. + +## L.7 Unsent messages now survive a reconnect rebuild (bug fix) + +`channel.reload()` and `thread.reload()` both preserve failed (locally unsent) messages across the +refresh. They already intended to, but the check for "did this one fall out?" read the paginator's +item *index* rather than its visible window — and on a disjoint rebuild (the loaded window shares no +id with the server's newest page, e.g. after scrolling up into old history) a message can sit in the +index while being absent from the rendered list. So the re-ingest was skipped on exactly the path +that needed it, and the user's unsent message disappeared from the list on reconnect. + +No API change and nothing to do — noted because if you worked around it by re-adding failed messages +yourself after a reconnect, that workaround is now redundant. + +For threads there was a second hole: the preservation relied on `Thread`'s `failedRepliesMap`, which +is only written by `upsertReplyLocally` — whose callers are the thread's own subscriptions and the +offline-DB path keyed on `ThreadManager.threadsById`. Neither covers a thread constructed directly and +never registered, which is the common path. `thread.reload()` now reads the failed replies out of the +reply paginator instead, so it holds for managed and unmanaged threads alike. + --- ## 19. Verify @@ -1287,6 +1498,18 @@ const { aiState } = useStateStore(channel.state, (s) => ({ aiState: s.aiState }) pin/archive updates membership-driven UI; and — with `` mounted — focusing a channel with unreads marks it read and reconnect does not blank/re-seed the open message list. +- **Connection recovery** (§L), on a real device, with offline support both on and + off: toggle airplane mode with the list open (it re-queries, never blanks) and + with a channel open (the window comes back byte-identical, scrolled into old + history included); background and foreground the app (same, and this is the path + v9 never recovered); open a channel while offline, then reconnect (its messages + load); send while offline, then reconnect (the queued message goes out); leave a + channel that was open, push it off page 1 of the list, reconnect, then message it + from another user (its preview updates — that is §L.3); open a thread and + reconnect (its replies refresh — §L.4); and cold-boot with a populated offline DB + (exactly one recovery, no double load). Also scroll up into old history, send a + message that fails, then reconnect — the unsent message must still be in the list + (§L.7). --- diff --git a/package/src/components/Channel/__tests__/Channel.test.tsx b/package/src/components/Channel/__tests__/Channel.test.tsx index 8c11ce9486..6caaeed283 100644 --- a/package/src/components/Channel/__tests__/Channel.test.tsx +++ b/package/src/components/Channel/__tests__/Channel.test.tsx @@ -763,6 +763,118 @@ describe('Channel initial load useEffect', () => { await waitFor(() => expect(contextError).toBe(false)); }); + it('does not mark a reply-less thread read on open, but does once it has replies', async () => { + // A parent with no replies has no server-side thread, so the mark-read 404s on every open. There + // is also nothing that could be unread, so the call is skipped rather than made and swallowed. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + await testChannel.watch(); + const markRead = jest + .spyOn(testChannel, 'markRead') + .mockResolvedValue({} as Awaited>); + + const parentMessage = generateMessage({ user }); + const makeThread = (replyCount: number) => { + const instance = new Thread({ + channel: testChannel, + client: chatClient, + parentMessage: testChannel.state.formatMessage({ + ...parentMessage, + reply_count: replyCount, + }), + }); + jest.spyOn(instance, 'reload').mockResolvedValue(undefined); + return instance; + }; + + const empty = makeThread(0); + const { unmount } = render( + + + + + , + ); + await waitFor(() => expect(empty.state.getLatestValue().active).toBe(true)); + expect(markRead).not.toHaveBeenCalled(); + unmount(); + + // Same component, a thread that does have replies: the call is made as before. + const withReplies = makeThread(3); + render( + + + + + , + ); + await waitFor(() => expect(markRead).toHaveBeenCalledWith({ thread_id: withReplies.id })); + }); + + it('does not raise the channel error when a brand-new thread has no server-side thread yet', async () => { + // Opening a parent with no replies makes 's metadata reload answer DoesNotExist (404). + // That is expected, so it must not reach `ChannelContext.error` — otherwise every freshly created + // thread shows the "could not load messages" state. + const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); + useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); + const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); + await testChannel.watch(); + + const parentMessage = generateMessage({ user }); + const threadInstance = new Thread({ + channel: testChannel, + client: chatClient, + parentMessage: testChannel.state.formatMessage(parentMessage), + }); + const notFound = Object.assign(new Error('Request failed with status code 404'), { + code: 16, + StatusCode: 404, + }); + const getThread = jest.spyOn(chatClient, 'getThreadAndHydrate').mockRejectedValue(notFound); + + let contextError: unknown; + render( + + + {/* is what issues the metadata reload this test is about. */} + + { + contextError = (ctx as { error: unknown }).error; + }} + context={ChannelContext as React.Context} + /> + + , + ); + + // Anchor on the rejection having actually happened, then on the reload having settled — asserting + // `contextError` before either would pass without anything running (`isLoading` starts false). + await waitFor(() => expect(getThread).toHaveBeenCalled()); + await waitFor(() => expect(threadInstance.state.getLatestValue().isLoading).toBe(false)); + await waitFor(() => + expect(threadInstance.state.getLatestValue().lastReloadError).toBeUndefined(), + ); + + expect(contextError).toBe(false); + }); + it('surfaces a failed thread reload on the channel context', async () => { // Recovery runs `thread.reload()` inside a `Promise.allSettled`, so a failure cannot reach this // component as a throw — it arrives on `thread.state.lastReloadError` and has to be ORed into diff --git a/package/src/components/Thread/Thread.tsx b/package/src/components/Thread/Thread.tsx index 16266f28e8..fcc06f107a 100644 --- a/package/src/components/Thread/Thread.tsx +++ b/package/src/components/Thread/Thread.tsx @@ -141,13 +141,17 @@ const ThreadWithContext = (props: ThreadPropsWithContext) => { // Mark the thread read on open. Mirrors the pre-refactor openThread behavior: channel.markRead with // a thread_id marks reliably even when the thread instance's own unread count is 0 (so it can't - // rely on the LLC's active-thread auto-read); a reply-less parent has no server-side thread yet and - // rejects with "thread not found", which we swallow. + // rely on the LLC's active-thread auto-read); a reply-less parent has no server-side thread yet, so + // the call is skipped rather than left to 404 on every open. useEffect(() => { const channel = threadInstance?.channel; if (!threadInstance?.id || !channel?.initialized) { return; } + // No replies means nothing that could be unread — true whether or not the thread exists yet. + if (threadInstance.state.getLatestValue().replyCount === 0) { + return; + } channel .markRead({ thread_id: threadInstance.id }) .catch((err) => console.warn('Marking thread as read on open failed with error:', err)); From e46524d0776da2a656c51e0aeb8f230eb7613a3c Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Tue, 25 Aug 2026 03:18:49 +0200 Subject: [PATCH 5/9] chore: bump migration guide --- ai-docs/ai-migration-v9-to-v10.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/ai-docs/ai-migration-v9-to-v10.md b/ai-docs/ai-migration-v9-to-v10.md index 31d30a74f4..ca853c85fc 100644 --- a/ai-docs/ai-migration-v9-to-v10.md +++ b/ai-docs/ai-migration-v9-to-v10.md @@ -1475,6 +1475,37 @@ offline-DB path keyed on `ThreadManager.threadsById`. Neither covers a thread co never registered, which is the common path. `thread.reload()` now reads the failed replies out of the reply paginator instead, so it holds for managed and unmanaged threads alike. +## L.8 The reconnect refresh now finds messages you never had (bug fix) + +Two defects meant a reconnect could refresh a channel or thread and still miss content that arrived +while you were away. Both are fixed; no API change. + +- **The request was sized to what was already loaded.** `channel.reload()` / `thread.reload()` asked + for `loadedCount` items, so a window holding one message asked the server for one message — new + content was undiscoverable, and the single item that came back was disjoint from the loaded window, + so the fold rebuilt and dropped what was there. Now at least a page is requested. +- **A window nothing had ever anchored discarded the page entirely.** When the first query returns + empty and the only content arrives live — a thread or channel created in the current session — the + fetched page was thrown away rather than merged. It is now anchored instead. + +Most visible as: create a thread, send a reply, go offline, someone else replies, come back — the new +replies never appeared, and re-entering the thread did not help. + +## L.9 A brand-new thread no longer reports an error (bug fix) + +A parent with no replies has no server-side thread, so `getThread` answers "not found". That was +being treated as a failed refresh: `thread.state.lastReloadError` was set, which the React Native SDK +ORs into `useChannelContext().error`, so simply opening a new thread raised the channel's error state. +It is now recognised as the expected answer — `reload()` resolves quietly instead of rejecting, and +publishes nothing. + +A genuine failure still publishes as before, including a thread that *did* have replies and comes back +not-found (its parent was deleted, possibly while you were offline). The two are told apart by +`replyCount`, which — unlike `deletedAt` — survives having missed the deletion event. + +Related: the SDK no longer issues a mark-read for a reply-less thread. There is nothing that could be +unread, and the call only ever 404'd. + --- ## 19. Verify From 4bdfc7c690892de52b6941378ac8c16bf3fcf573 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Tue, 25 Aug 2026 11:57:22 +0200 Subject: [PATCH 6/9] fix: remove localized error handling and trust llc --- package/src/components/Channel/Channel.tsx | 74 ++++---------- .../Channel/__tests__/Channel.test.tsx | 98 ++++++++++--------- .../Indicators/LoadingErrorIndicator.tsx | 2 +- .../channelContext/ChannelContext.tsx | 7 +- 4 files changed, 79 insertions(+), 102 deletions(-) diff --git a/package/src/components/Channel/Channel.tsx b/package/src/components/Channel/Channel.tsx index 7c26693e4d..1377a5d6c6 100644 --- a/package/src/components/Channel/Channel.tsx +++ b/package/src/components/Channel/Channel.tsx @@ -343,8 +343,8 @@ const availableCommandsSelector = (state: ChannelConfig) => ({ availableCommands: state.availableCommands, }); -const reloadErrorSelector = (state: { lastReloadError?: Error }) => ({ - lastReloadError: state.lastReloadError, +const loadErrorSelector = (state: { lastLoadError?: Error }) => ({ + lastLoadError: state.lastLoadError, }); const messageFocusSignalSelector = (state: { signal: { messageId?: string } | null }) => ({ @@ -467,7 +467,6 @@ const ChannelWithContext = (props: PropsWithChildren) = const styles = useStyles(); const [deleted, setDeleted] = useState(false); - const [error, setError] = useState(false); const lastReadRef = useRef(undefined); // The active thread is fully prop-driven: derive it synchronously during render so the reply // data is present on the first frame (no setState round-trip / one-frame gap). Opening a thread @@ -496,13 +495,10 @@ const ChannelWithContext = (props: PropsWithChildren) = messageFocusSignalSelector, ); - // A failed reconnect reload is published on the channel/thread rather than thrown at us, because the - // reload is issued by `client.connectionRecovery` inside a `Promise.allSettled`. Reading it here is - // what keeps the "could not load messages" indicator working now that this component owns neither - // call. ORed with the local `error` below, which still covers the mount-time `watch()` failure. - const { lastReloadError } = useStateStore(channel?.state, reloadErrorSelector) ?? {}; - const { lastReloadError: threadReloadError } = - useStateStore(threadInstance?.state, reloadErrorSelector) ?? {}; + const { lastLoadError } = useStateStore(channel?.state, loadErrorSelector) ?? {}; + const { lastLoadError: threadLoadError } = + useStateStore(threadInstance?.state, loadErrorSelector) ?? {}; + const error = lastLoadError ?? threadLoadError; /** * This ref keeps track of message IDs which have already been optimistically updated. @@ -594,7 +590,6 @@ const ChannelWithContext = (props: PropsWithChildren) = await channel?.watch(); } catch (err) { console.warn('Channel watch request failed with error:', err); - setError(true); errored = true; channel.offlineMode = true; } @@ -696,40 +691,18 @@ const ChannelWithContext = (props: PropsWithChildren) = return; } - // Drop an error latched while there was no connection — opening a channel offline leaves - // `watch()` throwing, which sets it below. The old `resyncChannel`/`resyncThread` cleared it at - // the top of every reconnect; nothing else does now that both reloads live in the LLC. - // - // Deliberately on `connection.changed`, NOT on `connection.recovered`: `NetworkDownIndicator` - // masks `error` behind `!isOnline`, and `isOnline` flips on exactly this event — so clearing any - // later leaves the banner reading "Error loading messages for this channel..." over perfectly - // good (often offline-cached) content for the whole length of the recovery. Same event as - // `useIsOnline`'s, so the two state updates batch and the banner never flips through the error - // text at all. Clearing early cannot hide a real failure: a reload that fails republishes on - // `channel.lastReloadError` / `thread.state.lastReloadError`, both ORed into the context error. - // - // Not gated on `thread` — a thread screen opened offline latches the same error. - const clearLatchedError = client.on('connection.changed', (event) => { - if (event.online) { - setError(false); - } - }).unsubscribe; - - // Mark-read, by contrast, HAS to wait for `connection.recovered`: it is dispatched once the - // reloads have landed, so `hasMoreHead` read here reflects the refreshed window. Channel view - // only, and only when that window is at the newest — if the user has paginated up into older - // history, leave their read state alone. - const markReadOnRecovery = client.on('connection.recovered', () => { + // Mark read has to wait for `connection.recovered`, as it is dispatched once the reloads have + // landed, so `hasMoreHead` read here reflects the refreshed window. Channel view only, and only + // when that window is at the newest, only if the user has paginated up into older history so leave + // their read state alone. + const { unsubscribe } = client.on('connection.recovered', () => { if (thread || channel.messagePaginator.hasMoreHead) { return; } markRead(); - }).unsubscribe; + }); - return () => { - clearLatchedError(); - markReadOnRecovery(); - }; + return unsubscribe; }, [channel, client, markRead, shouldSyncChannel, thread]); /** @@ -761,21 +734,10 @@ const ChannelWithContext = (props: PropsWithChildren) = } try { if (thread) { - try { - // jumpToMessage loads the message range into thread.messagePaginator (which backs the - // reply list) and emits the focus signal driving the thread-aware highlight + scroll. - // The reply-list loading spinner is driven off the paginator's own isLoading flag. - await threadInstance?.messagePaginator?.jumpToMessage(messageIdToLoadAround, { - focusReason: 'jump-to-message', - focusSignalTtlMs: DEFAULT_HIGHLIGHT_DURATION, - }); - } catch (err) { - if (err instanceof Error) { - setError(err); - } else { - setError(true); - } - } + await threadInstance?.messagePaginator?.jumpToMessage(messageIdToLoadAround, { + focusReason: 'jump-to-message', + focusSignalTtlMs: DEFAULT_HIGHLIGHT_DURATION, + }); } else { await loadChannelAroundMessageFn({ messageId: messageIdToLoadAround, @@ -943,7 +905,7 @@ const ChannelWithContext = (props: PropsWithChildren) = disabled: !!channel?.data?.frozen, enableMessageGroupingByUser, enforceUniqueReaction, - error: error || lastReloadError || threadReloadError || false, + error, hideDateSeparators, hideStickyDateHeader, highlightedMessageId, diff --git a/package/src/components/Channel/__tests__/Channel.test.tsx b/package/src/components/Channel/__tests__/Channel.test.tsx index 6caaeed283..87d769e4fa 100644 --- a/package/src/components/Channel/__tests__/Channel.test.tsx +++ b/package/src/components/Channel/__tests__/Channel.test.tsx @@ -157,7 +157,9 @@ describe('Channel', () => { it('should set an error if channel watch fails and render a LoadingErrorIndicator', async () => { const watchError = new Error('channel watch fail'); - jest.spyOn(channel, 'watch').mockImplementationOnce(() => Promise.reject(watchError)); + // Rejected at the API seam so the real `watch()` runs: it is what records the failure on + // `channel.state.lastLoadError`, which is the only thing this component reads. + jest.spyOn(channel, 'getOrCreate').mockRejectedValueOnce(watchError); // The LoadingErrorIndicator renders only when watch errors AND the paginator has no messages // to fall back on; seed an empty (rather than the default undefined) item window so the guard // `messages?.length === 0` holds. @@ -673,18 +675,20 @@ describe('Channel initial load useEffect', () => { await waitFor(() => expect(reload).toHaveBeenCalled()); }); - it('clears a latched error on a thread view too, once recovery lands', async () => { - // Sibling of the channel-view case below. `resyncThread` used to clear this at the top of every - // reconnect; when it moved into the LLC the clearing had to move to `connection.recovered` — and - // that handler must NOT be gated on `thread`, or a thread screen opened offline keeps the "Error - // loading messages for this channel..." banner forever after a perfectly good recovery. + it('surfaces and drops a channel load error on a thread view too', async () => { + // Sibling of the channel-view case below, and the reason the error is a straight store read + // rather than anything gated on which view is open: a thread screen opened offline sees the same + // failed `channel.watch()`, and used to keep the "Error loading messages for this channel..." + // banner forever after a perfectly good recovery. const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); const parentMessage = generateMessage({ user }); - const watchSpy = jest - .spyOn(testChannel, 'watch') + // Rejected at the API seam, not by mocking `watch()`: the real `watch()` is what records the + // failure on `channel.state.lastLoadError`, which is the whole surface under test. + const getOrCreateSpy = jest + .spyOn(testChannel, 'getOrCreate') .mockRejectedValue(new Error('offline: watch failed')); const threadInstance = new Thread({ channel: testChannel, @@ -713,28 +717,29 @@ describe('Channel initial load useEffect', () => { , ); - await waitFor(() => expect(contextError).toBe(true)); + await waitFor(() => expect(contextError).toBeInstanceOf(Error)); - watchSpy.mockRestore(); + getOrCreateSpy.mockRestore(); act(() => dispatchConnectionChanged(chatClient, false)); act(() => dispatchConnectionChanged(chatClient)); - await waitFor(() => expect(contextError).toBe(false)); + await waitFor(() => expect(contextError).toBeUndefined()); }); - it('clears the latched error as soon as the connection is back, not when recovery finishes', async () => { + it('drops the load error when the reload starts, not when it finishes', async () => { // Timing, not just outcome. `NetworkDownIndicator` masks `error` behind `!isOnline`, and - // `isOnline` flips on `connection.changed`. If the error were only cleared on - // `connection.recovered` — dispatched after offline replay, sync and every reload — the banner - // would read "Error loading messages for this channel..." over good offline-cached content for - // the whole length of the recovery. So the reload is held open here and the error must ALREADY - // be gone. + // `isOnline` flips on `connection.changed`. The clear has to happen no later than that same + // dispatch, or the banner reads "Error loading messages for this channel..." over good + // (often offline-cached) content for the whole length of the recovery. It does, because + // `Channel.watch()` clears above its first await and the reload reaches it synchronously — the + // same clear-before-attempt v9's `resyncChannel` did on its first line. So the request is held + // open here and the error must ALREADY be gone. const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); - const watchSpy = jest - .spyOn(testChannel, 'watch') + const getOrCreateSpy = jest + .spyOn(testChannel, 'getOrCreate') .mockRejectedValue(new Error('offline: watch failed')); let contextError: unknown; @@ -751,16 +756,17 @@ describe('Channel initial load useEffect', () => { , ); - await waitFor(() => expect(contextError).toBe(true)); + await waitFor(() => expect(contextError).toBeInstanceOf(Error)); - // Recovery is in flight and never settles, so `connection.recovered` cannot fire. - watchSpy.mockRestore(); - jest.spyOn(testChannel, 'reload').mockReturnValue(new Promise(() => {})); + // The re-attempt never comes back, so nothing about its OUTCOME can be what clears the error — + // only the clear that runs on the way in. Held at the API seam rather than by mocking `reload`, + // which would skip the very code under test. + getOrCreateSpy.mockReturnValue(new Promise(() => {})); act(() => dispatchConnectionChanged(chatClient, false)); act(() => dispatchConnectionChanged(chatClient)); - await waitFor(() => expect(contextError).toBe(false)); + await waitFor(() => expect(contextError).toBeUndefined()); }); it('does not mark a reply-less thread read on open, but does once it has replies', async () => { @@ -869,15 +875,15 @@ describe('Channel initial load useEffect', () => { await waitFor(() => expect(getThread).toHaveBeenCalled()); await waitFor(() => expect(threadInstance.state.getLatestValue().isLoading).toBe(false)); await waitFor(() => - expect(threadInstance.state.getLatestValue().lastReloadError).toBeUndefined(), + expect(threadInstance.state.getLatestValue().lastLoadError).toBeUndefined(), ); - expect(contextError).toBe(false); + expect(contextError).toBeUndefined(); }); it('surfaces a failed thread reload on the channel context', async () => { // Recovery runs `thread.reload()` inside a `Promise.allSettled`, so a failure cannot reach this - // component as a throw — it arrives on `thread.state.lastReloadError` and has to be ORed into + // component as a throw — it arrives on `thread.state.lastLoadError` and has to be ORed into // `ChannelContext.error`. That wiring is what this pins; the "recovery actually calls reload" // half is covered by "reloads an open thread on reconnect" above, so the failure is published // directly here rather than driven through a reconnect. @@ -911,27 +917,29 @@ describe('Channel initial load useEffect', () => { , ); - await waitFor(() => expect(contextError).toBe(false)); + await waitFor(() => expect(contextError).toBeUndefined()); const failure = new Error('thread reload failed'); - act(() => threadInstance.state.partialNext({ lastReloadError: failure })); + act(() => threadInstance.state.partialNext({ lastLoadError: failure })); await waitFor(() => expect(contextError).toBe(failure)); }); - it('clears an error latched by opening the channel offline once recovery lands', async () => { - // Opening a channel with no connection leaves `watch()` throwing, which latches `error` on the - // component (and `offlineMode` on the channel). The old `resyncChannel` cleared that error at the - // top of every reconnect; with the reload moved into `client.connectionRecovery` there is no - // longer a channel-branch resync to do it, so it has to be cleared on `connection.recovered` or - // the "Error loading messages for this channel..." indicator outlives the recovery it describes. + it('surfaces the error from opening the channel offline, and drops it once recovery lands', async () => { + // Opening a channel with no connection makes the mount-time `watch()` throw. This component + // catches nothing and holds no error state: `watch()` records the failure on + // `channel.state.lastLoadError`, which is read straight into `ChannelContext.error` — and the LLC + // drops it on the connection edge. Both halves of that wiring are pinned here, because if either + // breaks the "Error loading messages for this channel..." indicator either never appears or + // outlives the recovery it describes. const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); useMockedApis(chatClient, [getOrCreateChannelApi(mockedChannel)]); const testChannel = chatClient.channel('messaging', mockedChannel.channel.id); - // Not initialized and `watch()` rejecting = the offline-open path. - const watchSpy = jest - .spyOn(testChannel, 'watch') + // Not initialized and the API seam rejecting = the offline-open path, with the real `watch()` + // running so it is the LLC that records the failure. + const getOrCreateSpy = jest + .spyOn(testChannel, 'getOrCreate') .mockRejectedValue(new Error('offline: watch failed')); let contextError: unknown; @@ -948,20 +956,20 @@ describe('Channel initial load useEffect', () => { , ); - await waitFor(() => expect(contextError).toBe(true)); + await waitFor(() => expect(contextError).toBeInstanceOf(Error)); // Connection is back: the reload the LLC issues now succeeds. - watchSpy.mockRestore(); + getOrCreateSpy.mockRestore(); act(() => dispatchConnectionChanged(chatClient, false)); act(() => dispatchConnectionChanged(chatClient)); - await waitFor(() => expect(contextError).toBe(false)); + await waitFor(() => expect(contextError).toBeUndefined()); }); it('surfaces a failed reconnect reload on the channel context', async () => { // The reload is issued by `client.connectionRecovery` inside a `Promise.allSettled`, so a failure - // never reaches this component as a throw. It arrives on `channel.state.lastReloadError` instead, + // never reaches this component as a throw. It arrives on `channel.state.lastLoadError` instead, // and has to reach `ChannelContext.error` — that is what drives the "Error loading messages for // this channel…" indicator. const mockedChannel = generateChannelResponse({ messages: [generateMessage({})] }); @@ -986,10 +994,12 @@ describe('Channel initial load useEffect', () => { ); // Let the mount settle with a healthy channel first, so the error asserted below can only have // come from the reconnect reload and not from the mount-time watch. - await waitFor(() => expect(contextError).toBe(false)); + await waitFor(() => expect(contextError).toBeUndefined()); + // Same seam: the reload goes through `watch()`, which records the failure on its way out. Also + // shows the connection-edge clear cannot stomp a fresh failure — it runs first, then this lands. const failure = new Error('reload failed'); - jest.spyOn(testChannel, 'watch').mockRejectedValue(failure); + jest.spyOn(testChannel, 'getOrCreate').mockRejectedValue(failure); act(() => dispatchConnectionChanged(chatClient, false)); act(() => dispatchConnectionChanged(chatClient)); diff --git a/package/src/components/Indicators/LoadingErrorIndicator.tsx b/package/src/components/Indicators/LoadingErrorIndicator.tsx index e24e399906..cd00204c72 100644 --- a/package/src/components/Indicators/LoadingErrorIndicator.tsx +++ b/package/src/components/Indicators/LoadingErrorIndicator.tsx @@ -32,7 +32,7 @@ const LoadingErrorWrapper = (props: React.PropsWithChildren Promise; retry?: () => void; diff --git a/package/src/contexts/channelContext/ChannelContext.tsx b/package/src/contexts/channelContext/ChannelContext.tsx index 80cdc07704..cfdec77fd8 100644 --- a/package/src/contexts/channelContext/ChannelContext.tsx +++ b/package/src/contexts/channelContext/ChannelContext.tsx @@ -37,7 +37,12 @@ export type ChannelContextValue = { * This is similar to reaction UX on [iMessage application](https://en.wikipedia.org/wiki/IMessage). */ enforceUniqueReaction: boolean; - error: boolean | Error; + /** + * The error from the most recent attempt to load this channel's messages, or the open thread's + * replies — read straight off `channel.state.lastLoadError` / `thread.state.lastLoadError`, each + * cleared by the next attempt. `undefined` when the last load succeeded. + */ + error?: Error; /** * Hide inline date separators on channel */ From de0fdb093848ea88edb0422af20bd46bfb683270 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Tue, 25 Aug 2026 11:58:18 +0200 Subject: [PATCH 7/9] chore: bump mig guide --- ai-docs/ai-migration-v9-to-v10.md | 55 ++++++++++++++++++------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/ai-docs/ai-migration-v9-to-v10.md b/ai-docs/ai-migration-v9-to-v10.md index ca853c85fc..dabace2ee8 100644 --- a/ai-docs/ai-migration-v9-to-v10.md +++ b/ai-docs/ai-migration-v9-to-v10.md @@ -1402,18 +1402,25 @@ and on more reconnect paths than before. contexts, call `channel.activate()` / `thread.activate()` (each balanced with `deactivate()`) or those surfaces will not be recovered — being active is the one thing recovery cannot infer. -Two things stay deliberately UI-side, on **different** events: - -- **Mark-read after the reload**, on `connection.recovered` — it has to be post-reload so the - "is the window at the newest?" check reflects the refreshed window. Read policy is a UI decision - (`useMarkRead`, §5); refreshing state is not. -- **Clearing a connection error**, on `connection.changed { online: true }` — deliberately the earlier - event. The built-in `NetworkDownIndicator` masks the channel error behind `!isOnline`, so clearing - any later would leave "Error loading messages for this channel…" showing over good (often - offline-cached) content for the whole length of the recovery. Clearing early cannot hide a real - failure: a reload that fails republishes on `channel.lastReloadError` / - `thread.state.lastReloadError`. If you render your own error UI from `useChannelContext().error`, - the same reasoning applies. Read policy is a UI decision; refreshing state is not. +One thing stays deliberately UI-side: **mark-read after the reload**, on `connection.recovered`. It +has to be post-reload so the "is the window at the newest?" check reflects the refreshed window. Read +policy is a UI decision (`useMarkRead`, §5); refreshing state is not. + +**The error surface is not UI-side either.** `` holds no error state of its own — it reads +`channel.state.lastLoadError` and `thread.state.lastLoadError` straight into `useChannelContext().error` +(§L.5). Both failures a UI cares about are recorded by the client: `channel.watch()` records the +mount-time failure of a channel opened with no connection and — since `reload()` goes through +`watch()` — the reconnect refresh too. + +**Nothing clears these on a connection event.** A load error is invalidated by the next load, not by +coming back online: `watch()` and `Thread.reload()` each clear before they await anything, which is the +same clear-before-attempt v9's `resyncChannel` did on its first line. Because the clear sits above the +first await, a reconnect reload reaches it inside the synchronous `connection.changed` dispatch — the +same dispatch a UI flips its own online flag in, so the two land in one render and an error masked +behind `!isOnline` is never flashed over content that is about to refresh. Clearing on the way in +cannot hide a real failure: the attempt records its own on the way out. If you render your own error UI +from `useChannelContext().error` you get all of this for free; if you latch an error of your own, +delete it. ``'s own reconnect listener is gone for the same reason — the list re-query is item 1. @@ -1427,17 +1434,21 @@ Two things stay deliberately UI-side, on **different** events: thread held by more than one mount stays active until the last holder releases it. `active` is what recovery filters on, so an unbalanced `deactivate()` now costs a missed reload as well as a missed auto-read. -- **`thread.state.lastReloadError`** — the thread twin of the channel field below, same contract. -- **`channel.state.lastReloadError`** (+ the `channel.lastReloadError` getter) — mirrors - `BasePaginator.lastQueryError`. Cleared when `reload()` starts, set in its catch, and **rethrown**, - so a failed reconnect refresh stays visible to the UI even though the manager runs the reloads - inside `Promise.allSettled`. The SDK ORs it into `useChannelContext().error`, which is what keeps - the "Error loading messages for this channel…" indicator working. +- **`thread.state.lastLoadError`** — the thread twin of the channel field below, same contract, + written by `Thread.reload()`. +- **`channel.state.lastLoadError`** (+ the `channel.lastLoadError` getter) — mirrors + `BasePaginator.lastQueryError` one level up. Owned by **`watch()`**: cleared before its first await, + set in its catch, and **rethrown**. Owning it there rather than in `reload()` is what makes it cover + both failures a UI cares about — the reconnect refresh (the manager runs reloads inside + `Promise.allSettled`, which would otherwise swallow them) *and* the mount-time `watch()` of a channel + opened offline, which throws long before anything could later prove it stale. The SDK ORs both fields + into `useChannelContext().error`, which is what keeps the "Error loading messages for this channel…" + indicator working. ```tsx -// surface a failed reconnect refresh in your own UI -const { lastReloadError } = useStateStore(channel.state, (s) => ({ - lastReloadError: s.lastReloadError, +// surface a failed load in your own UI +const { lastLoadError } = useStateStore(channel.state, (s) => ({ + lastLoadError: s.lastLoadError, })); ``` @@ -1494,7 +1505,7 @@ replies never appeared, and re-entering the thread did not help. ## L.9 A brand-new thread no longer reports an error (bug fix) A parent with no replies has no server-side thread, so `getThread` answers "not found". That was -being treated as a failed refresh: `thread.state.lastReloadError` was set, which the React Native SDK +being treated as a failed refresh: `thread.state.lastLoadError` was set, which the React Native SDK ORs into `useChannelContext().error`, so simply opening a new thread raised the channel's error state. It is now recognised as the expected answer — `reload()` resolves quietly instead of rejecting, and publishes nothing. From 5616c00e1267ab2d3c9adda9bcbde823976ffbbf Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Wed, 26 Aug 2026 02:44:34 +0200 Subject: [PATCH 8/9] feat: remaining optimistic updates initial pass --- ai-docs/ai-migration-v9-to-v10.md | 143 +++++++++++ .../offline-support/optimistic-update.tsx | 238 +++++++++++++++++- package/src/components/Channel/Channel.tsx | 18 +- .../Message/hooks/useMessageOperations.ts | 14 +- package/src/mock-builders/DB/mock.ts | 17 +- .../src/store/__tests__/channelExists.test.ts | 44 ++++ package/src/store/apis/channelExists.ts | 16 +- 7 files changed, 471 insertions(+), 19 deletions(-) create mode 100644 package/src/store/__tests__/channelExists.test.ts diff --git a/ai-docs/ai-migration-v9-to-v10.md b/ai-docs/ai-migration-v9-to-v10.md index dabace2ee8..dad4ddc8a4 100644 --- a/ai-docs/ai-migration-v9-to-v10.md +++ b/ai-docs/ai-migration-v9-to-v10.md @@ -1519,6 +1519,149 @@ unread, and the call only ever 404'd. --- +# Part M — Optimistic edit, delete and unsent-message persistence + +Three v9 capabilities did not survive the move of the message lifecycle out of `` and into +`stream-chat`'s `MessageOperations` engine. All are restored, all inside the LLC. No API was removed +and there is nothing to migrate — but if you worked around any of them, the workaround is now +redundant and will double-apply. + +## M.1 Editing a message is optimistic again, and persisted (bug fix) + +An edit shows immediately and is written to the offline DB before the request is made, so it survives +a cold start and the hydration `` performs on mount. v9 did both (`Channel.tsx` wrote the +optimistic copy through `db.updateMessage`); v10 kept neither. + +`message_text_updated_at` is stamped optimistically too, so the "edited" indicator appears at once — +except when the message being edited is itself `failed`, which never had a server-confirmed text +update to advertise. + +**An edit is never rolled back.** Reverting would discard text the user typed. What changes is only +whether the failure is shown on the message: + +- **Offline support enabled and the request was queued for replay** — the message does **not** enter a + failed state. It is pending, not failed, and marking it failed lights up the retry affordance, which + re-*sends* the message rather than re-editing it. The promise still rejects, so any notification you + surface from a rejected `editMessage` is unaffected. +- **No offline DB, or a definitive rejection** (a server error that is not retryable, or a cancelled + request) — + the message keeps the edit and gains `status: 'failed'` plus `error`, as before. + +The predicate is the same one reactions already use: an offline DB is present **and** the error is +`isEphemeral`. + +## M.2 Deleting a message is optimistic, and reverted if it definitively fails (behavioral) + +`MessageOperations.delete` was the only operation that bypassed the optimistic lifecycle: it awaited +the request and then ingested the response. So a delete showed nothing until the server answered. + +Now: + +- A soft delete immediately marks the message `type: 'deleted'` with `deleted_at`, and + `deleted_for_me` for a `delete_for_me` delete. +- A **hard delete removes the message** instead of marking it, matching what the `message.deleted` WS + handler does for `hard_delete`. Previously the response message was ingested unconditionally, which + put a message the server had just destroyed back into the list. +- A delete that fails definitively is **reverted** — unlike an edit there is no user input to lose, and + a "Message deleted" placeholder on a message that still exists server-side only self-corrects on the + next query. A queued (offline) delete keeps the optimistic state. + + In practice the trigger is **offline support disabled plus no connectivity**, not a permission + rejection: the delete action is capability-gated in the UI (`deleteOwnMessage` / `deleteAnyMessage`), + so a user without permission is never offered it. If the revert is ever wrong — the server did delete + the message but the response was lost — the `message.deleted` event removes it again. +- Deleting a message nothing was displaying no longer inserts a phantom deleted row. + +The revert is guarded on object identity against the copy the optimistic step wrote, so a WS update +landing mid-request is never clobbered by the rollback. + +## M.3 Unsent messages survive closing the app (bug fix — data loss) + +Every send now writes the message to the offline DB **before** the request, pessimistically marked +`failed`, and overwrites it with `received` on success. A process death anywhere between composing and +the server's ack therefore leaves a message that hydrates as failed and retryable, instead of one that +silently disappears. This is v9's write-ahead (`Channel.tsx`), which v10 dropped when `sendMessage` +collapsed into `sendMessageWithLocalUpdate`. + +No schema change and no `dbVersion` bump: `status` already round-trips through the storable's +`extraData` blob. + +The retry payload does not need persisting alongside it. `MessageOperations.retry` reconstructs the +request from the message when its in-memory `failedSendCache` is cold, so a persisted failed message +stays retryable past that cache's 5-minute TTL and across restarts. + +`channel.reload()` also consults the DB, not just the in-memory window, when deciding which failed +messages a reconnect has to put back — a message evicted from the paginator, or one on a cold boot that +the paginator never held, was previously unrecoverable. v9 got this from a second, lagging copy of the +message list (the SDK's own React state); with a single reactive source of truth the persisted row is +that buffer. + +## M.4 An empty request response no longer overwrites local state (bug fix) + +`formatMessage(undefined)` does not throw — it returns `{ status: 'received', created_at: , +updated_at: }`. Because that `updated_at` is *now*, it beat the "is the server copy newer?" check +and was ingested as an id-less message. v9 guarded every apply with `if (response?.message)`; the guard +is back. Reachable from a custom `sendMessageRequest` / `updateMessageRequest` / `deleteMessageRequest` +that resolves without a `message`. + +## M.5 Editing or deleting a thread parent from inside a thread works (bug fix) + +The reply paginator's local filter is `{ cid, parent_id }`, and a parent message has no `parent_id` — +so routing a parent edit or delete through the open thread's instance (which is what the SDKs do while +a thread is on screen) handed it to a collection that could not hold it, and the operation was silently +dropped. Optimistic writes now fall back to the client-global message store when the paginator does not +accept the message, reaching it wherever it is held and fanning out to every collection that holds it — +the same addressing `applyReactionLocally` uses. The SDK additionally routes by membership rather than +by "is a thread open", mirroring `sendReaction`. + +## M.6 Additive on `AbstractOfflineDB` + +Both are **concrete** helpers composed from existing primitives, so a custom offline DB implementation +inherits them and has nothing new to implement: + +- `getFailedMessages({ cid })` — the channel's locally failed (unsent) messages, read back through + `getChannels`. +- `upsertMessageWithChannelGuard({ message })` — upserts one message, creating its channel row first + when the DB has never seen that channel, so the optimistic write cannot fail on the foreign key. + `updateMessage` cannot serve this: it is an UPDATE and no-ops when the row does not exist, which is + exactly the write-ahead case. + +## M.7 The offline channel guard actually guards now (bug fix) + +`channelExists` ran `SELECT EXISTS(SELECT 1 FROM channels WHERE cid = ?)` and returned +`rows.length > 0`. `SELECT EXISTS` always returns exactly one row — holding `0` or `1` — so the row +count carried no information and the helper reported `true` for every cid, present or not. It has done +so since offline support v2 shipped; the LLC's tests mock `channelExists`, so the logic was covered and +the SQL never was. + +Its only consumer is `AbstractOfflineDB.queriesWithChannelGuard`, used by ten WS-event handlers +(`message.new`, `message.deleted`, `message.updated`, `message.read`, `member.*`, `reaction.*`, …). Its +gate is `forceUpdate || !(await channelExists({ cid }))`, which collapsed to `forceUpdate` — so the +branch that recreates a missing channel row from the event never ran, and those writes died on the +`messages.cid → channels.cid` foreign key and were swallowed by the detached query runner. + +Reachable whenever an event arrives for a channel the DB has no row for: after `resetDB()` (which the +sync-failure path and a >30-day-stale sync both trigger) until the next channel-list query; in the +window between `queryChannels` resolving and its persistence completing (and `channel.query`'s +persistence is detached, widening it); and after being added to a channel mid-session. The message list +self-heals on the next query — reactions, read state, member changes and older messages in those +windows did not. + +**Both channel guards are also lazy now.** `queriesWithChannelGuard` and +`upsertMessageWithChannelGuard` attempt the write first and only probe for the channel row when it +fails, repairing and retrying once; a failure with the channel row present is rethrown without a retry. +Every statement involved is an upsert, so the retry is idempotent. The eager probe is kept for +`execute: false` callers (collecting queries for someone else's batch, so there is no failure to catch) +and for `forceUpdate`. + +That inversion matters because the probe is a native round-trip, not a cheap read. Measured through +op-sqlite on device: the probe costs ~0.5ms against ~8-12ms for the message upsert it protects, and +roughly two thirds of that is the JS↔native crossing rather than the query. Removing it from the happy +path takes it off every message received as well as every message written — so message writes are +cheaper than they were before this release, not more expensive. + +--- + ## 19. Verify - Typecheck the customer app; removed symbols surface as "Property does not diff --git a/package/src/__tests__/offline-support/optimistic-update.tsx b/package/src/__tests__/offline-support/optimistic-update.tsx index ecb54974fa..5fe0492110 100644 --- a/package/src/__tests__/offline-support/optimistic-update.tsx +++ b/package/src/__tests__/offline-support/optimistic-update.tsx @@ -86,6 +86,11 @@ const markConnectionUnhealthy = (client: StreamChat) => { (client.wsConnection as unknown as { isHealthy: boolean }).isHealthy = false; }; +/** The counterpart of {@link markConnectionUnhealthy}, for tests that go offline and then reconnect. */ +const markConnectionHealthy = (client: StreamChat) => { + (client.wsConnection as unknown as { isHealthy: boolean }).isHealthy = true; +}; + // React flushes passive effects child-first, so the test-callback effect below runs BEFORE `Channel`'s // own mount effects — verifiably: without this wait, `channel.configState.requestHandlers` at edit time // holds only the declaratively-registered `updateMessageRequest`, with no `sendMessageRequest`, because @@ -885,6 +890,13 @@ export const OptimisticUpdates = () => { { + // Same barrier every other "edit message" test uses. Without it the edit fires from + // this child mount effect BEFORE `Channel`'s own effect runs `channel.watch()`, whose + // seed then re-ingests the pre-edit copy from the mocked query response and overwrites + // the optimistic edit. Measured: the optimistic copy is correct at `editMessage` + // resolution and still correct a macrotask later, then the in-flight watch lands on + // top of it. That window is unreachable in production (see flushMountEffects). + await flushMountEffects(); // Go offline BEFORE editing so the default (no-handler) offline path runs. markConnectionUnhealthy(chatClient); try { @@ -914,6 +926,9 @@ export const OptimisticUpdates = () => { const dbMessage = dbMessages.find((row) => row.id === message.id); expect(updatedMessage?.text).toBe(editedText); + // Offline support is enabled and the edit was queued for replay, so this is "pending", + // not "failed" — the message must never enter a failed state on this path. + expect(updatedMessage?.status).not.toBe(MessageStatusTypes.FAILED); expect(dbMessage?.text).toBe(editedText); }, { timeout: 2500 }, @@ -928,6 +943,7 @@ export const OptimisticUpdates = () => { { + await flushMountEffects(); markConnectionUnhealthy(chatClient); try { await deleteMessage(message); @@ -959,6 +975,184 @@ export const OptimisticUpdates = () => { }); }); + describe('failed message persistence', () => { + it('persists a failed send so it survives a restart and reads back as retryable', async () => { + const localMessage = generateMessage({ + cid: channel.cid, + status: MessageStatusTypes.SENDING, + text: 'unsent across a restart', + user: chatClient.user as UserResponse, + user_id: chatClient.userID, + }); + + jest + .spyOn(channel.messageComposer, 'compose') + .mockResolvedValue({ localMessage, message: localMessage } as unknown as Awaited< + ReturnType + >); + + render( + + + { + await flushMountEffects(); + markConnectionUnhealthy(chatClient); + try { + await sendMessage(); + } catch (e) { + // do nothing + } + }} + context={MessageInputContext} + > + + + + , + ); + await waitFor(() => expect(screen.getByTestId('children')).toBeTruthy()); + + // The row itself is what makes a failed message survive a process death: v9 wrote it ahead of + // the request and v10 dropped that write, which is why closing the app lost unsent messages. + await waitFor(async () => { + const dbMessages = await BetterSqlite.selectFromTable<{ + extraData: string; + id: string; + text: string; + }>('messages'); + const dbMessage = dbMessages.find((row) => row.id === localMessage.id); + + expect(dbMessage).toBeTruthy(); + expect(dbMessage!.text).toBe(localMessage.text); + // `status` has no column of its own — it round-trips through the extraData blob. + expect(JSON.parse(dbMessage!.extraData).status).toBe(MessageStatusTypes.FAILED); + }); + + // And it has to come back through the DB's own read path, which is what a cold start hydrates + // from and what `Channel.reload` consults on reconnect. + const restored = await ( + chatClient.offlineDb as unknown as { + getFailedMessages: (o: { cid: string }) => Promise; + } + ).getFailedMessages({ cid: channel.cid }); + + expect(restored.map((message) => message.id)).toContain(localMessage.id); + expect(restored.find((message) => message.id === localMessage.id)?.text).toBe( + localMessage.text, + ); + }); + }); + + describe('channel guard cost', () => { + it('writes an optimistic message without probing for the channel row', async () => { + const localMessage = generateMessage({ + cid: channel.cid, + status: MessageStatusTypes.SENDING, + text: 'no guard probe please', + user: chatClient.user as UserResponse, + user_id: chatClient.userID, + }); + + jest + .spyOn(channel.messageComposer, 'compose') + .mockResolvedValue({ localMessage, message: localMessage } as unknown as Awaited< + ReturnType + >); + + let guardSpy: jest.SpyInstance | undefined; + + render( + + + { + await flushMountEffects(); + // Spied after mount, so the count covers only the send below — not the channel + // query that `Channel` performs while starting up. + guardSpy = jest.spyOn( + chatClient.offlineDb as unknown as { channelExists: () => Promise }, + 'channelExists', + ); + try { + await sendMessage(); + } catch (e) { + // do nothing + } + }} + context={MessageInputContext} + > + + + + , + ); + await waitFor(() => expect(screen.getByTestId('children')).toBeTruthy()); + + // The write has to actually have happened, or "no probe" would be trivially true. + await waitFor(async () => { + const dbMessages = await BetterSqlite.selectFromTable<{ id: string }>('messages'); + expect(dbMessages.some((row) => row.id === localMessage.id)).toBe(true); + }); + + // The guard is lazy: it attempts the write and only probes if that fails on the foreign key. + // A probe here means the eager version is back — a native round-trip per message write, for + // every message written AND every message received. + expect(guardSpy).not.toHaveBeenCalled(); + }); + }); + + describe('optimistic edit without offline support', () => { + it('keeps the optimistic edit AND surfaces the failure when there is no offline DB', async () => { + const message = channel.messagePaginator.headItems[0]; + const editedText = 'edited with no offline support'; + + chatClient.config.set({ + channel: { + requestHandlers: { + updateMessageRequest: (() => Promise.reject(new Error('validation'))) as never, + }, + }, + }); + + // No `enableOfflineSupport`, so `client.offlineDb` is never attached and there is no queue for + // the edit to fall back on. The failure is therefore definitive and must be shown on the + // message — the opposite of the offline-enabled case above, where it must NOT be. + render( + + + { + await flushMountEffects(); + try { + await editMessage({ + localMessage: { ...message, cid: channel.cid, text: editedText }, + options: {}, + }); + } catch (e) { + // do nothing + } + }} + context={MessageInputContext} + > + + + + , + ); + await waitFor(() => expect(screen.getByTestId('children')).toBeTruthy()); + + await waitFor(() => { + const updatedMessage = channel.messagePaginator.getItem(message.id); + + expect(chatClient.offlineDb).toBeUndefined(); + // The edit is never rolled back — reverting would throw away what the user typed. + expect(updatedMessage?.text).toBe(editedText); + expect(updatedMessage?.status).toBe(MessageStatusTypes.FAILED); + }); + }); + }); + describe('pending task execution', () => { it('pending task should be executed after connection is recovered', async () => { const message = channel.messagePaginator.headItems[0]; @@ -1123,13 +1317,33 @@ export const OptimisticUpdates = () => { jest .spyOn(channel, 'watch') .mockResolvedValue({} as Awaited>); + // Without this the reconnect below nukes the offline DB. `client.sync` is a POST, so it + // resolves with the `getOrCreateChannelApi` payload mocked in `beforeEach`, whose `events` is + // undefined; `OfflineDBSyncManager.sync` then throws reading `result.events.length` and its + // catch block calls `resetDB()` — taking the persisted failed message with it. Nothing to do + // with what these tests assert, so give sync an empty, well-formed reply. + jest + .spyOn(chatClient, 'sync') + .mockResolvedValue({ events: [] } as unknown as Awaited< + ReturnType + >); channel.messagePaginator.removeItem({ id: localMessage.id }); channel.messagePaginator.ingestItem(channel.state.formatMessage(serverMessage)); await getOfflineDb(chatClient).deletePendingTask({ id: pendingTask!.id }); await act(async () => { - await getOfflineDb(chatClient).syncManager.invokeSyncStatusListeners(true); + // The real reconnect signal. `invokeSyncStatusListeners(true)` on its own used to be enough + // because `Channel` subscribed to the offline DB's sync-status edge itself; on v10 that moved + // into the LLC's `ConnectionRecoveryManager`, which binds that subscription lazily from its + // `connection.changed` handler and only then reloads the active channels. Driving the edge + // directly therefore reached no subscriber at all — the assertions below never ran against a + // reload. `OfflineDBSyncManager` publishes the edge itself once it has replayed and synced. + markConnectionHealthy(chatClient); + dispatchConnectionChangedEvent(chatClient, true); + // Recovery is detached (`runDetached`), so yield once to let it start before the assertions + // below begin polling. + await flushMountEffects(); }); await waitFor(() => { @@ -1200,12 +1414,32 @@ export const OptimisticUpdates = () => { jest .spyOn(channel, 'watch') .mockResolvedValue({} as Awaited>); + // Without this the reconnect below nukes the offline DB. `client.sync` is a POST, so it + // resolves with the `getOrCreateChannelApi` payload mocked in `beforeEach`, whose `events` is + // undefined; `OfflineDBSyncManager.sync` then throws reading `result.events.length` and its + // catch block calls `resetDB()` — taking the persisted failed message with it. Nothing to do + // with what these tests assert, so give sync an empty, well-formed reply. + jest + .spyOn(chatClient, 'sync') + .mockResolvedValue({ events: [] } as unknown as Awaited< + ReturnType + >); channel.messagePaginator.removeItem({ id: localMessage.id }); await getOfflineDb(chatClient).deletePendingTask({ id: pendingTask!.id }); await act(async () => { - await getOfflineDb(chatClient).syncManager.invokeSyncStatusListeners(true); + // The real reconnect signal. `invokeSyncStatusListeners(true)` on its own used to be enough + // because `Channel` subscribed to the offline DB's sync-status edge itself; on v10 that moved + // into the LLC's `ConnectionRecoveryManager`, which binds that subscription lazily from its + // `connection.changed` handler and only then reloads the active channels. Driving the edge + // directly therefore reached no subscriber at all — the assertions below never ran against a + // reload. `OfflineDBSyncManager` publishes the edge itself once it has replayed and synced. + markConnectionHealthy(chatClient); + dispatchConnectionChangedEvent(chatClient, true); + // Recovery is detached (`runDetached`), so yield once to let it start before the assertions + // below begin polling. + await flushMountEffects(); }); await waitFor(() => { diff --git a/package/src/components/Channel/Channel.tsx b/package/src/components/Channel/Channel.tsx index 1377a5d6c6..b79c5ec1df 100644 --- a/package/src/components/Channel/Channel.tsx +++ b/package/src/components/Channel/Channel.tsx @@ -856,12 +856,18 @@ const ChannelWithContext = (props: PropsWithChildren) = if (!channel) { throw new Error('Channel has not been initialized'); } - // The LLC handles the optimistic local update (ingest into the paginator), the network - // request (honoring any `updateMessageRequest` registered through - // `client.config.set({ channel: { requestHandlers } })`), the received/failed state transitions, - // and offline queueing. - // Thread edits route through the thread instance's own message operations. - await (threadInstance ?? channel).updateMessageWithLocalUpdate({ localMessage, options }); + // The LLC handles the optimistic local update, the network request (honoring any + // `updateMessageRequest` registered through `client.config.set({ channel: { requestHandlers } })`), + // the received/failed state transitions, offline queueing and the offline-DB write. + // + // Routed by MEMBERSHIP rather than "a thread is open", mirroring `useMessageOperations`' + // `sendReaction`: a reply loaded in the open thread is edited through the thread instance, and + // anything else — including the thread's own PARENT message, which the reply paginator cannot + // hold — through the channel. + const target = threadInstance?.messagePaginator.getItem(localMessage.id) + ? threadInstance + : channel; + await target.updateMessageWithLocalUpdate({ localMessage, options }); }, ); diff --git a/package/src/components/Message/hooks/useMessageOperations.ts b/package/src/components/Message/hooks/useMessageOperations.ts index f5fbe3027a..fe930e5371 100644 --- a/package/src/components/Message/hooks/useMessageOperations.ts +++ b/package/src/components/Message/hooks/useMessageOperations.ts @@ -126,9 +126,17 @@ export const useMessageOperations = (): MessageOperations => { options = { hard: true }; } - // The LLC performs the delete request (honoring any configState delete handler) and ingests - // the deleted message into the paginator. Thread deletes route through the thread instance. - await (threadInstance ?? channel).deleteMessageWithLocalUpdate({ + // The LLC owns the whole delete lifecycle: the optimistic `deleted` marking (or removal, for a + // hard delete), the request (honoring any configState delete handler), the offline-DB write, and + // the revert if the delete is definitively rejected. + // + // Routed by MEMBERSHIP, like `sendReaction` above: a reply loaded in the open thread goes through + // the thread instance, everything else — including the thread's own parent message, which the + // reply paginator cannot hold — through the channel. + const target = threadInstance?.messagePaginator.getItem(message.id) + ? threadInstance + : channel; + await target.deleteMessageWithLocalUpdate({ localMessage: message, options, }); diff --git a/package/src/mock-builders/DB/mock.ts b/package/src/mock-builders/DB/mock.ts index 8d03370c98..2d03d32331 100644 --- a/package/src/mock-builders/DB/mock.ts +++ b/package/src/mock-builders/DB/mock.ts @@ -32,11 +32,20 @@ export const sqliteMock = { rmSync(testDbName, { force: true }); }, execute: async (queryInput: string, params: unknown[]) => { - const query = queryInput.trim().toLowerCase(); + const query = queryInput.trim(); + // Lower-cased COPY, used only to classify the statement and to parse PRAGMA tokens. The query + // itself must be executed with its original casing: SQL keywords are case-insensitive, but + // string literals are not — and `selectMessagesForChannels` builds its result rows with + // `json_object('extraData', a.extraData, ...)`, whose keys are literals. Lower-casing the whole + // statement renamed every one of those keys (`extradata`, `createdAt` -> `createdat`, ...), so + // `mapStorableToMessage`'s destructuring silently produced `undefined` for every camelCase + // field — including the `extraData` blob that carries `status`. op-sqlite runs the SQL as + // written, so this only ever misled tests. + const classifier = query.toLowerCase(); const stmt = db.prepare(query); let result: unknown[] = []; - if (query.indexOf('select') === 0) { + if (classifier.indexOf('select') === 0) { const modifiedParams = params?.map((p) => (typeof p === 'boolean' ? Number(p) : p)) || []; result = await new Promise((resolve) => resolve(stmt.all(modifiedParams))); @@ -47,8 +56,8 @@ export const sqliteMock = { }; } - if (query.indexOf('pragma') === 0) { - const pragmaQueryTokens = query.split(' '); + if (classifier.indexOf('pragma') === 0) { + const pragmaQueryTokens = classifier.split(' '); if (pragmaQueryTokens[2] === '=') { db.pragma(`${pragmaQueryTokens[1]} = ${pragmaQueryTokens[3]}`); } else { diff --git a/package/src/store/__tests__/channelExists.test.ts b/package/src/store/__tests__/channelExists.test.ts new file mode 100644 index 0000000000..b9a85a8070 --- /dev/null +++ b/package/src/store/__tests__/channelExists.test.ts @@ -0,0 +1,44 @@ +import { generateChannelResponse } from '../../mock-builders/generator/channel'; +import { BetterSqlite } from '../../test-utils/BetterSqlite'; +import { channelExists } from '../apis/channelExists'; +import { upsertChannels } from '../apis/upsertChannels'; +import { SqliteClient } from '../SqliteClient'; + +/** + * Runs against real SQLite rather than a mocked DB on purpose: the bug this guards was in the SQL + * itself, so a mock that answers `true`/`false` on command would have kept passing forever. + */ +describe('channelExists', () => { + beforeEach(async () => { + await SqliteClient.initializeDatabase(); + await BetterSqlite.openDB(); + }); + + afterEach(() => { + BetterSqlite.dropAllTables(); + BetterSqlite.closeDB(); + }); + + it('reports false for a channel the database does not have', async () => { + expect(await channelExists({ cid: 'messaging:never-persisted' })).toBe(false); + }); + + it('reports true for a channel the database does have', async () => { + const channelResponse = generateChannelResponse({ members: [], messages: [] }); + await upsertChannels({ + channels: [channelResponse] as unknown as Parameters[0]['channels'], + }); + + expect(await channelExists({ cid: channelResponse.channel.cid as string })).toBe(true); + }); + + it('distinguishes one channel from another', async () => { + const persisted = generateChannelResponse({ members: [], messages: [] }); + await upsertChannels({ + channels: [persisted] as unknown as Parameters[0]['channels'], + }); + + expect(await channelExists({ cid: persisted.channel.cid as string })).toBe(true); + expect(await channelExists({ cid: 'messaging:some-other-channel' })).toBe(false); + }); +}); diff --git a/package/src/store/apis/channelExists.ts b/package/src/store/apis/channelExists.ts index 6c4a264b63..5b57015719 100644 --- a/package/src/store/apis/channelExists.ts +++ b/package/src/store/apis/channelExists.ts @@ -1,10 +1,18 @@ import { SqliteClient } from '../SqliteClient'; +/** + * Whether a channel row exists, which callers use to avoid writing a row whose `cid` foreign key + * would not resolve. + * + * Deliberately `SELECT 1 ... LIMIT 1` rather than `SELECT EXISTS(...)`: `EXISTS` always returns + * exactly one row (holding `0` or `1`), so the row COUNT carries no information and the previous + * implementation reported `true` for every cid, existing or not. Returning zero rows for a miss is + * what makes the answer readable without depending on the result column's name. + */ export const channelExists = async ({ cid }: { cid: string }) => { - const channels = await SqliteClient.executeSql( - 'SELECT EXISTS(SELECT 1 FROM channels WHERE cid = ?)', - [cid], - ); + const channels = await SqliteClient.executeSql('SELECT 1 FROM channels WHERE cid = ? LIMIT 1', [ + cid, + ]); SqliteClient.logger?.('info', 'channelExists', { cid, From e96aee27b16e94f57b2a80f1918de2a78bcb1e3b Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 27 Aug 2026 12:41:09 +0200 Subject: [PATCH 9/9] fix: db crash on wrongful reaction with no parent upsert --- .../store/__tests__/insertReaction.test.ts | 107 ++++++++++++++++++ package/src/store/apis/insertReaction.ts | 14 ++- package/src/store/apis/updateReaction.ts | 9 +- .../store/sqlite-utils/createUpsertQuery.ts | 44 +++++-- .../createUpsertQueryIfParentExists.ts | 58 ++++++++++ 5 files changed, 220 insertions(+), 12 deletions(-) create mode 100644 package/src/store/__tests__/insertReaction.test.ts create mode 100644 package/src/store/sqlite-utils/createUpsertQueryIfParentExists.ts diff --git a/package/src/store/__tests__/insertReaction.test.ts b/package/src/store/__tests__/insertReaction.test.ts new file mode 100644 index 0000000000..1dd4640f62 --- /dev/null +++ b/package/src/store/__tests__/insertReaction.test.ts @@ -0,0 +1,107 @@ +import { generateChannelResponse } from '../../mock-builders/generator/channel'; +import { generateMessage } from '../../mock-builders/generator/message'; +import { generateReaction } from '../../mock-builders/generator/reaction'; +import { BetterSqlite } from '../../test-utils/BetterSqlite'; +import { insertReaction } from '../apis/insertReaction'; +import { updateReaction } from '../apis/updateReaction'; +import { upsertChannels } from '../apis/upsertChannels'; +import { upsertMessages } from '../apis/upsertMessages'; +import { SqliteClient } from '../SqliteClient'; + +/** + * Runs against real SQLite rather than a mocked DB on purpose: what this guards is a foreign key + * declared in the schema, and a mocked `executeSqlBatch` accepts any statement you hand it. + */ +describe('reaction writes when the message is not cached', () => { + const cid = 'messaging:reaction-guard'; + + const cacheAMessage = async (id: string) => { + const channelResponse = generateChannelResponse({ members: [], messages: [] }); + channelResponse.channel.cid = cid; + await upsertChannels({ + channels: [channelResponse] as unknown as Parameters[0]['channels'], + }); + const message = generateMessage({ cid, id }); + await upsertMessages({ + messages: [message] as unknown as Parameters[0]['messages'], + }); + return message; + }; + + const storedReactions = () => BetterSqlite.selectFromTable('reactions'); + + beforeEach(async () => { + await SqliteClient.initializeDatabase(); + await BetterSqlite.openDB(); + }); + + afterEach(() => { + BetterSqlite.dropAllTables(); + BetterSqlite.closeDB(); + }); + + it('inserts the reaction when the message is cached', async () => { + const message = await cacheAMessage('cached-message'); + const reaction = generateReaction({ message_id: message.id, type: 'love' }); + + await insertReaction({ message, reaction }); + + expect(await storedReactions()).toHaveLength(1); + }); + + // The failure this closes: a `/sync` replay carrying a reaction on a message outside the cached + // window aborted the whole batch with `FOREIGN KEY constraint failed`, so all 45 unrelated events + // in it were lost too. + it('skips the reaction, without throwing, when the message was never cached', async () => { + await cacheAMessage('some-other-message'); + const reaction = generateReaction({ message_id: 'never-persisted', type: 'love' }); + + await expect( + insertReaction({ + message: { id: 'never-persisted', reaction_groups: {} } as Parameters< + typeof insertReaction + >[0]['message'], + reaction, + }), + ).resolves.not.toThrow(); + + expect(await storedReactions()).toHaveLength(0); + }); + + it('does not abort the rest of the batch it shares', async () => { + const message = await cacheAMessage('cached-message'); + const orphanQueries = await insertReaction({ + execute: false, + message: { id: 'never-persisted', reaction_groups: {} } as Parameters< + typeof insertReaction + >[0]['message'], + reaction: generateReaction({ message_id: 'never-persisted', type: 'like' }), + }); + const validQueries = await insertReaction({ + execute: false, + message, + reaction: generateReaction({ message_id: message.id, type: 'love' }), + }); + + await SqliteClient.executeSqlBatch([...orphanQueries, ...validQueries]); + + // The orphan is dropped and the reaction that had a parent still lands. + expect(await storedReactions()).toHaveLength(1); + }); + + it('applies the same guard to updateReaction', async () => { + await cacheAMessage('some-other-message'); + const reaction = generateReaction({ message_id: 'never-persisted', type: 'love' }); + + await expect( + updateReaction({ + message: { id: 'never-persisted', reaction_groups: {} } as Parameters< + typeof updateReaction + >[0]['message'], + reaction, + }), + ).resolves.not.toThrow(); + + expect(await storedReactions()).toHaveLength(0); + }); +}); diff --git a/package/src/store/apis/insertReaction.ts b/package/src/store/apis/insertReaction.ts index da1c2006af..72f218e232 100644 --- a/package/src/store/apis/insertReaction.ts +++ b/package/src/store/apis/insertReaction.ts @@ -2,7 +2,7 @@ import type { LocalMessage, MessageResponse, ReactionResponse } from 'stream-cha import { mapReactionToStorable } from '../mappers/mapReactionToStorable'; import { createUpdateQuery } from '../sqlite-utils/createUpdateQuery'; -import { createUpsertQuery } from '../sqlite-utils/createUpsertQuery'; +import { createUpsertQueryIfParentExists } from '../sqlite-utils/createUpsertQueryIfParentExists'; import { SqliteClient } from '../SqliteClient'; import type { PreparedQueries } from '../types'; @@ -19,7 +19,17 @@ export const insertReaction = async ({ const storableReaction = mapReactionToStorable(reaction); - queries.push(createUpsertQuery('reactions', storableReaction)); + // Only a channel's cached window of messages is stored, so a reaction can arrive for a message + // this database has never held - an old one someone reacts to, or a `/sync` replay after a cold + // start. Writing it anyway violates the `reactions.messageId` foreign key and aborts the whole + // batch it travels in. + queries.push( + createUpsertQueryIfParentExists('reactions', storableReaction, { + column: 'id', + table: 'messages', + value: reaction.message_id, + }), + ); const stringifiedNewReactionGroups = JSON.stringify(message.reaction_groups); diff --git a/package/src/store/apis/updateReaction.ts b/package/src/store/apis/updateReaction.ts index ef919bb5c6..59ea3cc86d 100644 --- a/package/src/store/apis/updateReaction.ts +++ b/package/src/store/apis/updateReaction.ts @@ -6,6 +6,7 @@ import { mapUserToStorable } from '../mappers/mapUserToStorable'; import { createDeleteQuery } from '../sqlite-utils/createDeleteQuery'; import { createUpdateQuery } from '../sqlite-utils/createUpdateQuery'; import { createUpsertQuery } from '../sqlite-utils/createUpsertQuery'; +import { createUpsertQueryIfParentExists } from '../sqlite-utils/createUpsertQueryIfParentExists'; import { SqliteClient } from '../SqliteClient'; import type { PreparedQueries } from '../types'; @@ -34,7 +35,13 @@ export const updateReaction = async ({ userId: reaction.user_id, }), ); - queries.push(createUpsertQuery('reactions', storableReaction)); + queries.push( + createUpsertQueryIfParentExists('reactions', storableReaction, { + column: 'id', + table: 'messages', + value: reaction.message_id, + }), + ); let updatedReactionGroups: string | undefined; if (message.reaction_groups) { diff --git a/package/src/store/sqlite-utils/createUpsertQuery.ts b/package/src/store/sqlite-utils/createUpsertQuery.ts index 809c789298..b5d90418b1 100644 --- a/package/src/store/sqlite-utils/createUpsertQuery.ts +++ b/package/src/store/sqlite-utils/createUpsertQuery.ts @@ -2,18 +2,17 @@ import { Schema, tables } from '../schema'; import type { PreparedQueries, TableColumnNames, TableRow } from '../types'; /** - * Creates a simple upsert query for sqlite. + * The pieces every upsert statement is built from, shared with + * {@link import('./createUpsertQueryIfParentExists').createUpsertQueryIfParentExists} so the two + * forms cannot drift in how they filter columns or resolve conflict keys. * - * @param {string} table Table name - * @param {Object} row Table row to insert or update. - * @param {Array} conflictCheckKeys Custom list of columns to check conflicts for - https://www.sqlite.org/lang_UPSERT.html. By default conflicts are checked on primary keys. - * @returns {string} Final upsert query for sqlite + * @internal */ -export const createUpsertQuery = ( +export const upsertStatementParts = ( table: T, row: Partial>, conflictCheckKeys?: Array>, -): PreparedQueries => { +) => { const filteredRow: typeof row = {}; // In case of "DO UPDATE SET", we only want to update the properties which @@ -41,8 +40,35 @@ export const createUpsertQuery = ( ${conflictMatchersWithoutPK.join(',')}` : ''; + return { + columns: fields.join(','), + conflictConstraint, + questionMarks, + values: Object.values(filteredRow), + }; +}; + +/** + * Creates a simple upsert query for sqlite. + * + * @param {string} table Table name + * @param {Object} row Table row to insert or update. + * @param {Array} conflictCheckKeys Custom list of columns to check conflicts for - https://www.sqlite.org/lang_UPSERT.html. By default conflicts are checked on primary keys. + * @returns {string} Final upsert query for sqlite + */ +export const createUpsertQuery = ( + table: T, + row: Partial>, + conflictCheckKeys?: Array>, +): PreparedQueries => { + const { columns, conflictConstraint, questionMarks, values } = upsertStatementParts( + table, + row, + conflictCheckKeys, + ); + return [ - `INSERT INTO ${table} (${fields.join(',')}) VALUES (${questionMarks}) ${conflictConstraint}`, - Object.values(filteredRow), + `INSERT INTO ${table} (${columns}) VALUES (${questionMarks}) ${conflictConstraint}`, + values, ]; }; diff --git a/package/src/store/sqlite-utils/createUpsertQueryIfParentExists.ts b/package/src/store/sqlite-utils/createUpsertQueryIfParentExists.ts new file mode 100644 index 0000000000..f31440bc38 --- /dev/null +++ b/package/src/store/sqlite-utils/createUpsertQueryIfParentExists.ts @@ -0,0 +1,58 @@ +import { upsertStatementParts } from './createUpsertQuery'; + +import { Schema } from '../schema'; +import type { PreparedQueries, TableColumnNames, TableRow } from '../types'; + +/** + * The row this write depends on - the parent side of a foreign key declared in {@link Schema}. + */ +type ParentRow = { + column: string; + table: keyof Schema; + value: unknown; +}; + +/** + * An upsert that writes nothing at all when the row it references is absent. + * + * Deliberately NOT an option on `createUpsertQuery`: "insert or update" is a contract worth keeping + * exact, and this is a third thing - "insert or update, or do nothing" - so it says so in its name + * rather than hiding behind a flag. + * + * For the tables whose schema declares a foreign key. Only part of a channel's messages are ever + * cached, so a child row can genuinely arrive for a parent this database has never held - a reaction + * on an old message, say. SQLite rejects that child, and since these queries are executed as one + * batch, the one rejected statement aborts every unrelated write alongside it. + * + * Skipping is the honest outcome rather than the lesser evil: these tables mirror what is held + * locally, so a child with no parent has nothing to hang off, and it comes back on its own once the + * parent is cached (a message arrives carrying its own reactions). + * + * Expressed as `WHERE EXISTS` inside the statement rather than as a separate probe so it costs no + * extra round trip and cannot race with the write it guards - and the lookup it does is the same + * index lookup the foreign key already forces on every insert. SQLite requires that `WHERE` when an + * `INSERT ... SELECT` is followed by `ON CONFLICT`, so the clause doing the guarding is also what + * keeps the upsert unambiguous. + * + * @param table Table name. + * @param row Table row to insert or update. + * @param parent The row that must exist for anything to be written. + * @param conflictCheckKeys Custom list of columns to check conflicts for. Defaults to primary keys. + */ +export const createUpsertQueryIfParentExists = ( + table: T, + row: Partial>, + parent: ParentRow, + conflictCheckKeys?: Array>, +): PreparedQueries => { + const { columns, conflictConstraint, questionMarks, values } = upsertStatementParts( + table, + row, + conflictCheckKeys, + ); + + return [ + `INSERT INTO ${table} (${columns}) SELECT ${questionMarks} WHERE EXISTS (SELECT 1 FROM ${parent.table} WHERE ${parent.column} = ?) ${conflictConstraint}`, + [...values, parent.value], + ]; +};