Skip to content

Bring back the transactional server - #191

Merged
rosston merged 7 commits into
mainfrom
undo-breaking-changes
Sep 17, 2026
Merged

rosston merged 7 commits into
mainfrom
undo-breaking-changes

Conversation

@rosston

@rosston rosston commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

Summary

0.8.0 removed CYPRESS_RAILS_TRANSACTIONAL_SERVER and told apps to reset their own test data with database_cleaner. That was a mistake. The setup the README recommended doesn't reliably reset data under cypress-rails' multi-threaded Puma server, so data can leak from one test into the next.

This PR restores the transactional server, built this time on a stable Rails contract instead of copied Rails internals, and makes the reset endpoint finish its work before responding. For apps on Rails 7.1 or newer, the public API is back to exactly what 0.7.1 had.

This is stacked on #187, #188, and #190.

Why 0.8.0's reasoning was wrong

The commit that removed the transactional server gave three reasons:

It breaks on Rails 7.2+ (NoMethodError on ConnectionPool#lock_thread=) and
chasing further Rails internals changes isn't sustainable - cypress-rails has
no special knowledge of an app's database, and dedicated gems like
database_cleaner already do this well and stay current with Rails.

Each of those turned out to be wrong, or right about the problem but wrong about the fix.

"cypress-rails has no special knowledge of an app's database"

cypress-rails doesn't need to know anything about an app's schema or data, but it does own the thing that decides how the database gets used: the server. cypress-rails boots the app under Puma with multiple threads, and each thread can use its own database connection. Whether test data resets cleanly depends on how those connections share, or don't share, a transaction. That's a consequence of running the server, so it's cypress-rails' concern, not something each app can reasonably solve on its own.

"database_cleaner already does this well"

database_cleaner is built for tests that run in the same thread as the code they exercise. Its :transaction strategy opens a transaction on one database connection and rolls it back later. Under cypress-rails' server, a request handled by another Puma thread uses a different connection, outside that transaction. Its writes commit for real and survive the reset, and data written inside the transaction is invisible to it. On SQLite, the open transaction also makes other connections' writes fail with "database is locked".

The README's example made this worse in a way that's easy to miss. It set DatabaseCleaner.strategy = :transaction at the top of an initializer. If ActiveRecord hasn't loaded yet when that line runs, as in the example app, the assignment is silently ignored, and database_cleaner falls back to its default, which is also :transaction. So the example appeared to work as written, but anyone who changed it to a strategy that does work in this setup, like :truncation, could silently end up with :transaction anyway.

The strategies that do work, truncation and deletion, wipe all data on every reset, so every app has to rebuild its fixtures and seed data in the hook, with its own tradeoffs. Recommending database_cleaner handed a hard concurrency problem to every user, along with guidance that didn't work.

"chasing Rails internals isn't sustainable"

This part was true: the old implementation copied connection-pinning code from Rails, and Rails 7.2 changed it (lock_thread= became pin_connection!). But the fix was to stop copying internals, not to drop the feature.

Rails already has a stable contract for exactly this problem. ActiveRecord::TestFixtures wraps each test in a transaction shared across threads, and it's driven through its before_setup and after_teardown hooks. Minitest's ActiveSupport::TestCase calls those hooks, and rspec-rails has called them around every example since at least 3.5.0 (2016). They've been essentially unchanged from Rails 6.0 through Rails' main branch, and Rails picks the right pinning mechanism for each version behind them. Rails can't change that contract without breaking both test frameworks, so it's a far safer foundation than the code 0.7.1 copied.

Why it looked fine at first

After 0.8.0, CI on main stayed green, which made the change look safe. The reason was luck: Rails' connection pool hands out the most recently returned connection first, so as long as the example app's requests never overlapped, each one reused the connection holding database_cleaner's transaction.

Changing request timing was enough to break that. After the example app moved to shakapacker (#187), the Rails 7.1 jobs started failing intermittently with "database is locked". After it moved to Postgres (#188), the real symptom showed up: an edit from one test survived into the next. Locally, it failed 3 of 5 and 6 of 8 runs on Rails 7.1. Apps with more concurrent requests, such as XHR on page load, Turbo frames, or ActiveStorage images, are far more likely to hit this than the example app was.

What this PR does

  • Restores CYPRESS_RAILS_TRANSACTIONAL_SERVER (on by default), CypressRails::Config#transactional_server, the after_transaction_start hook, and the after_state_reset hook name, all exactly as they were in 0.7.1.
  • Rebuilds ManagesTransactions on ActiveRecord::TestFixtures' lifecycle hooks. A dedicated thread owns the transaction, because on Rails 7.1 a Puma thread's end-of-request cleanup would hand the shared connection back to the pool.
  • Makes /cypress_rails_reset_state finish the reset before responding, and return a 500 with the error when a hook raises. It used to defer the reset to whichever request came next.
  • Requires Rails 7.1 or newer, which is what CI covers.
  • Updates the README, including when and how to turn the transactional server off, and the database_cleaner pitfalls above.
  • Adds CI jobs that run the example app with the transactional server off, so that documented setup can't silently break.

Why the reset endpoint is synchronous now

Deferring the reset to the next request isn't what caused the flaky resets. Bringing back the shared transaction fixes that. But the deferred design had problems of its own:

  • A failing hook cascaded. The failure landed on whatever request came next, and since the "reset needed" flag stayed set, every later request re-ran the failing hook. Now the cy.request that asked for the reset fails, with the exception message, and the next reset starts clean.
  • Concurrent requests could race the reset. Every request checked an unsynchronized flag, and nothing made other requests wait while one of them performed the reset. Now the reset runs in its own request, one at a time, and Cypress waits for it before the test continues.
  • Reset work ran inside unrelated app requests, affecting their logging, timing, and error reporting.

The deferred design existed so the old rollback could run on a request thread where the pinned connection lived. With a dedicated thread owning the transaction, that reason is gone.

Compatibility

Relative to 0.7.1, the only breaking change is the Rails 7.1 minimum. The reset endpoint's behavior change is compatible with the documented usage. Relative to 0.8.0, this reverses 0.8.0's breaking changes, which is breaking for apps that adopted it. For example, after_reset_requested is gone, so an initializer still calling it raises at boot. The full comparison is in docs/changes-in-0.9.0.md.

How this was verified

  • Unit tests cover ManagesTransactions against a real SQLite database on ActiveRecord 7.1, 7.2, 8.0, and 8.1: other threads see the shared transaction, rollback undoes every thread's writes, and repeated resets stay pinned. They also cover reset ordering and concurrent resets. I deliberately broke the code three ways (removing the reset lock, getting the TestFixtures include order wrong, and skipping a connection release), and each time the test meant to catch it failed.
  • The example app passed locally on Postgres across Ruby 3.2, 3.3, 3.4, and 4.0 with Rails 7.1, 7.2, and 8.0, with the transactional server on and off, including repeated runs on Rails 7.1, where it used to fail most often. It also passed on SQLite on Rails 7.1 through 8.1 in both modes.
  • Failure behavior was checked with temporary specs: a raising reset hook returns a 500 and the next reset recovers, a failing test's writes are still rolled back at exit, and before_server_stop still runs even if that rollback raises.
  • CI passed on two consecutive runs.

Related issues

@rosston
rosston changed the base branch from postgres-example to rails-8.1 September 17, 2026 17:35
@rosston rosston self-assigned this Sep 17, 2026
@rosston
rosston added this pull request to stack #189 September 17, 2026 17:35
@rosston
rosston force-pushed the undo-breaking-changes branch from 0777245 to 84922c5 Compare September 17, 2026 18:01
@rosston
rosston force-pushed the undo-breaking-changes branch from 84922c5 to f583be7 Compare September 17, 2026 18:10
Base automatically changed from rails-8.1 to main September 17, 2026 18:12
0.8.0 removed CYPRESS_RAILS_TRANSACTIONAL_SERVER and told apps to reset
state with database_cleaner's :transaction strategy. That can't work
under cypress-rails' multi-threaded Puma server: the strategy opens a
transaction on one pooled connection, so requests served on other
connections commit for real and survive the reset. The example app went
flaky as soon as request timing changed.

Restore the transactional server (on by default) and the
after_transaction_start hook, and undo the after_state_reset rename.
This time ManagesTransactions drives ActiveRecord::TestFixtures through
its before_setup/after_teardown hooks, which Minitest and rspec-rails
both depend on, instead of copying Rails' connection-pinning internals,
which is what broke on Rails 7.2. A dedicated thread owns the
transaction, because on Rails 7.1 a Puma thread's end-of-request cleanup
would check the pinned connection back in.

/cypress_rails_reset_state now finishes the reset before responding,
runs one reset at a time, and returns a 500 with the error when a hook
raises. Previously it flagged a reset for whichever request happened to
come next.

The example app exercises both modes. With the transactional server
off, it truncates and reloads its fixtures itself.
The transactional server relies on ActiveRecord::TestFixtures'
before_setup/after_teardown hooks, which don't exist in Rails 5.2, and
CI only covers Rails 7.1 and newer.
Also cover turning the transactional server off, including the
database_cleaner pitfalls that made 0.8.0's recommended setup
unreliable.
The extra jobs with the transactional server turned off keep the
documented non-transactional reset pattern from silently breaking.
The unit tests now install sqlite3, whose Linux builds are only
published for platforms like x86_64-linux-gnu. The RubyGems and Bundler
that ship with Ruby 3.0 predate that platform naming, so Bundler tried
to compile sqlite3 from source instead, and that build failed.
When the Ruby 3.0 job failed, fail-fast cancelled the rest, so there was
no way to tell whether they would have passed. example-app-tests already
works this way.
This will be the in depth doc that the shorter CHANGELOG entry will
point to.
@rosston
rosston force-pushed the undo-breaking-changes branch from f583be7 to 6504db1 Compare September 17, 2026 18:12
@rosston
rosston merged commit 5727f69 into main Sep 17, 2026
69 checks passed
@rosston
rosston deleted the undo-breaking-changes branch September 17, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant