Skip to content

Commit 3dc5c7b

Browse files
DerDreschnersusnux
authored andcommitted
fix(dav): Prevent race condition in useDAVFiles
Assisted-by: ClaudeCode:claude-fable-5 Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
1 parent e541e9a commit 3dc5c7b

2 files changed

Lines changed: 64 additions & 7 deletions

File tree

‎lib/composables/dav.spec.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,4 +240,53 @@ describe('dav composable', () => {
240240
await waitRefLoaded(isLoading)
241241
expect(abort).toBeCalledTimes(1)
242242
})
243+
244+
it('a superseded (aborted) load does not clear the loading state of the newer load', async () => {
245+
// Each getDirectoryContents call stays pending until resolved manually,
246+
// and rejects with an AbortError when its signal is aborted — mirroring a
247+
// real DAV request that is cancelled on fast navigation.
248+
const pending: Array<{ resolve: () => void }> = []
249+
const client = {
250+
stat: vi.fn((v) => ({ data: { path: v } })),
251+
getDirectoryContents: vi.fn((_path, opts) => new Promise((resolve, reject) => {
252+
opts.signal?.addEventListener('abort', () => {
253+
const error = new Error('aborted')
254+
error.name = 'AbortError'
255+
reject(error)
256+
})
257+
pending.push({ resolve: () => resolve({ data: [] }) })
258+
})),
259+
}
260+
nextcloudFiles.getClient.mockImplementationOnce(() => client)
261+
nextcloudFiles.resultToNode.mockImplementation((v) => v)
262+
263+
const view = ref<'files' | 'recent' | 'favorites'>('files')
264+
const path = ref('/')
265+
const { loadFiles, isLoading } = useDAVFiles(view, path)
266+
267+
// Load A is in flight
268+
loadFiles()
269+
await nextTick()
270+
expect(pending).toHaveLength(1)
271+
expect(isLoading.value).toBe(true)
272+
273+
// Load B supersedes A: navigating aborts A and starts B loading
274+
path.value = '/subdir/'
275+
await nextTick()
276+
expect(pending).toHaveLength(2)
277+
278+
// Let A's aborted request reject and run its catch/finally
279+
await nextTick()
280+
await nextTick()
281+
282+
// Regression: A's finally must not flip isLoading to false while B is
283+
// still loading — otherwise the FilePicker sees isLoading=false with a
284+
// null folder and confirms an empty selection ("No nodes selected").
285+
expect(isLoading.value).toBe(true)
286+
287+
// Completing B settles the loading state
288+
pending[1].resolve()
289+
await waitRefLoaded(isLoading)
290+
expect(isLoading.value).toBe(false)
291+
})
243292
})

‎lib/composables/dav.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,20 @@ export function useDAVFiles(
6868
async function loadDAVFiles() {
6969
if (abortController) {
7070
abortController.abort()
71-
abortController = undefined
7271
}
7372

74-
abortController = new AbortController()
73+
const thisAbortController = new AbortController()
74+
abortController = thisAbortController
7575
isLoading.value = true
7676
try {
7777
if (currentView.value === 'favorites') {
78-
files.value = await getFavoriteNodes({ client, path: currentPath.value, signal: abortController.signal })
78+
files.value = await getFavoriteNodes({ client, path: currentPath.value, signal: thisAbortController.signal })
7979
folder.value = null
8080
} else if (currentView.value === 'recent') {
81-
files.value = await getRecentNodes({ client, signal: abortController.signal })
81+
files.value = await getRecentNodes({ client, signal: thisAbortController.signal })
8282
folder.value = null
8383
} else {
84-
const content = await getNodes({ client, path: currentPath.value, signal: abortController.signal })
84+
const content = await getNodes({ client, path: currentPath.value, signal: thisAbortController.signal })
8585
folder.value = content.folder
8686
files.value = content.contents
8787
}
@@ -92,8 +92,16 @@ export function useDAVFiles(
9292
}
9393
throw error
9494
} finally {
95-
abortController = undefined
96-
isLoading.value = false
95+
// Only clear the shared loading state if this invocation is still the
96+
// current load. When a newer loadDAVFiles() has already aborted and
97+
// superseded this one, resetting here would flip `isLoading` to false
98+
// (and drop the newer abort controller) while the newer load is still
99+
// in flight and `folder` is still null — which lets the FilePicker
100+
// confirm with an empty selection and throw "No nodes selected".
101+
if (abortController === thisAbortController) {
102+
abortController = undefined
103+
isLoading.value = false
104+
}
97105
}
98106
}
99107

0 commit comments

Comments
 (0)