fix(rack-attack): surface a missing rack-attack gem instead of failing silently - #150
Merged
Merged
Conversation
…g silently Turning on enable_rack_attack_tracking without the rack-attack gem installed was safe but undiagnosable. The subscriber is gated on defined?(Rack::Attack), so it silently never registered, and the Rate Limits page rendered the same "No Rate Limit Events Found" empty state it shows when the gem IS installed and simply no rule has matched yet. That is the same ambiguity that made #143 hard to report: a permanently blank page with no way to tell "not wired up" from "nothing to show". The sibling flags (enable_actioncable_tracking, enable_activestorage_tracking) already warn when their prerequisite is missing; this one did not. Now: - Configuration#validate! logs a warning when tracking is on but ::Rack::Attack is undefined. It warns rather than auto-disabling, because validation can run before the host's Rack::Attack initializer — a missing constant at validate! time does not prove it is missing at after_initialize, when the subscriber actually registers. Auto-disabling would break that ordering. - The empty state distinguishes the two cases, telling the user the gem is not loaded rather than reporting zero events. Behaviour is otherwise unchanged: still opt-in, still a no-op without the gem, still zero runtime cost when disabled. Also corrects docs that predated #143 and still claimed breadcrumbs were a required dependency and that tracking auto-disables without them. Neither has been true since events moved to their own table. Covers docs/FEATURES.md, docs/guides/CONFIGURATION.md, their Jekyll mirrors, and the settings-page description that still called this breadcrumb storage. Test plan: - RSpec: 3640 examples, 0 failures (+4) - RuboCop clean on all changed Ruby files - Both branches of the empty state covered; the gem-present case uses stub_const since rack-attack is not a dependency and is absent in the suite Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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
Enabling
enable_rack_attack_trackingwithout therack-attackgem installed was safe but undiagnosable. This makes the failure visible in two places, and corrects docs that predated #143.Why
The subscriber is gated on
defined?(Rack::Attack)(engine.rb:84), so with no gem it silently never registers. The Rate Limits page then rendered the same "No Rate Limit Events Found" empty state it shows when the gem is installed and simply no rule has matched yet.Three states rendered identically:
rack-attacknot installed → subscriber never registeredThat ambiguity is what made #143 hard to report in the first place — a permanently blank page with no way to tell "not wired up" from "nothing to show". The sibling flags (
enable_actioncable_tracking,enable_activestorage_tracking) already warn when their prerequisite is missing; this one did not.How
Startup warning —
Configuration#validate!logs when tracking is on but::Rack::Attackis undefined.It warns rather than auto-disables, deliberately. Validation can run before the host's Rack::Attack initializer, so a missing constant at
validate!time does not prove it will still be missing atafter_initialize, when the subscriber actually registers. Auto-disabling would break that ordering — which is why this diverges from the auto-disable pattern used by the ActionCable/ActiveStorage flags.Empty state — the page now says the gem is not loaded instead of reporting zero events. The controller sets
@rack_attack_missingso the view isn't probing constants itself.Docs — several files still claimed breadcrumbs were a required dependency and that tracking auto-disables without them. Neither has been true since #143 moved events to their own table. Fixed in
docs/FEATURES.md,docs/guides/CONFIGURATION.md, their Jekyll mirrors, and the settings-page description that still called this breadcrumb storage.Not changed
Still opt-in, still a no-op without the gem, still zero runtime cost when disabled. No capture-path code touched.
Test plan
stub_const, sincerack-attackis not a dependency and is absent from the suite — which also means the gem-missing branch is the suite's default state🤖 Generated with Claude Code