Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle permission errors during plugin loading #9229

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

syntaxsurge
Copy link
Contributor

@syntaxsurge syntaxsurge commented Feb 17, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

This pull request introduces a modification to the load_plugins function within plugins.py to gracefully handle PermissionError exceptions that occur during the plugin loading process. The modification ensures that yt-dlp can continue to run without crashing when it encounters permission issues, specifically when trying to access directories for which it does not have permissions.

This issue has been observed in environments where yt-dlp attempts to access restricted directories (e.g., /root/yt_dlp_plugins) and throws a PermissionError, leading to the program's termination. Such behavior has been reported when running yt-dlp --version in a virtual environment and similarly affects other operations, including starting FastAPI applications with Gunicorn where yt-dlp is a dependency.

Example Error:

PermissionError: [Errno 13] Permission denied: '/root/yt_dlp_plugins'

The proposed change wraps the plugin loading iteration in a try-except block to catch PermissionError exceptions, logs the error for debugging purposes, and allows the program to proceed by skipping the problematic plugin loading phase.

Fixes: Not applicable as this is a general improvement to error handling.

Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

@seproDev seproDev added the bug Bug that is not site-specific label Feb 17, 2024
@pukkandan
Copy link
Member

Don't wrap the entire code block. Only try except where the error happens

Restructure the error handling in the module loading process to precisely catch PermissionError during module discovery and to localize exception handling around module loading operations. This approach avoids broad try-except wrapping and instead applies try-except blocks where errors are explicitly expected, enhancing code clarity and maintainability.
@syntaxsurge
Copy link
Contributor Author

syntaxsurge commented Feb 20, 2024

pukkandan

I've addressed the issue with the nested try-block. This error occurs on Linux, specifically Ubuntu 22, in my case. The problem arose when I attempted to use yt-dlp as a non-root user who does not have permissions for the root directory. Initially, it merely raised an exception, but we want to handle this exception more gracefully by notifying the user that yt-dlp is operating without plugins because the plugin classes cannot be loaded due to insufficient permissions.

Error message:

(venv) jade@jade-ASUS-TUF-Gaming-A15:~/PycharmProjects/api_saas/kubernetes$ yt-dlp --version
Traceback (most recent call last):
  File "/home/jade/PycharmProjects/api_saas/venv/bin/yt-dlp", line 5, in <module>
    from yt_dlp import main
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/__init__.py", line 19, in <module>
    from .downloader.external import get_external_downloader
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/downloader/__init__.py", line 26, in <module>
    from .external import FFmpegFD, get_external_downloader
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/downloader/external.py", line 14, in <module>
    from ..postprocessor.ffmpeg import EXT_TO_OUT_FORMATS, FFmpegPostProcessor
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/postprocessor/__init__.py", line 38, in <module>
    _PLUGIN_CLASSES = load_plugins('postprocessor', 'PP')
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/plugins.py", line 139, in load_plugins
    for finder, module_name, _ in iter_modules(name):
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/plugins.py", line 122, in iter_modules
    pkg = importlib.import_module(fullname)
  File "/usr/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1002, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 945, in _find_spec
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/plugins.py", line 99, in find_spec
    search_locations = list(map(str, self.search_locations(fullname)))
  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/plugins.py", line 89, in search_locations
    if candidate.is_dir():
  File "/usr/lib/python3.10/pathlib.py", line 1305, in is_dir
    return S_ISDIR(self.stat().st_mode)
  File "/usr/lib/python3.10/pathlib.py", line 1097, in stat
    return self._accessor.stat(self, follow_symlinks=follow_symlinks)

After implementing the proposed solution:

(venv) jade@jade-ASUS-TUF-Gaming-A15:~/PycharmProjects/api_saas/kubernetes$ yt-dlp --version
Permission error while accessing modules in postprocessor: [Errno 13] Permission denied: '/root/yt_dlp_plugins'
2023.12.30

