Skip to content

[py] Better error for downloads on local webdrivers #15756

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

Merged
merged 5 commits into from
May 22, 2025

Conversation

cgoldberg
Copy link
Contributor

@cgoldberg cgoldberg commented May 20, 2025

User description

🔗 Related Issues

Fixes #15753

💥 What does this PR do?

This PR raises NotImplementedError when trying to call get_downloadable_files() or download_file() on any local WebDriver. These methods are only available on the parent WebDriver class (AKA Remote).

This just makes the error thrown a little less cryptic for the user.

🔄 Types of changes

  • Bug fix

PR Type

Bug fix


Description

  • Raise NotImplementedError for download methods on local drivers

  • Improve error clarity for unsupported download operations

  • Affects Chromium, Firefox, IE, Safari, WebKitGTK, and WPEWebKit drivers


Changes walkthrough 📝

Relevant files
Bug fix
webdriver.py
Add NotImplementedError for download methods in Chromium driver

py/selenium/webdriver/chromium/webdriver.py

  • Added download_file and get_downloadable_files methods
  • Both methods now raise NotImplementedError
  • +6/-0     
    webdriver.py
    Add NotImplementedError for download methods in Firefox driver

    py/selenium/webdriver/firefox/webdriver.py

  • Added download_file and get_downloadable_files methods
  • Both methods now raise NotImplementedError
  • +6/-0     
    webdriver.py
    Add NotImplementedError for download methods in IE driver

    py/selenium/webdriver/ie/webdriver.py

  • Added download_file and get_downloadable_files methods
  • Both methods now raise NotImplementedError
  • +6/-0     
    webdriver.py
    Add NotImplementedError for download methods in Safari driver

    py/selenium/webdriver/safari/webdriver.py

  • Added download_file and get_downloadable_files methods
  • Both methods now raise NotImplementedError
  • +6/-0     
    webdriver.py
    Add NotImplementedError for download methods in WebKitGTK driver

    py/selenium/webdriver/webkitgtk/webdriver.py

  • Added download_file and get_downloadable_files methods
  • Both methods now raise NotImplementedError
  • +6/-0     
    webdriver.py
    Add NotImplementedError for download methods in WPEWebKit driver

    py/selenium/webdriver/wpewebkit/webdriver.py

  • Added download_file and get_downloadable_files methods
  • Both methods now raise NotImplementedError
  • +6/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added the C-py Python Bindings label May 20, 2025
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Message

    The NotImplementedError is raised without a descriptive message explaining why these methods are not available on local WebDrivers or suggesting alternatives.

    raise NotImplementedError

    Copy link
    Contributor

    qodo-merge-pro bot commented May 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add descriptive error messages

    Add descriptive error messages to the NotImplementedError exceptions to inform
    users that these methods are not supported for local webdrivers. This provides
    clearer guidance on why the operation failed.

    py/selenium/webdriver/chromium/webdriver.py [226-230]

     def download_file(self, *args, **kwargs):
    -    raise NotImplementedError
    +    raise NotImplementedError("The download_file method is not supported for local webdrivers")
     
     def get_downloadable_files(self, *args, **kwargs):
    -    raise NotImplementedError
    +    raise NotImplementedError("The get_downloadable_files method is not supported for local webdrivers")
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why: Adding descriptive error messages to NotImplementedError improves developer experience by clarifying why the methods are not available, but it does not affect core functionality or correctness.

    Low
    • Update

    @cgoldberg cgoldberg merged commit 4fc2582 into SeleniumHQ:trunk May 22, 2025
    16 checks passed
    @cgoldberg cgoldberg deleted the py-downloads-not-implemented branch May 22, 2025 17:44
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🐛 Bug]: [py] Downloading files doesn't work for local drivers
    2 participants