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

[rb] Fix "no anonymous block parameter" in ruby 3.1 #15315

Merged

Conversation

smortex
Copy link
Contributor

@smortex smortex commented Feb 20, 2025

User description

Motivation and Context

When running on Ruby 3.1.2p20 (shipped with Debian 12), the use of anonymous blocks cause exceptions to be raised. This was fixed in later Ruby versions, but will not be backported to the version of Ruby shipped by Debian 12 nor the versions or ruby shipped with all Debian derivative (e.g. Ubuntu).

This PR by dependabot show the failure: the previously working test suite cannot start:
opus-codium/pakotoa#272
The log (reproduced bellow) shows that the CI setup does not complete successfully:

  RAILS_ENV=test bundle exec rails db:setup
  shell: /usr/bin/bash -e {0}
bin/rails aborted!
SyntaxError: /home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/selenium-webdriver-4.29.0/lib/selenium/webdriver/common/network.rb:73: no anonymous block parameter
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/selenium-webdriver-4.29.0/lib/selenium/webdriver/common.rb:105:in `<main>'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/selenium-webdriver-4.29.0/lib/selenium/webdriver.rb:29:in `<main>'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/selenium-webdriver-4.29.0/lib/selenium-webdriver.rb:20:in `<main>'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/zeitwerk-2.6.18/lib/zeitwerk/kernel.rb:34:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bundler-2.3.15/lib/bundler/runtime.rb:60:in `block (2 levels) in require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bundler-2.3.15/lib/bundler/runtime.rb:55:in `each'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bundler-2.3.15/lib/bundler/runtime.rb:55:in `block in require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bundler-2.3.15/lib/bundler/runtime.rb:44:in `each'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bundler-2.3.15/lib/bundler/runtime.rb:44:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bundler-2.3.15/lib/bundler.rb:187:in `require'
/home/runner/work/pakotoa/pakotoa/config/application.rb:9:in `<main>'
/home/runner/work/pakotoa/pakotoa/Rakefile:6:in `require_relative'
/home/runner/work/pakotoa/pakotoa/Rakefile:6:in `<main>'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/commands/rake/rake_command.rb:43:in `block in with_rake'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/commands/rake/rake_command.rb:41:in `with_rake'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/commands/rake/rake_command.rb:20:in `perform'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/command.rb:156:in `invoke_rake'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/command.rb:73:in `block in invoke'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/command.rb:149:in `with_argv'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/command.rb:69:in `invoke'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/railties-7.1.5.1/lib/rails/commands.rb:18:in `<main>'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
/home/runner/work/pakotoa/pakotoa/vendor/bundle/ruby/3.1.0/gems/bootsnap-1.18.4/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30:in `require'
bin/rails:4:in `<main>'
(See full trace by running task with --trace)
Error: Process completed with exit code 1.

A similar issue in Rails was fixed in a similar way here:
rails/rails#51476

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation. (n/a)
  • I have updated the documentation accordingly. (n/a)
  • I have added tests to cover my changes. (n/a)
  • All new and existing tests passed. some tests seems to be broken in the trunk branch, upon review of the result of GitHub actions run in my forked repository, it seems to be the same failures (activesupport-8.0.1 requires ruby version >= 3.2.0, which is incompatible with the current version, 3.1.6).

PR Type

Bug fix, Enhancement


Description

  • Fix compatibility issues with Ruby 3.1 by replacing anonymous block parameters with explicitly named ones.

  • Update rubocop configuration to enforce explicit block naming.

  • Ensure backward compatibility with Ruby versions used in Debian and its derivatives.


Changes walkthrough 📝

Relevant files
Bug fix
9 files
server.rb
Replace anonymous block parameters with explicit ones in
net_http_start.
+3/-3     
bidi.rb
Use explicit block parameters in callback methods.             
+2/-2     
log_inspector.rb
Replace anonymous block parameters in log handling methods.
+4/-4     
network.rb
Use explicit block parameters in network event handling. 
+2/-2     
has_network_interception.rb
Replace anonymous block parameters in network interception methods.
+2/-2     
network.rb
Use explicit block parameters in request and response handler methods.
+4/-4     
script.rb
Replace anonymous block parameters in script message handler methods.
+4/-4     
bridge.rb
Use explicit block parameters in command addition methods.
+2/-2     
guards.rb
Replace anonymous block parameters in guard condition methods.
+2/-2     
Enhancement
1 files
.rubocop.yml
Update rubocop configuration to enforce explicit block naming.
+3/-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.
  • Sorry, something went wrong.

    @CLAassistant
    Copy link

    CLAassistant commented Feb 20, 2025

    CLA assistant check
    All committers have signed the CLA.

    When running on Ruby 3.1.2p20 (shipped with Debian 12), the use of
    anonymous blocks cause exceptions to be raised.  This was fixed in later
    Ruby versions, but will not be backported to the version of Ruby shipped
    by Debian 12 nor the versions or ruby shipped with all Debian
    derivative.
    
    Explicitly name blocks to avoid triggering this issue with affected
    versions of Ruby.
    
    While here, also adjust the rubocop configuration to detect this issue
    and mandate the use of explicitly named blocks.
    
    Signed-off-by: Romain Tartière <romain@blogreen.org>
    @smortex smortex force-pushed the fix-no-anonymous-block-parameter-in-ruby-3.1 branch from ec01f7f to c9ce790 Compare February 20, 2025 20:36
    @smortex smortex marked this pull request as ready for review February 20, 2025 21:10
    smortex added a commit to opus-codium/pakotoa that referenced this pull request Feb 20, 2025

    Partially verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
    Skip this version as it breaks at least with Ruby 3.1 on Debian 12.
    
    A PR was opened upstream to fix the issue:
    SeleniumHQ/selenium#15315
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Proxy Connection

    The proxy connection handling in net_http_start method should be validated to ensure proper proxy authentication and connection settings are maintained after the changes

    def net_http_start(address, &block)
      http_proxy = ENV.fetch('http_proxy', nil) || ENV.fetch('HTTP_PROXY', nil)
      if http_proxy
        http_proxy = "http://#{http_proxy}" unless http_proxy.start_with?('http://')
        uri = URI.parse(http_proxy)
    
        Net::HTTP.start(address, nil, uri.host, uri.port, &block)
      else
        Net::HTTP.start(address, use_ssl: true, &block)
      end

    Copy link
    Contributor

    qodo-merge-pro bot commented Feb 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    Configure SSL for proxy connections

    The proxy port and host configuration is incomplete - if a proxy is used, the
    SSL settings should be explicitly configured to ensure secure connections work
    properly through the proxy.

    rb/lib/selenium/server.rb [131]

    -Net::HTTP.start(address, nil, uri.host, uri.port, &block)
    +Net::HTTP.start(address, nil, uri.host, uri.port, use_ssl: true, &block)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: Adding SSL configuration for proxy connections is a critical security enhancement that ensures secure communication when using a proxy server. Without explicit SSL settings, connections might be vulnerable.

    Medium
    • Update

    Sorry, something went wrong.

    @smortex smortex changed the title Fix "no anonymous block parameter" in ruby 3.1 [ruby] Fix "no anonymous block parameter" in ruby 3.1 Feb 20, 2025
    @smortex smortex changed the title [ruby] Fix "no anonymous block parameter" in ruby 3.1 [rb] Fix "no anonymous block parameter" in ruby 3.1 Feb 20, 2025
    smortex referenced this pull request Feb 21, 2025

    Partially verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
    @VietND96 VietND96 requested review from aguspe and p0deje February 21, 2025 19:26
    @VietND96
    Copy link
    Member

    VietND96 commented Feb 21, 2025

    @aguspe, @p0deje, Can you check if this requires a patch version 4.29.1 for Ruby?

    Partially verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
    @p0deje
    Copy link
    Member

    p0deje commented Feb 21, 2025

    @VietND96 Yes, I can cut a release once this is merged

    @smortex
    Copy link
    Contributor Author

    smortex commented Feb 21, 2025

    FYI, I added a comment for what seems to be the root cause of the Ruby tests failures here:
    d77c827#commitcomment-152849277

    I can add a commit to this PR or create another PR to try to address it is desired, just tell me how you prefer to proceed.

    @p0deje p0deje merged commit eb5cc61 into SeleniumHQ:trunk Feb 22, 2025
    10 of 23 checks passed
    @smortex smortex deleted the fix-no-anonymous-block-parameter-in-ruby-3.1 branch February 22, 2025 22:10
    @p0deje
    Copy link
    Member

    p0deje commented Feb 22, 2025

    @smortex should be fixed in 4.29.1

    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025

    Partially verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
    Fix "no anonymous block parameter" in ruby 3.1
    
    When running on Ruby 3.1.2p20 (shipped with Debian 12), the use of
    anonymous blocks cause exceptions to be raised.  This was fixed in later
    Ruby versions, but will not be backported to the version of Ruby shipped
    by Debian 12 nor the versions or ruby shipped with all Debian
    derivative.
    
    Explicitly name blocks to avoid triggering this issue with affected
    versions of Ruby.
    
    While here, also adjust the rubocop configuration to detect this issue
    and mandate the use of explicitly named blocks.
    
    Signed-off-by: Romain Tartière <romain@blogreen.org>
    Co-authored-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    4 participants