As observed, yt-dlp does not load correctly when executed as a non-root user in Ubuntu, both in the terminal and when imported in Python. This issue stems from the fact that yt-dlp attempts to load plugins at every startup. However, some users may not have the necessary permissions to access the plugin directory located in /root. To address this, the program has been modified to run normally while logging a message to inform users that they do not have permission to access the plugins, and therefore, the plugins will not be loaded.

After applying this fix, running yt-dlp --version in the terminal now correctly displays the version, along with logs indicating that the plugin has not been loaded.

@pukkandan
Copy link
Member

  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/plugins.py", line 89, in search_locations
    if candidate.is_dir():

The try..except is still too broad. Why not move it to where error actually happens? Like this

diff --git a/yt_dlp/plugins.py b/yt_dlp/plugins.py
index 6422c7a51..9f61bb6e0 100644
--- a/yt_dlp/plugins.py
+++ b/yt_dlp/plugins.py
@@ -86,11 +86,14 @@ def _get_package_paths(*root_paths, containing_folder='plugins'):
         parts = Path(*fullname.split('.'))
         for path in orderedSet(candidate_locations, lazy=True):
             candidate = path / parts
-            if candidate.is_dir():
-                yield candidate
-            elif path.suffix in ('.zip', '.egg', '.whl') and path.is_file():
-                if parts in dirs_in_zip(path):
+            try:
+                if candidate.is_dir():
                     yield candidate
+                elif path.suffix in ('.zip', '.egg', '.whl') and path.is_file():
+                    if parts in dirs_in_zip(path):
+                        yield candidate
+            except PermissionError as e:
+                write_string(f'Permission error while accessing modules in {e.filename}\n')

     def find_spec(self, fullname, path=None, target=None):
         if fullname not in self.packages:

Narrow down the try-except scope in _get_package_paths to specifically catch PermissionError when accessing directories. This targeted approach improves error handling by logging permission issues only where they occur, ensuring the application informs users about inaccessible plugin directories without broadly catching exceptions.
@syntaxsurge
Copy link
Contributor Author

  File "/home/jade/PycharmProjects/api_saas/venv/lib/python3.10/site-packages/yt_dlp/plugins.py", line 89, in search_locations
    if candidate.is_dir():

The try..except is still too broad. Why not move it to where error actually happens? Like this

diff --git a/yt_dlp/plugins.py b/yt_dlp/plugins.py
index 6422c7a51..9f61bb6e0 100644
--- a/yt_dlp/plugins.py
+++ b/yt_dlp/plugins.py
@@ -86,11 +86,14 @@ def _get_package_paths(*root_paths, containing_folder='plugins'):
         parts = Path(*fullname.split('.'))
         for path in orderedSet(candidate_locations, lazy=True):
             candidate = path / parts
-            if candidate.is_dir():
-                yield candidate
-            elif path.suffix in ('.zip', '.egg', '.whl') and path.is_file():
-                if parts in dirs_in_zip(path):
+            try:
+                if candidate.is_dir():
                     yield candidate
+                elif path.suffix in ('.zip', '.egg', '.whl') and path.is_file():
+                    if parts in dirs_in_zip(path):
+                        yield candidate
+            except PermissionError as e:
+                write_string(f'Permission error while accessing modules in {e.filename}\n')

     def find_spec(self, fullname, path=None, target=None):
         if fullname not in self.packages:

Thank you for your guidance. I've implemented the change as you suggested. This should make the error handling more specific and informative, directly addressing the issue without unnecessarily broad exception catching. I appreciate your feedback and have committed the changes accordingly.

The output is now:

(venv) jade@jade-ASUS-TUF-Gaming-A15:~/PycharmProjects/api_saas/kubernetes$ yt-dlp --version
Permission error while accessing modules in /root/yt_dlp_plugins: [Errno 13] Permission denied: '/root/yt_dlp_plugins'
2023.12.30

@pukkandan pukkandan merged commit 9a8afad into yt-dlp:master Feb 20, 2024
14 checks passed
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
Authored by: syntaxsurge, pukkandan
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that is not site-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants