Repository navigation
ActiveRecord::ConnectionFailed not caught by the failsafe (terminated connection surfaces as an error) #307
Description
Summary
SolidCache::Store::Failsafe::TRANSIENT_ACTIVE_RECORD_ERRORS rescues ActiveRecord::ConnectionNotEstablished but not ActiveRecord::ConnectionFailed. When the cache database connection is terminated mid-transaction, Solid Cache raises ConnectionFailed instead of degrading to a cache miss, so the error propagates to the caller (a 500 on a web request, for example).
How we hit it
We run Solid Cache on a dedicated PostgreSQL database and set idle_in_transaction_session_timeout on that connection to bound row-lock contention on solid_cache_entries. A hot key_hash gets held by Entry.lock_and_write while the Ruby block runs, and under load the holding thread can stay idle-in-transaction long enough for PostgreSQL to terminate the backend:
PG::ConnectionBad: PQconsumeInput() FATAL: terminating connection due to idle-in-transaction timeout
The write that follows the SELECT ... FOR UPDATE inside lock_and_write then raises ActiveRecord::ConnectionFailed. That class is a subclass of ActiveRecord::StatementInvalid (added in Rails 7.1), not of ActiveRecord::ConnectionNotEstablished, so the current failsafe list does not catch it.
Reproduction
conn = SolidCache::Record.connection
conn.execute("SET idle_in_transaction_session_timeout = '300ms'")
SolidCache::Entry.lock_and_write("probe") do |_value|
sleep 1 # transaction stays idle > 300ms -> PostgreSQL kills the session
"v"
end
# => ActiveRecord::ConnectionFailed: ... terminating connection due to idle-in-transaction timeout
SolidCache::Store::Failsafe::TRANSIENT_ACTIVE_RECORD_ERRORS.any? { |k| error.is_a?(k) }
# => falseProposal
Add ActiveRecord::ConnectionFailed to TRANSIENT_ACTIVE_RECORD_ERRORS. A terminated connection is the kind of transient failure the cache should degrade on, consistent with the existing ConnectionNotEstablished entry.
TRANSIENT_ACTIVE_RECORD_ERRORS = [
ActiveRecord::AdapterTimeout,
ActiveRecord::ConnectionFailed, # added
ActiveRecord::ConnectionNotEstablished,
ActiveRecord::Deadlocked,
ActiveRecord::LockWaitTimeout,
ActiveRecord::QueryCanceled,
ActiveRecord::StatementTimeout
]ActiveRecord::ConnectionFailed exists in Rails 7.1+, which is within Solid Cache's supported range, so referencing it directly is safe.
Happy to send a PR with a test if this looks right.
Environment: solid_cache 1.0.10, Rails 8.1, PostgreSQL 17.
Activity
Catch ActiveRecord::ConnectionFailed in the failsafe
Catch ActiveRecord::ConnectionFailed in the failsafe
hiSandog commentedon Jul 14, 2026
The provided
SolidCache::Store::Failsafe::TRANSIENT_ACTIVE_RECORD_ERRORScase is concrete enough to become focused regression coverage for “ActiveRecord::ConnectionFailed not caught by the failsafe (terminated connection surfaces as an error)”. It would be useful to assert the reported failure first and the expected outcome after the fix, so later refactors keep the same contract.a-abdellatif98 commentedon Aug 15, 2026
I'd like to pick this up if nobody else is on it. @navidemad, you offered a PR back in May, are you still planning to send one? Happy to stand down if so.
Confirmed still present on
main(da70ef4). Two things that support the proposal:The gemspec already requires
activerecord >= 7.2andConnectionFailedlanded in 7.1, so it can be referenced directly with no version guard.More interesting, the error hierarchy suggests this was an oversight rather than a deliberate exclusion. On Rails 8.1:
TRANSIENT_ACTIVE_RECORD_ERRORSalready lists three of the fourQueryAbortedsubclasses.ConnectionFailedis the only one missing, and it was added in 7.1, after the original list was written. So this reads less like "we decided connection loss is not transient" and more like a class that appeared later and never got picked up.That raises a question worth settling before I write the patch. Do you want:
a. Add
ActiveRecord::ConnectionFailedto the list explicitly, as proposed above. Minimal diff, no behaviour change beyond the reported case.b. Collapse
AdapterTimeout,QueryCanceledandStatementTimeoutinto their shared parentActiveRecord::QueryAborted. This fixes the reported case and makes the list self maintaining if Active Record adds furtherQueryAbortedsubclasses, at the cost of opting in to those future classes sight unseen.I lean towards a. The explicit list is easier to audit, and silently inheriting future error classes into a failsafe path is the kind of thing that hides a real problem as a cache miss. But b is defensible if you read the current list as "everything
QueryAborted, plus deadlocks and lock waits", and I'll write whichever you prefer.For tests, I'd extend the existing
emulating_timeoutshelper intest/test_helper.rbto accept an error class, soFailureSafetyBehaviorandFailureRaisingBehaviorboth run againstConnectionFailedalongside the currentStatementTimeout. That keeps coverage in the established shape instead of bolting on a one off test.PR to follow unless @navidemad would rather take it.
navidemad commentedon Aug 15, 2026
Thanks for the offer, and sorry for the delay. I went with (a) and opened #317, so no need to pick it up.
Agreed on the reasoning: explicit list, and it lines up with Active Record's own
retryable_connection_error?, which already putsConnectionFailednext toConnectionNotEstablished. One small correction on the timeline:ConnectionFailedshipped in 7.1 (Oct 2023) and 7.0/7.1 support was dropped in 067429b before the list was written in 3e10ec4, so it was never a version guard, just an omission.I took your
emulating_timeouts(error)suggestion for the tests. Review welcome on the PR.navidemad commentedon Aug 15, 2026
Correction: I had missed #308 by @ajaynomics from June, which already has the same fix. #317 overlaps with it (I borrowed its FailureSafetyBehavior approach for the tests) and adds error handler and non-transient error coverage. Either is fine to merge.