Catch ActiveRecord::ConnectionFailed in the failsafe#308
Open
ajaynomics wants to merge 1 commit into
Open
Conversation
A terminated cache database connection raises ActiveRecord::ConnectionFailed, a subclass of ActiveRecord::StatementInvalid added in Rails 7.1. It wasn't in TRANSIENT_ACTIVE_RECORD_ERRORS, so instead of degrading to a cache miss the error propagated to the caller. Add it alongside the other transient connection errors and cover it with the failure safety behavior. Fixes: rails#307
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
ActiveRecord::ConnectionFailedtoFailsafe::TRANSIENT_ACTIVE_RECORD_ERRORSso that a terminated cache database connection degrades to a cache miss instead of propagating to the caller.Fixes #307.
Why
ActiveRecord::ConnectionFailed(added in Rails 7.1) is raised when a connection is terminated mid-query. Critically, it is a subclass ofActiveRecord::StatementInvalid, not ofActiveRecord::ConnectionNotEstablished— so even though the failsafe already rescues connection-related errors, this one slipped straight past the rescue list and surfaced to application code. Adding it alongside the existing transient connection errors restores the intended "cache is best-effort, never fatal" behavior.How it's tested
A new
SolidCacheConnectionFailedFailsafeTestreuses the sharedFailureSafetyBehaviorand mirrors the existing timeout test (the pattern moved to in #285), emulating a terminated connection by stubbing the adapter to raiseActiveRecord::ConnectionFailed. It guards the regression: cache operations drop to a miss rather than raising.The test helpers were also tidied up —
emulating_timeoutsandemulating_connection_failuresnow delegate to a sharedemulating_errors(error_class)helper to avoid the near-verbatim duplication.All test configurations pass (
TARGET_DB=sqlite bundle exec rake test): 400 runs, 0 failures, 0 errors across all 8 config matrices. RuboCop is clean.Notes
No CHANGELOG entry — the repo has never maintained a CHANGELOG file, and recent bug-fix PRs (e.g. #286, #287) add none.