Relax error restriction in initializer #35321
Merged
Conversation
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
At initialization time this can fail in all sorts of ways. Apart from ActiveRecord, there is also the rails cache that in `production` needs redis. This can raise a couple of errors if a connection is not possible, including `RuntimeError`.
ClearlyClaire
approved these changes
Jul 9, 2025
ClearlyClaire
pushed a commit
that referenced
this pull request
Jul 9, 2025
ClearlyClaire
pushed a commit
that referenced
this pull request
Jul 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #35303
In the above issue it was noted that you can no longer run
RAILS_ENV=production bin/rails assets:precompilewithout access to redis.The code already accounted for the fact, that the database might not be available, but it did not recognize that the same might be true for the cache backend.
I had initially hoped I could just add a redis error to the list, and indeed the
redisgem has its own error type for connection errors. But we use the "hiredis" driver which is a C extension that in some instances just raisesRuntimeError.I could have added
Redis::CannotConnectError,Redis::ConnectionError(what is the difference, I wonder?) andRuntimeError. But I figured that a) once you includeRuntimeErroryou are already casting a very wide net and b) there might be other errors (from AR, redis or even other places) that could be applicable here. So I decided to use the catch-all and rescue from all kinds ofStandardError.On the one hand, this is considered bad practice because it can mask all kinds of errors one would rather catch as early as possible. On the other hand, this is "only" an initializer and the method in question is also excersized elsewhere, including in tests.