Skip to content

Commit 2c1518e

Browse files
committed
fix(files): surface a failed pre-retype list refresh instead of swallowing it
`invalidateQueries` resolves whether or not the refetch succeeded, so the refresh reported success while leaving the dead storage key in the cache - the caller awaiting a usable key could not tell the two apart. The hook now rejects on a failed refetch. The retype logs and proceeds rather than aborting: the edits are already durable, the type change is explicit, and the rename's own invalidation refetches straight after, so the cost of a failed refresh is one stale first paint - the pre-fix behaviour - not a lost change.
1 parent e177502 commit 2c1518e

3 files changed

Lines changed: 59 additions & 9 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/files.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,15 @@ export function Files() {
12451245
// The persist minted a new storage key and deleted the previous blob, so the cached record
12461246
// the viewer renders from now points at a key that 404s. Wait for the refreshed list before
12471247
// the rename swaps editors, or the newly mounted viewer reads the dead key.
1248-
await refreshFiles(workspaceId)
1248+
try {
1249+
await refreshFiles(workspaceId)
1250+
} catch (err) {
1251+
// Proceed rather than abort: the edits are already durable, the retype is an explicit
1252+
// action, and the rename's own invalidation refetches immediately after. The cost of a
1253+
// failed refresh is one stale first paint, which is the pre-fix behaviour - not losing
1254+
// the user's type change on a transient list fetch.
1255+
logger.warn('Retyping against a stale file list; the first read may 404', { err })
1256+
}
12491257
} else {
12501258
// Not an error - `unchanged` means there was nothing to write, and `skipped` means the write
12511259
// did not land in time. The retype proceeds either way; this is the breadcrumb for a stale

apps/sim/hooks/queries/workspace-files.test.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,40 @@ describe('useRefreshWorkspaceFiles', () => {
340340
unmount()
341341
})
342342

343+
/**
344+
* react-query resolves an invalidation whether or not the refetch succeeded. A caller awaiting
345+
* this for a usable storage key would otherwise read the dead one back as if it were fresh.
346+
*/
347+
it('rejects when the refetch fails instead of resolving on the stale cache', async () => {
348+
let call = 0
349+
const queryFn = vi.fn(async () => {
350+
call += 1
351+
if (call > 1) throw new Error('network down')
352+
return [{ id: 'file-1', key: 'workspace/ws-1/old-key' }]
353+
})
354+
355+
const { refresh, queryClient, unmount } = renderRefresh()
356+
const queryKey = workspaceFilesKeys.list(WS, 'active')
357+
await act(async () => {
358+
await queryClient.fetchQuery({ queryKey, queryFn })
359+
})
360+
361+
let rejection: unknown = null
362+
await act(async () => {
363+
await refresh()(WS).catch((err) => {
364+
rejection = err
365+
})
366+
})
367+
368+
expect(rejection).toBeInstanceOf(Error)
369+
// The stale record is still cached — the caller has to decide what to do about it, not be told
370+
// the refresh worked.
371+
expect(queryClient.getQueryData<{ key: string }[]>(queryKey)?.[0].key).toBe(
372+
'workspace/ws-1/old-key'
373+
)
374+
unmount()
375+
})
376+
343377
it('leaves another workspace list alone', async () => {
344378
const otherQueryFn = vi.fn(async () => [{ id: 'file-2', key: 'k2' }])
345379
const { refresh, queryClient, unmount } = renderRefresh()

apps/sim/hooks/queries/workspace-files.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -581,25 +581,33 @@ export function useUpdateWorkspaceFileContent() {
581581
}
582582

583583
/**
584-
* Refetch the workspace file list and resolve once the fresh records have landed.
584+
* Refetch the workspace file list, resolving once the fresh records have landed and **rejecting**
585+
* if the refetch failed.
585586
*
586587
* Every content write mints a new storage key and deletes the previous blob, so a cached record's
587588
* `key` is dead the moment one lands. A caller that is about to mount a viewer from that record -
588589
* a retype, which swaps editors optimistically - has to wait for the refreshed list, or the new
589590
* viewer fetches a key the store has already deleted.
591+
*
592+
* The rejection is the point: react-query resolves an invalidation whether or not the refetch
593+
* succeeded, so a caller awaiting a usable key cannot otherwise tell fresh records from the dead
594+
* ones still sitting in the cache. Callers decide what a failure means for them.
590595
*/
591596
export function useRefreshWorkspaceFiles() {
592597
const queryClient = useQueryClient()
593598

594599
return useCallback(
595600
(workspaceId: string) =>
596-
queryClient.invalidateQueries({
597-
queryKey: workspaceFilesKeys.workspaceLists(workspaceId),
598-
// `all`, not the default `active`: the caller awaits this to get a usable key back, and an
599-
// invalidation that only marks an unobserved list stale resolves immediately with the dead
600-
// key still cached - the exact staleness this exists to close.
601-
refetchType: 'all',
602-
}),
601+
queryClient.invalidateQueries(
602+
{
603+
queryKey: workspaceFilesKeys.workspaceLists(workspaceId),
604+
// `all`, not the default `active`: the caller awaits this to get a usable key back, and an
605+
// invalidation that only marks an unobserved list stale resolves immediately with the dead
606+
// key still cached - the exact staleness this exists to close.
607+
refetchType: 'all',
608+
},
609+
{ throwOnError: true }
610+
),
603611
[queryClient]
604612
)
605613
}

0 commit comments

Comments
 (0)