Skip to content

Commit 9b5c57c

Browse files
committed
Fix copy operationId capture race condition
- Buffer events arriving before copyFiles() returns the real operationId, then replay matching ones - Eliminates 5x duplicated filter block via shared filterEvent() guard
1 parent e2c6ee1 commit 9b5c57c

1 file changed

Lines changed: 56 additions & 38 deletions

File tree

apps/desktop/src/lib/file-operations/copy/CopyProgressDialog.svelte

Lines changed: 56 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,50 @@
9494
let isCancelling = $state(false)
9595
let isRollingBack = $state(false)
9696
97+
// Events that arrived before we know our operationId (from the copyFiles() response).
98+
// Without buffering, a stale event from a previous copy could claim the ID slot first.
99+
type BufferedEvent =
100+
| { type: 'progress'; event: WriteProgressEvent }
101+
| { type: 'complete'; event: WriteCompleteEvent }
102+
| { type: 'error'; event: WriteErrorEvent }
103+
| { type: 'cancelled'; event: WriteCancelledEvent }
104+
| { type: 'conflict'; event: WriteConflictEvent }
105+
let pendingEvents: BufferedEvent[] = []
106+
107+
/** Returns true if the event belongs to this operation and should be processed. */
108+
function filterEvent(entry: BufferedEvent): boolean {
109+
if (operationId === null) {
110+
pendingEvents.push(entry)
111+
return false
112+
}
113+
return entry.event.operationId === operationId
114+
}
115+
116+
function replayBufferedEvents() {
117+
const events = pendingEvents
118+
pendingEvents = []
119+
for (const entry of events) {
120+
if (entry.event.operationId !== operationId) continue
121+
switch (entry.type) {
122+
case 'progress':
123+
handleProgress(entry.event)
124+
break
125+
case 'complete':
126+
handleComplete(entry.event)
127+
break
128+
case 'error':
129+
handleError(entry.event)
130+
break
131+
case 'cancelled':
132+
handleCancelled(entry.event)
133+
break
134+
case 'conflict':
135+
handleConflict(entry.event)
136+
break
137+
}
138+
}
139+
}
140+
97141
// Conflict state
98142
let conflictEvent = $state<WriteConflictEvent | null>(null)
99143
let isResolvingConflict = $state(false)
@@ -134,15 +178,7 @@
134178
}
135179
136180
function handleProgress(event: WriteProgressEvent) {
137-
// Filter by operationId (events are global)
138-
// If operationId is null, accept the event and capture the ID (handles race condition
139-
// where events arrive before copyFiles() returns the operationId to the frontend)
140-
if (operationId === null) {
141-
operationId = event.operationId
142-
log.debug('Captured operationId from event: {operationId}', { operationId })
143-
} else if (event.operationId !== operationId) {
144-
return
145-
}
181+
if (!filterEvent({ type: 'progress', event })) return
146182
147183
log.debug('Progress event: {phase} {filesDone}/{filesTotal} files, {bytesDone}/{bytesTotal} bytes', {
148184
phase: event.phase,
@@ -161,13 +197,7 @@
161197
}
162198
163199
function handleComplete(event: WriteCompleteEvent) {
164-
// Filter by operationId (events are global)
165-
// Accept if operationId is null (race condition) or matches
166-
if (operationId === null) {
167-
operationId = event.operationId
168-
} else if (event.operationId !== operationId) {
169-
return
170-
}
200+
if (!filterEvent({ type: 'complete', event })) return
171201
172202
log.info('Copy complete: {filesProcessed} files, {bytesProcessed} bytes', {
173203
filesProcessed: event.filesProcessed,
@@ -179,13 +209,7 @@
179209
}
180210
181211
function handleError(event: WriteErrorEvent) {
182-
// Filter by operationId (events are global)
183-
// Accept if operationId is null (race condition) or matches
184-
if (operationId === null) {
185-
operationId = event.operationId
186-
} else if (event.operationId !== operationId) {
187-
return
188-
}
212+
if (!filterEvent({ type: 'error', event })) return
189213
190214
log.error('Copy error: {errorType}', { errorType: event.error.type, error: event.error })
191215
@@ -194,13 +218,7 @@
194218
}
195219
196220
function handleCancelled(event: WriteCancelledEvent) {
197-
// Filter by operationId (events are global)
198-
// Accept if operationId is null (race condition) or matches
199-
if (operationId === null) {
200-
operationId = event.operationId
201-
} else if (event.operationId !== operationId) {
202-
return
203-
}
221+
if (!filterEvent({ type: 'cancelled', event })) return
204222
205223
log.info('Copy cancelled after {filesProcessed} files, rolledBack={rolledBack}', {
206224
filesProcessed: event.filesProcessed,
@@ -212,13 +230,7 @@
212230
}
213231
214232
function handleConflict(event: WriteConflictEvent) {
215-
// Filter by operationId (events are global)
216-
// Accept if operationId is null (race condition) or matches
217-
if (operationId === null) {
218-
operationId = event.operationId
219-
} else if (event.operationId !== operationId) {
220-
return
221-
}
233+
if (!filterEvent({ type: 'conflict', event })) return
222234
223235
log.info('Conflict detected: {sourcePath} -> {destinationPath}', {
224236
sourcePath: event.sourcePath,
@@ -300,6 +312,7 @@
300312
301313
operationId = result.operationId
302314
log.info('Copy operation started with operationId: {operationId}', { operationId })
315+
replayBufferedEvents()
303316
} catch (err) {
304317
log.error('Failed to start copy operation: {error}', { error: err })
305318
cleanup()
@@ -995,23 +1008,28 @@
9951008
border-top: 1px solid var(--color-border-primary);
9961009
}
9971010
998-
/* Size colors (matching file list) */
1011+
/* Size colors (matching file list) - these are used dynamically */
1012+
/*noinspection CssUnusedSymbol*/
9991013
.size-bytes {
10001014
color: var(--color-text-secondary);
10011015
}
10021016
1017+
/*noinspection CssUnusedSymbol*/
10031018
.size-kb {
10041019
color: var(--color-size-kb);
10051020
}
10061021
1022+
/*noinspection CssUnusedSymbol*/
10071023
.size-mb {
10081024
color: var(--color-size-mb);
10091025
}
10101026
1027+
/*noinspection CssUnusedSymbol*/
10111028
.size-gb {
10121029
color: var(--color-size-gb);
10131030
}
10141031
1032+
/*noinspection CssUnusedSymbol*/
10151033
.size-tb {
10161034
color: var(--color-size-tb);
10171035
}

0 commit comments

Comments
 (0)