[py] cleanup for ruff b904 raise from by iampopovich · Pull Request #18030 · SeleniumHQ/selenium

iampopovich · 2026-09-13T03:34:08Z

🔗 Related Issues

💥 What does this PR do?

fixes leftovers in #17989 tranche 2

🔧 Implementation Notes

B904 has no autofix, and the mechanical reading of it (from err everywhere)
is wrong in five places, so each of the 17 sites was decided individually:
from err where the cause carries information, from None where it does not —
an unset ContextVar's LookupError, and the ConnectionRefusedError in
Server.start, which is the success signal (the port was free).

Two sites needed more than a from: execute_script moved the script id into
the message rather than chain a KeyError whose whole content is that id, and
clean_driver split two failures that shared one (untrue for the
AttributeError path) message.

py/pyproject.toml is last so every commit stays green under
//py:ruff-check and the series bisects. The AddonFormatError cleanup is a
separate commit placed after it: the second positional argument was a Python 2
three-argument-raise port that from e now supersedes — it was never
__traceback__, and with two args Exception.__str__ printed a memory address
into every add-on parsing failure.

🤖 AI assistance

No substantial AI assistance used
AI assisted (complete below)
- Tool(s): Claude code model Sonnet 5 High effort, /code-review built-in skill
- What was generated: semantic analysis for exeptions chaining. rebasing for applied changes in worktree and editing commit messages for them
- I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

Cleanup (formatting, renaming)
Non Breaking change (fix or feature that would cause existing functionality to change) in commit e6af7f2

BEFORE str(exc) = '("[Errno 2] No such file or directory: \'.../install.rdf\'", <traceback object at 0x78a2203c1d80>)'
AFTER  str(exc) = "[Errno 2] No such file or directory: '.../install.rdf'"

`_get_cdp_details` re-raised as WebDriverException without a cause, hiding which capabilities key (`goog:chromeOptions` or `ms:edgeOptions`) returned None, so chain the AttributeError. `execute_script` suppresses the KeyError instead -- it carried nothing the message did not already say -- but the script id it did carry is now part of the message, so a caller with several pinned scripts can tell which one is missing.

The sibling raise a few lines down already passed `from err`; this branch is reached from the same EACCES handler and should report the same cause.

`clean_driver` collapsed two unrelated failures into one message: no --driver passed (TypeError from subscripting None) and a --driver that SupportedDrivers does not know (AttributeError from getattr). The second case reported "This test requires a --driver to be specified", which is untrue. argparse `choices=drivers` currently makes the second case unreachable, but only for as long as `drivers` and SupportedDrivers stay in sync, and the merged handler gave no hint when they drift. Split the two, and name the offending driver in the message. The other two fixtures do not call getattr, so their message is accurate; they just suppress the context.

…erver Causes that carry information are chained with `from err`. Two are suppressed instead: the LookupError from a ContextVar carries nothing beyond "unset", and the ConnectionRefusedError in Server.start is the success path -- it means the port was free -- so it would only mislead in a startup-timeout traceback.

Part of SeleniumHQ#17989. All 17 findings are addressed by the preceding commits, so the carve-out can go and the rule keeps new ones from landing.

`AddonFormatError` has no `__init__`, so the second positional argument never became `__traceback__` -- it just landed in `args`, and with two args `Exception.__str__` prints the whole tuple. Every add-on parsing failure ended its traceback with a memory address: AddonFormatError: ("[Errno 2] ... install.rdf", <traceback object at 0x7f..>) The pattern dates to 983d5b2 (2014), a port of Python 2's three-argument `raise`, which was the only way to keep the original traceback back then. The preceding commit added `from e`, so `__cause__` now carries the cause properly and the argument is redundant as well as noisy. It also kept every frame of the caught exception alive for as long as the AddonFormatError was referenced. This changes `str(exc)` and `exc.args` for the two raises that passed it. The class is not re-exported from any package `__init__`, and the third raise in the same file already passed a single argument. `import sys` goes with them; nothing else in the file used it.

qodo-code-review · 2026-09-13T14:27:22Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Copilot

🔵 Needs a closer look

Add regression coverage for the intentional exception-message and chaining changes.

Pull request overview

This PR completes the Python Ruff B904 cleanup, making exception chaining deliberate and correcting Firefox add-on error formatting.

Changes:

Adds explicit exception chaining or suppression.
Improves script, CDP, service, and fixture diagnostics.
Enables B904 enforcement and fixes AddonFormatError construction.

File summaries

File	Reviewed change
`py/selenium/webdriver/remote/webdriver.py`	Updates script and CDP exception handling. [P2] Add regression coverage for message and chaining behavior.
`py/selenium/webdriver/remote/server.py`	Chains port errors and suppresses expected startup context.
`py/selenium/webdriver/firefox/firefox_profile.py`	Corrects add-on exception construction and chaining. [P2] Add coverage for the resulting error text.
`py/selenium/webdriver/common/utils.py`	Adds causes to free-port errors.
`py/selenium/webdriver/common/service.py`	Chains service startup errors.
`py/pyproject.toml`	Removes the Ruff B904 exclusion.
`py/private/cdp.py`	Adds explicit CDP exception chaining.
`py/conftest.py`	Clarifies driver validation and exception handling.

Review details

Suppressed comments (2)

py/selenium/webdriver/firefox/firefox_profile.py:295

[P2] The AddonFormatError construction is an intentional user-visible behavior change, but there is no test covering the resulting message. Add a unit test (ideally covering both this path and the XML-parse path below) that verifies str(exc) is the underlying error text rather than the old two-argument tuple representation.

            raise AddonFormatError(str(e)) from e

py/selenium/webdriver/remote/webdriver.py:611

[P2] Add a regression test for the intentional message and chaining change here. test_calling_unpinned_script_causes_error currently checks only the exception type, so it would not catch dropping script.id from the message or reintroducing the implicit KeyError context that this from None is meant to suppress.

                raise JavascriptException(f"Pinned script could not be found: {script.id}") from None

Files reviewed: 8/8 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

cgoldberg

LGTM. I like exception chaining

iampopovich · 2026-09-13T23:47:22Z

@cgoldberg thanks for your review

iampopovich added 6 commits September 13, 2026 06:15

[py] chain the OSError context when the service path is None

90422ca

The sibling raise a few lines down already passed `from err`; this branch is reached from the same EACCES handler and should report the same cause.

[py] enforce ruff B904 (raise ... from) in the Python bindings

cc69e5f

Part of SeleniumHQ#17989. All 17 findings are addressed by the preceding commits, so the carve-out can go and the rule keeps new ones from landing.

selenium-ci added the C-py Python Bindings label Sep 13, 2026

iampopovich marked this pull request as ready for review September 13, 2026 14:27

Copilot AI lite review requested due to automatic review settings September 13, 2026 14:27

Copilot started reviewing on behalf of iampopovich September 13, 2026 14:27 View session

Copilot AI reviewed Sep 13, 2026

View reviewed changes

fix: use a variable for script ID in JavascriptException message

921fc88

cgoldberg approved these changes Sep 13, 2026

View reviewed changes

iampopovich merged commit 413306e into SeleniumHQ:trunk Sep 13, 2026
33 checks passed

This was referenced Oct 1, 2026

Bump Selenium.WebDriver from 4.48.0 to 4.50.0 bunty989/WebAutomationFramework#229

Closed

Bump Selenium.WebDriver across 4 directories javiertuya/selema#1164

Open

Bump Selenium.WebDriver from 4.49.0 to 4.50.0 webdriverbidi-net/webdriverbidi-net#281

Open

Navigation Menu

Uh oh!

[py] cleanup for ruff b904 raise from - #18030

[py] cleanup for ruff b904 raise from #18030
iampopovich merged 7 commits into
SeleniumHQ:trunkfrom
iampopovich:py-ruff-b904-raise-from

iampopovich commented Sep 13, 2026

Uh oh!

qodo-code-review Bot commented Sep 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

cgoldberg left a comment

Uh oh!

iampopovich commented Sep 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

iampopovich commented Sep 13, 2026

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

🤖 AI assistance

💡 Additional Considerations

🔄 Types of changes

Uh oh!

qodo-code-review Bot commented Sep 13, 2026

Qodo reviews are paused for this user.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

cgoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

iampopovich commented Sep 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants