Skip to content

Commit 81e8be0

Browse files
committed
fix(mothership): don't hold a send whose network returned while it was failing
An `online` event can fire while the failing POST is still pending, so the release ran before the message was held and the message then waited for a release that had already happened. A send now notes whether the browser came back online while it was in flight, and if so goes back to the queue unheld, for the drain to send under its usual rules.
1 parent c8aa1d2 commit 81e8be0

2 files changed

Lines changed: 70 additions & 20 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,6 +2156,47 @@ describe('useChat remount send recovery', () => {
21562156
)
21572157
})
21582158

2159+
/**
2160+
* The browser can come back online while the failing POST is still pending,
2161+
* so the release fires before the message is held. It must not then wait
2162+
* for a release that already happened.
2163+
*/
2164+
it('sends a message whose POST failed after the network had already returned', async () => {
2165+
const history = idleHistory('chat-online-mid-send')
2166+
mockRequestJson.mockImplementation(() => Promise.resolve({ chat: history }))
2167+
let failFirstPost: (() => void) | undefined
2168+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2169+
const url = String(input)
2170+
if (url === '/api/mothership/chat' && init?.method === 'POST') {
2171+
state.postBodies.push(JSON.parse(String(init.body)))
2172+
if (state.postBodies.length === 1) {
2173+
return new Promise<Response>((_, reject) => {
2174+
failFirstPost = () => reject(new TypeError('Failed to fetch'))
2175+
})
2176+
}
2177+
return emptySseResponse()
2178+
}
2179+
if (url.includes('/api/mothership/chat/stream')) {
2180+
return Response.json({ error: 'Stream not found' }, { status: 404 })
2181+
}
2182+
return fetchStub(input, init)
2183+
})
2184+
const { getResult } = renderUseChatInChat(history.id, history)
2185+
await act(async () => {
2186+
void getResult().sendMessage('Sent as the network came back')
2187+
})
2188+
await waitFor(() => failFirstPost !== undefined)
2189+
2190+
await act(async () => {
2191+
window.dispatchEvent(new Event('online'))
2192+
failFirstPost?.()
2193+
})
2194+
await waitFor(() => state.postBodies.length === 2)
2195+
2196+
expect(state.postBodies[1].message).toBe('Sent as the network came back')
2197+
expect(state.postBodies[1].userMessageId).toBe(state.postBodies[0].userMessageId)
2198+
})
2199+
21592200
/** The `online` event can fire while no surface for the chat is mounted. */
21602201
it('sends a held message when its chat mounts after the network came back', async () => {
21612202
const history = idleHistory('chat-held-while-away')

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,14 @@ interface FinalizeOptions {
209209
* A send handed back to the caller instead of rendered. `userMessageId` is what
210210
* a retry reuses so the server deduplicates the two attempts. An `unreachable`
211211
* send is held in the queue until the browser is back online or the user sends
212-
* it: dispatching it again at once would fail the same way.
212+
* it: dispatching it again at once would fail the same way. When the browser
213+
* came back online while that send was failing (`networkReturned`), the release
214+
* it would have waited for has already fired, so it is not held.
213215
*/
214216
interface WithdrawnSendResult {
215217
userMessageId: string
216218
unreachable?: boolean
219+
networkReturned?: boolean
217220
}
218221

219222
/**
@@ -729,6 +732,8 @@ export function useChat(
729732
const pendingStopModeRef = useRef<StopGenerationMode | null>(null)
730733
const workflowIdRef = useRef(options?.workflowId)
731734
workflowIdRef.current = options?.workflowId
735+
/** Counts `online` events, so a send can tell the network returned while it was failing. */
736+
const onlineEventsRef = useRef(0)
732737
/** Identifies this chatless surface across mounts, for the sends it holds. */
733738
const heldSendSurface = `${scopeKey}:${options?.workflowId ?? 'home'}`
734739
const onToolResultRef = useRef(options?.onToolResult)
@@ -3422,6 +3427,7 @@ export function useChat(
34223427

34233428
let consumedByTranscript = false
34243429
let sendReachedServer = false
3430+
const onlineEventsAtSend = onlineEventsRef.current
34253431

34263432
setError(null)
34273433
setTransportStreaming()
@@ -4019,7 +4025,11 @@ export function useChat(
40194025
? 'Message not sent: Sim could not be reached. It will send when you are back online.'
40204026
: getErrorMessage(err, 'Failed to send message')
40214027
)
4022-
return { userMessageId, unreachable: true }
4028+
return {
4029+
userMessageId,
4030+
unreachable: true,
4031+
...(onlineEventsRef.current !== onlineEventsAtSend ? { networkReturned: true } : {}),
4032+
}
40234033
}
40244034

40254035
const activeStreamId = streamIdRef.current
@@ -4210,14 +4220,11 @@ export function useChat(
42104220
options?.assistantSearch,
42114221
options?.assistantSearchLevel
42124222
),
4213-
...(result.unreachable
4214-
? {
4215-
retryRequired: true,
4216-
heldUntilOnline: true,
4217-
...(activeChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)
4218-
? { heldSurface: heldSendSurface }
4219-
: {}),
4220-
}
4223+
...(result.unreachable && !result.networkReturned
4224+
? { retryRequired: true, heldUntilOnline: true }
4225+
: {}),
4226+
...(result.unreachable && activeChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)
4227+
? { heldSurface: heldSendSurface }
42214228
: {}),
42224229
})
42234230
},
@@ -4825,14 +4832,12 @@ export function useChat(
48254832
useMothershipQueueStore.getState().insertAt(dispatchChatKey, originalIndex, {
48264833
...dispatched,
48274834
...(retainedHandoff ? { queuedSendHandoff: retainedHandoff } : {}),
4828-
retryRequired: !retriesOnItsOwn,
4829-
...(withdrawn?.unreachable
4830-
? {
4831-
heldUntilOnline: true,
4832-
...(dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)
4833-
? { heldSurface: heldSendSurface }
4834-
: {}),
4835-
}
4835+
retryRequired: withdrawn?.unreachable ? !withdrawn.networkReturned : !retriesOnItsOwn,
4836+
...(withdrawn?.unreachable && !withdrawn.networkReturned
4837+
? { heldUntilOnline: true }
4838+
: {}),
4839+
...(withdrawn?.unreachable && dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)
4840+
? { heldSurface: heldSendSurface }
48364841
: {}),
48374842
...(withdrawnUserMessageId ? { resumeUserMessageId: withdrawnUserMessageId } : {}),
48384843
})
@@ -5053,12 +5058,16 @@ export function useChat(
50535058
useEffect(() => {
50545059
if (typeof window === 'undefined') return
50555060
const releaseHeldSends = () => useMothershipQueueStore.getState().releaseHeldUntilOnline()
5061+
const handleOnline = () => {
5062+
onlineEventsRef.current++
5063+
releaseHeldSends()
5064+
}
50565065
if (chatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) {
50575066
useMothershipQueueStore.getState().adoptHeldSends(chatKey, heldSendSurface)
50585067
}
50595068
if (navigator.onLine) releaseHeldSends()
5060-
window.addEventListener('online', releaseHeldSends)
5061-
return () => window.removeEventListener('online', releaseHeldSends)
5069+
window.addEventListener('online', handleOnline)
5070+
return () => window.removeEventListener('online', handleOnline)
50625071
}, [chatKey, heldSendSurface])
50635072

50645073
/** A recovered send already in history belongs to its accepted turn, even after Stop. */

0 commit comments

Comments
 (0)