Fix: Prevent PermissionError during browser cleanup on Windows #48
Conversation
|
Hi @Nabeelshar, thanks for your contribution! I'll review now |
pydoll/browser/base.py
Outdated
| """ | ||
| if await self._is_browser_running(): | ||
| await self._execute_command(BrowserCommands.CLOSE) | ||
| self._browser_process_manager.stop_process() | ||
|
|
There was a problem hiding this comment.
You can remove these blank lines
pydoll/browser/managers.py
Outdated
| @@ -95,6 +97,7 @@ class TempDirectoryManager: | |||
| def __init__(self, temp_dir_factory=TemporaryDirectory): | |||
| self._temp_dir_factory = temp_dir_factory | |||
| self._temp_dirs = [] | |||
| self._logger = logging.getLogger(__name__) | |||
There was a problem hiding this comment.
Add the logger as a global variable, that way we can use it in any class of this file:
logger = logging.getLogger(__name__)
class ...
pydoll/browser/managers.py
Outdated
| self._safe_rmtree(temp_dir.name) | ||
| self._temp_dirs = [] | ||
|
|
||
| def _safe_rmtree(self, path): |
There was a problem hiding this comment.
I think you can refactor this to better readability. Here's a suggestion:
def _safe_rmtree(self, path):
if not os.path.exists(path):
return
if self._attempt_standard_cleanup(path):
return
self._manual_cleanup(path)
def _attempt_standard_cleanup(self, path, max_attempts=3):
# use recursion here, since there's just 3 iterations, it's more readable
try:
shutil.rmtree(path)
return True
except (OSError, PermissionError) as e:
if not max_attempts:
logger.warning(f"log here") # global logger
return False
time.sleep(0.5)
return self._attempt_standard_cleanup(path, max_attempts - 1)
def _manual_cleanup(self, path):
try:
self._remove_contents(path)
with suppress(OSError, PermissionError):
os.rmdir(path)
except Exception as e:
logger.warning(f"log here")
def _remove_contents(self, path):
for root, dirs, files in os.walk(path, topdown=False):
for name in files:
with suppress(PermissionError, OSError):
os.unlink(os.path.join(root, name))
for name in dirs:
with suppress(PermissionError, OSError):
os.rmdir(os.path.join(root, name))Be free to make your own ideas, but this method needs to be refactores, it's too large
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Also, please follow the Conventional Commits style; without it, I can't generate the releases. Just undo your commit and commit again with the |
Resolves an issue #44 where temporary browser files remain locked during Python's exit cleanup process, causing PermissionError exceptions. Modified browser management to ensure proper resource release and file handle closure before program termination.
Error addressed: