Skip to content

Commit 910d7d2

Browse files
committed
fix(core): unify IO error handling and improve cross-device move reliability
- Implemented standard _handle_gio_error across all FileOperationRunnables. - Added explicit handling for cross-mount moves (WOULD_RECURSE) in TransferRunnable. - Optimized signal forwarding in FileOperations to reduce redundant handlers. - Consolidated bookmark management by removing legacy bookmarks.py bridge. - Enhanced FileJob with status tracking for better lifecycle auditing.
1 parent ef86ca5 commit 910d7d2

File tree

3 files changed

+35
-140
lines changed

3 files changed

+35
-140
lines changed

core/file_operations.py

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from PySide6.QtCore import QObject, Signal, Slot, QThreadPool, QMutex, QMutexLocker, Qt
77

88
from core.file_workers import (
9-
FileJob, FileOperationSignals, _make_gfile, _gfile_path,
9+
FileJob, FileOperationSignals, _make_gfile, _gfile_path, _split_name_ext,
1010
TransferRunnable, RenameRunnable, CreateFolderRunnable,
1111
CreateFileRunnable, CreateSymlinkRunnable,
1212
generate_candidate_path,
@@ -18,15 +18,6 @@
1818

1919
_MAX_RENAME_ATTEMPTS = 1000
2020

21-
def _split_name_ext(filename: str) -> tuple[str, str]:
22-
"""Split filename into (base, ext). Handles .tar.gz and dotfiles."""
23-
if filename.endswith(".tar.gz"):
24-
return filename[:-7], ".tar.gz"
25-
dot = filename.rfind(".")
26-
if dot <= 0:
27-
return filename, ""
28-
return filename[:dot], filename[dot:]
29-
3021
class FileOperations(QObject):
3122
"""Central controller for non-blocking file I/O via QThreadPool."""
3223

@@ -52,27 +43,18 @@ def __init__(self, parent=None):
5243

5344
# Internal Signal Hub
5445
self._signals = FileOperationSignals()
55-
self._signals.started.connect(self._on_started)
56-
self._signals.progress.connect(self._on_progress)
46+
self._signals.started.connect(self.operationStarted)
47+
self._signals.progress.connect(self.operationProgress)
5748
self._signals.finished.connect(self._on_finished)
5849
self._signals.operationError.connect(self._on_error)
5950

6051
# Trash Specific Internal Signals
6152
self._signals.itemListed.connect(self.itemListed)
6253
self._signals.trashNotSupported.connect(self.trashNotSupported)
63-
64-
def setUndoManager(self, undo_manager):
65-
self._undo_manager = undo_manager
6654

6755
# -------------------------------------------------------------------------
6856
# SIGNAL HANDLERS
6957
# -------------------------------------------------------------------------
70-
def _on_started(self, job_id: str, op_type: str, path: str):
71-
self.operationStarted.emit(job_id, op_type, path)
72-
73-
def _on_progress(self, job_id: str, current: int, total: int):
74-
self.operationProgress.emit(job_id, current, total)
75-
7658
def _on_finished(self, tid: str, job_id: str, op_type: str, result_path: str, success: bool, message: str):
7759
with QMutexLocker(self._mutex):
7860
if job_id in self._jobs:

core/file_workers.py

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,11 @@ class FileJob:
7171
dest: str = "" # Destination path (or new name for rename)
7272
transaction_id: str = "" # Links this job to a larger transaction (batch)
7373
cancellable: Gio.Cancellable = field(default_factory=Gio.Cancellable)
74-
auto_rename: bool = False # [NEW] If True, automatically find a free name (For New Folder / Duplicate)
74+
auto_rename: bool = False # If True, automatically find a free name (For New Folder / Duplicate)
7575
skipped_files: List[str] = field(default_factory=list) # For partial success
7676
overwrite: bool = False # If True, overwrite existing files without prompt
7777
rename_to: str = "" # Specific for Restore: if set, restore with this filename
78+
status: str = "pending" # Lifecycle: pending → running → done/error/cancelled
7879

7980
class FileOperationSignals(QObject):
8081
"""
@@ -131,6 +132,25 @@ def _progress_callback(self, current_bytes, total_bytes, user_data):
131132
"""Gio progress callback adapter."""
132133
self.emit_progress(current_bytes, total_bytes)
133134

135+
def _handle_gio_error(self, e: GLib.Error, op_type: str, path: str, conflict_data=None):
136+
"""
137+
Shared GIO error handler. Covers EXISTS (conflict), CANCELLED (silent),
138+
and all other errors. Always calls emit_finished(False, ...).
139+
"""
140+
if e.code == Gio.IOErrorEnum.EXISTS:
141+
if conflict_data is None:
142+
conflict_data = build_conflict_payload(path, path)
143+
self.signals.operationError.emit(
144+
self.job.transaction_id, self.job.id,
145+
op_type, path, str(e), conflict_data
146+
)
147+
elif e.code != Gio.IOErrorEnum.CANCELLED:
148+
self.signals.operationError.emit(
149+
self.job.transaction_id, self.job.id,
150+
op_type, path, str(e), None
151+
)
152+
self.emit_finished(False, str(e))
153+
134154
def _run_create_operation(self, op_type: str, target_path: str):
135155
"""
136156
Shared logic for all create operations (folder, file, symlink).
@@ -151,12 +171,7 @@ def _run_create_operation(self, op_type: str, target_path: str):
151171
if e.code == Gio.IOErrorEnum.EXISTS:
152172
counter += 1
153173
continue
154-
if e.code != Gio.IOErrorEnum.CANCELLED:
155-
self.signals.operationError.emit(
156-
self.job.transaction_id, self.job.id,
157-
op_type, candidate, str(e), None
158-
)
159-
self.emit_finished(False, str(e))
174+
self._handle_gio_error(e, op_type, candidate)
160175
return
161176
self.emit_finished(False, "Auto-rename limit reached")
162177
return
@@ -167,27 +182,12 @@ def _run_create_operation(self, op_type: str, target_path: str):
167182
self._do_create(gfile)
168183
self.emit_finished(True, target_path)
169184
except GLib.Error as e:
170-
if e.code == Gio.IOErrorEnum.EXISTS:
171-
conflict_data = build_conflict_payload(target_path, target_path)
172-
self.signals.operationError.emit(
173-
self.job.transaction_id, self.job.id,
174-
op_type, target_path, str(e), conflict_data
175-
)
176-
self.emit_finished(False, str(e))
177-
else:
178-
if e.code != Gio.IOErrorEnum.CANCELLED:
179-
self.signals.operationError.emit(
180-
self.job.transaction_id, self.job.id,
181-
op_type, target_path, str(e), None
182-
)
183-
self.emit_finished(False, str(e))
185+
self._handle_gio_error(e, op_type, target_path)
184186

185187
def _do_create(self, gfile):
186188
"""Override in subclass. Execute the actual Gio create call."""
187189
raise NotImplementedError
188190

189-
# Removed local build_conflict_data helper, using global build_conflict_payload
190-
191191
def _split_name_ext(filename: str) -> tuple[str, str]:
192192
"""Split filename into (base, ext). Handles .tar.gz and dotfiles."""
193193
if filename.endswith(".tar.gz"):
@@ -260,7 +260,8 @@ def run(self):
260260
self.emit_finished(True, "Success", result_override=final_dest)
261261
return
262262
except GLib.Error as e:
263-
# If it's a cross-device move, or directory merge, handle manually
263+
# Cross-device move or directory merge: handle manually.
264+
# 29 = G_IO_ERROR_WOULD_RECURSE (not always exposed in Python bindings)
264265
if e.code in [Gio.IOErrorEnum.NOT_SUPPORTED, Gio.IOErrorEnum.WOULD_MERGE, 29]:
265266
# Cross-device move or merge -> Recursive copy + delete
266267
# Note: Recursive transfer logic handles its own errors, we need to catch EXISTS there too?
@@ -302,13 +303,10 @@ def run(self):
302303
else:
303304
# Conflict!
304305
conflict_data = build_conflict_payload(self.job.source, final_dest)
305-
self.signals.operationError.emit(self.job.transaction_id, self.job.id, self.job.op_type, final_dest, str(e), conflict_data)
306-
self.emit_finished(False, str(e))
306+
self._handle_gio_error(e, self.job.op_type, final_dest, conflict_data)
307307
return
308308
else:
309-
if e.code != Gio.IOErrorEnum.CANCELLED:
310-
self.signals.operationError.emit(self.job.transaction_id, self.job.id, self.job.op_type, final_dest, str(e), None)
311-
self.emit_finished(False, str(e))
309+
self._handle_gio_error(e, self.job.op_type, final_dest)
312310
return
313311

314312
# If loop finishes without return (counter maxed)
@@ -348,7 +346,7 @@ def _recursive_transfer(self, source, dest, is_move=False):
348346
if is_move and not local_error:
349347
try:
350348
source.delete(self.job.cancellable)
351-
except:
349+
except Exception:
352350
pass
353351
else:
354352
# File Transfer
@@ -367,22 +365,17 @@ def run(self):
367365
# dest holds the new NAME, not full path
368366
result = gfile.set_display_name(self.job.dest, self.job.cancellable)
369367
if result:
370-
# Use the Gio.File returned by set_display_name for the correct path
371368
abs_path = _gfile_path(result)
372369
self.emit_finished(True, "Success", result_override=abs_path)
373370
else:
374-
self.emit_finished(False, str(e))
371+
self.emit_finished(False, "Rename returned no result")
375372
except GLib.Error as e:
376373
if e.code == Gio.IOErrorEnum.EXISTS:
377-
src_gfile = _make_gfile(self.job.source)
378-
dest_path = _gfile_path(src_gfile.get_parent().get_child(self.job.dest))
374+
dest_path = _gfile_path(gfile.get_parent().get_child(self.job.dest))
379375
conflict_data = build_conflict_payload(self.job.source, dest_path)
380-
self.signals.operationError.emit(self.job.transaction_id, self.job.id, "rename", dest_path, str(e), conflict_data)
381-
self.emit_finished(False, str(e))
376+
self._handle_gio_error(e, "rename", dest_path, conflict_data)
382377
else:
383-
if e.code != Gio.IOErrorEnum.CANCELLED:
384-
self.signals.operationError.emit(self.job.transaction_id, self.job.id, "rename", self.job.source, str(e), None)
385-
self.emit_finished(False, str(e))
378+
self._handle_gio_error(e, "rename", self.job.source)
386379

387380
class CreateFolderRunnable(FileOperationRunnable):
388381
"""Creates a new directory."""

core/gio_bridge/bookmarks.py

Lines changed: 0 additions & 80 deletions
This file was deleted.

0 commit comments

Comments
 (0)