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

turbo_frame_request_id safe operator in not handled correctly #613

Closed
jwoodrow opened this issue Apr 11, 2024 · 2 comments · Fixed by #616
Closed

turbo_frame_request_id safe operator in not handled correctly #613

jwoodrow opened this issue Apr 11, 2024 · 2 comments · Fixed by #616

Comments

@jwoodrow
Copy link

When rendering a layout from specs, the request value will be nil but using the safe operator followed by the [] method leads to an undefined method [] for nil

We render an axlsx in some of our specs to validate the content of the rendered file like this

ApplicationController.new.render_to_string(
  formats:  [:xlsx],
  handlers: [:axlsx],
  filename:,
  template: 'path/to/view',
  locals:
)

this ends up failing here

request&.headers["Turbo-Frame"]

def turbo_frame_request_id
  request&.headers["Turbo-Frame"]
end

I would have used request&.headers&.dig('Turbo-Frame') to avoid this.

We found this out because we tried setting up rails/mission_control-jobs which depends on turbo-rails (which we don't currently use)

@seanpdoyle
Copy link
Contributor

@jwoodrow thank you for opening this issue. Could you share a script that reproduces the behavior you're experiencing? I've shared a reproduction script below that you could modify to suit your needs. For example, you could replace the SystemTest and the HTML beneath the __END__ separator with an ActiveSupport::TestCase test that's closer to the source of your issue.

Based on what you've shared above, I'm unsure how the #[] access is raising a NoMethodError. the & operator should prevent the rest of the expression from executing when request == nil. That means the request object is present, but the #headers is returning nil. I'm not sure what could be the root cause of those circumstances.

Save this snippet to `bug.rb` then execute `ruby bug.rb`
require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails"
  gem "propshaft"
  gem "puma"
  gem "sqlite3"
  gem "turbo-rails"

  gem "capybara"
  gem "cuprite", require: "capybara/cuprite"
end

ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] = "test"

require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_view/railtie"
# require "action_mailer/railtie"
# require "active_job/railtie"
require "action_cable/engine"
# require "action_mailbox/engine"
# require "action_text/engine"
require "rails/test_unit/railtie"

class App < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f

  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  config.secret_key_base = "secret_key_base"
  config.consider_all_requests_local = true
  config.action_cable.cable = {"adapter" => "async"}
  config.turbo.draw_routes = false

  Rails.logger = config.logger = Logger.new($stdout)

  routes.append do
    root to: "application#index"
  end
end

Rails.application.initialize!

ActiveRecord::Schema.define do
  create_table :messages, force: true do |t|
    t.text :body, null: false
  end
end

class Message < ActiveRecord::Base
end

class ApplicationController < ActionController::Base
  include Rails.application.routes.url_helpers

  class_attribute :template, default: DATA.read

  def index
    render inline: template, formats: :html
  end
end

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: {js_errors: true}
end

Capybara.configure do |config|
  config.server = :puma, {Silent: true}
  config.default_normalize_ws = true
end

require "rails/test_help"

class TurboSystemTest < ApplicationSystemTestCase
  test "reproduces bug" do
    visit root_path

    assert_text "Loaded with Turbo"
  end
end

__END__
<!DOCTYPE html>
<html>
  <head>
    <%= csrf_meta_tags %>

    <script type="importmap">
      {
        "imports": {
          "@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>"
        }
      }
    </script>

    <script type="module">
      import "@hotwired/turbo-rails"

      addEventListener("turbo:load", () => document.body.innerHTML = "Loaded with Turbo")
    </script>
  </head>

  <body>Loaded without Turbo</body>
</html>

@jwoodrow
Copy link
Author

I could try to get you a reproduction script but AFAIK the &. Operator doesn't prevent the rest of the line from executing, it simply makes it so if the previous function call or value returned nil the next function call will be bypassed to return nil. That's why rubocop has a rule saying that if you start using &. You need to use it before each function call in the chain. Seeing as [] is a function call too then this situation happens.

seanpdoyle added a commit to seanpdoyle/turbo-rails that referenced this issue Apr 15, 2024
Follow-up to [hotwired#529][]
Closes [hotwired#613][]

Add test coverage to ensure that rendering outside of a request does not
raise `nil`-related `NoMethodError`.

Alongside a change to replace the `#turbo_frame_request_id` method's `&`
operator with a conditional, this commit also makes a similar change to
the `#turbo_native_app?` method, since it accesses the user agent in a
similar way.

[hotwired#529]: hotwired#529
[hotwired#613]: hotwired#613
seanpdoyle added a commit to seanpdoyle/turbo-rails that referenced this issue Apr 16, 2024
Follow-up to [hotwired#529][]
Closes [hotwired#613][]

Remove the `&` operator from the `Turbo::Frames::FrameRequest` concern,
since it isn't solving the original issues in the way that it intended.

Instead, add documentation to the `README.md` that highlights
`turbo-rails`'s compatibility with [ActionController::Renderer][].

[hotwired#529]: hotwired#529
[hotwired#613]: hotwired#613
[ActionController::Renderer]: https://api.rubyonrails.org/classes/ActionController/Renderer.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants