Merge pull request #24629 from opf/fix/selenium-rack-session-login-flake

Retry rack session login on stale inspector node

Author: myabc

spec/support/authentication_helpers.rb

@@ -29,6 +29,7 @@
 #++
 
 require "rack_session_access/capybara"
+require "retriable"
 
 module AuthenticationHelpers
   def self.included(base)
@@ -47,7 +48,7 @@ def login_as(user)
           session_value_for(user).to_s
         )
       else
-        page.set_rack_session(session_value_for(user))
+        set_rack_session_with_retry(session_value_for(user))
       end
     end
 
@@ -86,8 +87,32 @@ def logout
     allow(RequestStore).to receive(:[]).and_call_original
   end
 
+  # Chrome reports a node that went stale mid-command as an unhandled
+  # inspector error, which selenium-webdriver surfaces as UnknownError
+  # rather than StaleElementReferenceError, so Capybara's synchronize
+  # does not retry it.
+  STALE_PAGE_ERRORS = {
+    Selenium::WebDriver::Error::StaleElementReferenceError => nil,
+    Selenium::WebDriver::Error::UnknownError => /Node with given id does not belong to the document/
+  }.freeze
+  private_constant :STALE_PAGE_ERRORS
+
   private
 
+  # set_rack_session drives a real form on /rack_session: it submits the
+  # form and immediately reads the page text to confirm the update. Under
+  # Selenium that read can resolve the body node just before the submit's
+  # navigation lands and then read it just after, hitting a stale node.
+  # Writing the session data again is idempotent, so the whole call is
+  # retried a bounded number of times. Retriable is called directly rather
+  # than through retry_block because the latter no-ops under
+  # RSPEC_RETRY_RETRY_COUNT=0, which is where this race bites most.
+  def set_rack_session_with_retry(session_value)
+    Retriable.retriable(tries: 3, on: STALE_PAGE_ERRORS) do
+      page.set_rack_session(session_value)
+    end
+  end
+
   def js_enabled?
     RSpec.current_example.metadata[:js]
   end