[rust] Set Rust version to 1.89.0 in WORKSPACE by bonigarcia · Pull Request #16368 · SeleniumHQ/selenium

bonigarcia · 2025-10-01T16:55:46Z

User description

🔗 Related Issues

💥 What does this PR do?

This PR set the Rust compiler version to 1.89.0 in WORKSPACE. This will allow us to use the latest features of the Rust language. This PR also contains some smell-fixes only possible with this version.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

Update

PR Type

Enhancement

Description

Update Rust compiler version to 1.89.0 in WORKSPACE
Refactor conditional logic using modern Rust syntax
Replace deprecated method calls with newer alternatives
Switch linker from gold to lld in build configuration

Diagram Walkthrough

flowchart LR
  A["WORKSPACE"] -- "update version" --> B["Rust 1.89.0"]
  B -- "enables" --> C["Modern Syntax"]
  C -- "refactor" --> D["Conditional Logic"]
  C -- "update" --> E["Method Calls"]
  F["Build Config"] -- "switch linker" --> G["lld"]

File Walkthrough

Relevant files

Configuration changes

WORKSPACE `Set Rust version to 1.89.0` WORKSPACE Update Rust toolchain version from default to 1.89.0	+1/-1
BUILD `Update linker configuration` common/remote-build/cc/BUILD Switch linker from gold to lld in link flags	+1/-1

Enhancement

downloads.rs `Update iterator method call` rust/src/downloads.rs Replace deprecated `last()` with `next_back()` method	+1/-1
lib.rs `Modernize conditional logic with let-chains` rust/src/lib.rs Refactor nested if-let statements using let-chains syntax Simplify conditional logic for snap path handling	+10/-11
lock.rs `Refactor lock handling with let-chains` rust/src/lock.rs Replace nested if statements with let-chains pattern Simplify lock path existence checking logic	+4/-6
main.rs `Modernize error handling syntax` rust/src/main.rs Convert nested if-let to let-chains syntax Replace ternary-like expression with if-else block Improve error handling readability	+23/-23

qodo-code-review · 2025-10-01T16:56:26Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

⚪

Panic on unwrap

Description: Calling selenium_manager.find_best_driver_from_cache().unwrap() inside error handling can
panic on None/Err, potentially turning a recoverable failure into a crash and enabling
denial-of-service via triggering this path.
main.rs [258-276]

Referred Code

    && let Some(best_driver_from_cache) =
        selenium_manager.find_best_driver_from_cache().unwrap()
{
    log.debug_or_warn(
        format!(
            "There was an error managing {} ({}); using driver found in the cache",
            selenium_manager.get_driver_name(),
            err
        ),
        selenium_manager.is_offline(),
    );
    log_driver_and_browser_path(
        log,
        &best_driver_from_cache,
        &selenium_manager.get_browser_path_or_latest_from_cache(),
        selenium_manager.get_receiver(),
    );
    flush_and_exit(OK, log, Some(err));
}

Unsafe file deletion

Description: Deleting a filesystem path from get_lock_path() without validating that it resides in an
expected directory could allow unintended file deletion if the path is manipulated
elsewhere.
lock.rs [73-77]

Referred Code

if let Some(lock) = get_lock_path()
    && lock.exists()
{
    fs::remove_file(lock).unwrap_or_default();
}

Ticket Compliance

🟡

🎫 #5678

🔴	Investigate and resolve "Error: ConnectFailure (Connection refused)" occurring after the first ChromeDriver instantiation on Ubuntu 16.04 with Chrome 65 and ChromeDriver 2.35 (Selenium 3.9.0).
	Provide a reliable fix or mitigation so subsequent ChromeDriver instances do not log the connection failure.
	Ideally include steps or code changes that address repeated instantiation behavior.

🟡

🎫 #1234

🔴	Ensure that clicking links with JavaScript in the href triggers as in Selenium 2.47.1, addressing regression seen in 2.48.x with Firefox 42.
	Provide compatibility or fix in click handling so JS href executes.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

⚪

No custom compliance provided

Follow the guide to enable custom compliance check.

Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

qodo-code-review · 2025-10-01T16:57:54Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid panicking in error handler Replace `.unwrap()` with `let Ok(Some(...))` pattern matching to safely handle the `Result` from `find_best_driver_from_cache()` and prevent a potential panic in the error handler. rust/src/main.rs [257-276] if selenium_manager.is_fallback_driver_from_cache() - && let Some(best_driver_from_cache) = - selenium_manager.find_best_driver_from_cache().unwrap() + && let Ok(Some(best_driver_from_cache)) = selenium_manager.find_best_driver_from_cache() { log.debug_or_warn( format!( "There was an error managing {} ({}); using driver found in the cache", selenium_manager.get_driver_name(), err ), selenium_manager.is_offline(), ); log_driver_and_browser_path( log, &best_driver_from_cache, &selenium_manager.get_browser_path_or_latest_from_cache(), selenium_manager.get_receiver(), ); flush_and_exit(OK, log, Some(err)); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential panic caused by `.unwrap()` within an error handling block, which would crash the program instead of handling the error gracefully.	Medium
Learned best practice	Robustly extract filename segment Avoid taking a mutable `segments` for a single `next_back()` call and guard for trailing slashes explicitly. Use `rsplit('/')` on the path string to robustly extract the last non-empty segment. rust/src/downloads.rs [55-58] -.path_segments() -.and_then(\|mut segments\| segments.next_back()) -.and_then(\|name\| if name.is_empty() { None } else { Some(name) }) -.unwrap_or("tmp.bin"); +let target_name = response + .url() + .path() + .rsplit('/') + .find(\|s\| !s.is_empty()) + .unwrap_or("tmp.bin"); Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate inputs and states early to avoid logic errors; ensure iterator usage matches expected type and handles empty paths robustly.	Low
Update

selenium-ci added C-rust Rust code is mostly Selenium Manager B-manager Selenium Manager labels Oct 1, 2025

qodo-code-review bot added the Review effort 2/5 label Oct 1, 2025

[rust] Set Rust version to 1.89.0 in WORKSPACE

78e5c61

bonigarcia force-pushed the sm_fix_rust_version branch from bed718c to 78e5c61 Compare October 1, 2025 17:38

bonigarcia merged commit 8e84f0d into trunk Oct 1, 2025
28 checks passed

bonigarcia deleted the sm_fix_rust_version branch October 1, 2025 18:26

iampopovich mentioned this pull request Oct 2, 2025

[rust] Fix for non empty driver path in webdriver manager #16365

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[rust] Set Rust version to 1.89.0 in WORKSPACE#16368

[rust] Set Rust version to 1.89.0 in WORKSPACE#16368
bonigarcia merged 1 commit intotrunkfrom
sm_fix_rust_version

bonigarcia commented Oct 1, 2025 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Oct 1, 2025 •

edited

Loading

Uh oh!

qodo-code-review bot commented Oct 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bonigarcia commented Oct 1, 2025 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-code-review bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

Uh oh!

qodo-code-review bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bonigarcia commented Oct 1, 2025 •

edited by qodo-code-review bot

Loading

qodo-code-review bot commented Oct 1, 2025 •

edited

Loading

qodo-code-review bot commented Oct 1, 2025 •

edited

Loading