Report the errors swallowed while computing a cache digest

docs/CHANGELOG.md

@@ -10,6 +10,10 @@ nav_order: 6
 
 ## main
 
+* Report the errors swallowed while computing a component's cache digest, instead of degrading to an untracked component with no indication that anything went wrong. Digest errors are now raised in local environments and logged at `warn` elsewhere, configurable with `config.view_component.raise_on_cache_digest_errors`.
+
+    *Erik Axel Nielsen*
+
 ## 4.15.0
 
 * Add experimental caching support, opt-in per component via `include ViewComponent::ExperimentallyCacheable`.

docs/api.md

@@ -294,6 +294,19 @@ A custom default layout used for the previews index page and individual previews
 
     config.view_component.previews.default_layout = "preview_layout"
 
+### `.raise_on_cache_digest_errors`
+
+Whether to raise when computing a component's cache digest fails.
+
+Digest failures are otherwise swallowed, since a stale fragment is
+preferable to a failed render, and reported to the log at `warn`. That
+trade is wrong in development and test, where an untracked component
+looks exactly like a component that was never cached.
+
+Defaults to `true` in local environments and `false` elsewhere:
+
+    config.view_component.raise_on_cache_digest_errors = false
+
 ## ViewComponent::TestHelpers
 
 ### `#render_in_view_context(...)`

docs/guide/caching.md

@@ -189,6 +189,22 @@ The same works in a template, where the branch is often the more natural place f
 
 Declared components must include `ViewComponent::ExperimentallyCacheable` themselves, since a component that hasn't opted in has no digest to depend on.
 
+## When a digest can't be computed
+
+Computing a digest touches the autoloader, the filesystem, and Action View's dependency trackers, any of which can fail. In production those failures are swallowed: a component that can't be digested is left untracked, which is exactly the behavior it had before opting in, and a stale fragment beats a failed render.
+
+That trade is wrong while developing, where an untracked component is indistinguishable from a component that was never cached in the first place. Digest failures are therefore raised in local environments and logged at `warn` everywhere else:
+
+```console
+[ViewComponent] Ignored an error while resolving PostComponent: NoMethodError: ...
+```
+
+To swallow them locally too, or to raise them in production:
+
+```ruby
+config.view_component.raise_on_cache_digest_errors = false
+```
+
 ## Caveats
 
 **Self-caching components can't take content from their callers.** Besides a block, this covers `with_content` and slots set by the caller:

lib/view_component/cache_digest.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "active_support/dependencies/autoload"
+require "active_support/log_subscriber"
 require "action_view/digestor"
 require "action_view/render_parser"
 
@@ -146,8 +147,8 @@ def partial_paths_in(source, name)
         RENDER_PARSER.new(name, source).render_calls.uniq.select do |path|
           source.include?(path) || source.include?(path.sub(%r{(\A|/)_}, '\1'))
         end
-      rescue
-        # Never let digest computation break rendering.
+      rescue => error
+        handle_error(error, "scanning #{name} for rendered partials")
         []
       end
 
@@ -221,6 +222,30 @@ def install!
         end
       end
 
+      # Report an exception the digest machinery swallowed.
+      #
+      # Every rescue here degrades to "this component has no dependencies",
+      # which is indistinguishable from a component that was never cached: the
+      # digest stops changing and the application serves stale HTML. A
+      # misconfiguration, an autoload failure, or a raising `inherited` hook is
+      # therefore completely invisible.
+      #
+      # In production a stale fragment beats a failed render, so the exception
+      # is only logged. Locally the trade goes the other way, so it's re-raised
+      # by default; see `config.view_component.raise_on_cache_digest_errors`.
+      #
+      # @private
+      def handle_error(error, context)
+        raise error if ViewComponent::Base.config.raise_on_cache_digest_errors
+
+        logger&.warn { "[ViewComponent] Ignored an error while #{context}: #{error.class}: #{error.message}" }
+      end
+
+      # @return [ActiveSupport::BroadcastLogger, Logger, nil] nil outside a Rails application.
+      def logger
+        ActiveSupport::LogSubscriber.logger
+      end
+
       private
 
       # Resolve a constant name to a component that opted into caching.
@@ -234,8 +259,8 @@ def constantize_component(constant_name)
         return unless component.respond_to?(:__vc_cacheable?) && component.__vc_cacheable?
 
         component
-      rescue
-        # Never let digest computation break rendering.
+      rescue => error
+        handle_error(error, "resolving #{constant_name}")
         nil
       end
     end

lib/view_component/cache_digest/dependency_tracking.rb

@@ -27,10 +27,10 @@ def find_dependencies(name, template, view_paths = nil)
         end
 
         dependencies + CacheDigest.dependencies_in(template)
-      rescue
-        # A broken digest is preferable to a broken render. Falling back to the
-        # dependencies Rails found on its own means the component simply isn't
-        # tracked, which is the pre-existing behavior.
+      rescue => error
+        # Falling back to the dependencies Rails found on its own means the
+        # component simply isn't tracked, which is the pre-existing behavior.
+        CacheDigest.handle_error(error, "tracking component dependencies in #{name}")
         super
       end
 

lib/view_component/cache_digest/resolver.rb

@@ -30,10 +30,11 @@ def find_templates(name, prefix, partial, details, locals = [])
         return [] unless component
 
         [build_template(component, virtual_path, details)]
-      rescue
-        # Never let digest resolution break rendering. Returning no template
-        # makes the Digestor treat this as a missing node, which degrades to
-        # the behavior components have without this feature.
+      rescue => error
+        # Returning no template makes the Digestor treat this as a missing
+        # node, which degrades to the behavior components have without this
+        # feature.
+        CacheDigest.handle_error(error, "building the digest template for #{virtual_path || name}")
         []
       end
 

lib/view_component/config.rb

@@ -14,7 +14,8 @@ def defaults
         ActiveSupport::OrderedOptions.new.merge!({
           generate: default_generate_options,
           previews: default_previews_options,
-          instrumentation_enabled: false
+          instrumentation_enabled: false,
+          raise_on_cache_digest_errors: default_raise_on_cache_digest_errors
         })
       end
 
@@ -131,6 +132,20 @@ def defaults
       # Whether ActiveSupport notifications are enabled.
       # Defaults to `false`.
 
+      # @!attribute raise_on_cache_digest_errors
+      #
+      # @return [Boolean]
+      # Whether to raise when computing a component's cache digest fails.
+      #
+      # Digest failures are otherwise swallowed, since a stale fragment is
+      # preferable to a failed render, and reported to the log at `warn`. That
+      # trade is wrong in development and test, where an untracked component
+      # looks exactly like a component that was never cached.
+      #
+      # Defaults to `true` in local environments and `false` elsewhere:
+      #
+      #     config.view_component.raise_on_cache_digest_errors = false
+
       def default_preview_paths
         (default_rails_preview_paths + default_rails_engines_preview_paths).uniq
       end
@@ -155,6 +170,10 @@ def registered_rails_engines_with_previews
         end
       end
 
+      def default_raise_on_cache_digest_errors
+        defined?(Rails.env) && Rails.env.local?
+      end
+
       def default_generate_options
         options = ActiveSupport::OrderedOptions.new(false)
         options.preview_path = ""

lib/view_component/engine.rb

@@ -15,6 +15,7 @@ class Engine < Rails::Engine # :nodoc:
         options[config_option] ||= ViewComponent::Base.public_send(config_option)
       end
       options.instrumentation_enabled = false if options.instrumentation_enabled.nil?
+      options.raise_on_cache_digest_errors = Rails.env.local? if options.raise_on_cache_digest_errors.nil?
       options.previews.enabled = (Rails.env.development? || Rails.env.test?) if options.previews.enabled.nil?
 
       if options.previews.enabled

test/sandbox/test/config_test.rb

@@ -13,6 +13,7 @@ def test_defaults_are_correct
       assert_equal @config.previews.controller, "ViewComponentsController"
       assert_equal @config.previews.route, "/rails/view_components"
       assert_equal @config.instrumentation_enabled, false
+      assert_equal @config.raise_on_cache_digest_errors, Rails.env.local?
       assert_equal @config.previews.paths, ["#{Rails.root}/test/components/previews"]
     end
 

test/sandbox/test/experimentally_cacheable_test.rb

@@ -204,8 +204,22 @@ def test_partial_paths_are_not_extracted_from_sources_without_render
   end
 
   def test_partial_path_extraction_swallows_parser_errors
+    swallowing_digest_errors do |log|
+      ViewComponent::CacheDigest::RENDER_PARSER.stub(:new, ->(*) { raise "boom" }) do
+        assert_empty ViewComponent::CacheDigest.partial_paths_in("render \"a/b\"", "a/b")
+      end
+
+      assert_match "Ignored an error while scanning a/b for rendered partials: RuntimeError: boom", log.string
+    end
+  end
+
+  def test_partial_path_extraction_raises_parser_errors_locally
     ViewComponent::CacheDigest::RENDER_PARSER.stub(:new, ->(*) { raise "boom" }) do
-      assert_empty ViewComponent::CacheDigest.partial_paths_in("render \"a/b\"", "a/b")
+      error = assert_raises(RuntimeError) do
+        ViewComponent::CacheDigest.partial_paths_in("render \"a/b\"", "a/b")
+      end
+
+      assert_equal "boom", error.message
     end
   end
 
@@ -437,32 +451,74 @@ def test_resolver_is_identified_by_class
   def test_resolver_returns_no_template_when_synthesis_fails
     resolver = ViewComponent::CacheDigest::Resolver.instance
 
+    swallowing_digest_errors do |log|
+      ViewComponent::CacheDigest.stub(:component_for, ->(_) { raise "boom" }) do
+        assert_empty resolver.find_templates("cacheable_component", "view_component/cache_digest", true, {})
+      end
+
+      assert_match(
+        "Ignored an error while building the digest template for " \
+          "view_component/cache_digest/cacheable_component: RuntimeError: boom",
+        log.string
+      )
+    end
+  end
+
+  def test_resolver_raises_when_synthesis_fails_locally
+    resolver = ViewComponent::CacheDigest::Resolver.instance
+
     ViewComponent::CacheDigest.stub(:component_for, ->(_) { raise "boom" }) do
-      assert_empty resolver.find_templates("cacheable_component", "view_component/cache_digest", true, {})
+      assert_raises(RuntimeError) do
+        resolver.find_templates("cacheable_component", "view_component/cache_digest", true, {})
+      end
     end
   end
 
   def test_dependency_tracking_falls_back_when_scanning_fails
     template = build_template("<%= render CacheableComponent.new(title: 'a') %>")
 
+    swallowing_digest_errors do |log|
+      ViewComponent::CacheDigest.stub(:dependencies_in, ->(_) { raise "boom" }) do
+        refute_includes(
+          ActionView::DependencyTracker.find_dependencies("some/template", template, []),
+          "view_component/cache_digest/cacheable_component"
+        )
+      end
+
+      assert_match "Ignored an error while tracking component dependencies in some/template", log.string
+    end
+  end
+
+  def test_dependency_tracking_raises_when_scanning_fails_locally
+    template = build_template("<%= render CacheableComponent.new(title: 'a') %>")
+
     ViewComponent::CacheDigest.stub(:dependencies_in, ->(_) { raise "boom" }) do
-      refute_includes(
-        ActionView::DependencyTracker.find_dependencies("some/template", template, []),
-        "view_component/cache_digest/cacheable_component"
-      )
+      assert_raises(RuntimeError) { ActionView::DependencyTracker.find_dependencies("some/template", template, []) }
     end
   end
 
   def test_constantizing_swallows_unexpected_errors
-    Object.const_set(:BoomComponent, Class.new do
-      def self.__vc_cacheable?
-        raise ArgumentError
+    with_boom_component do
+      swallowing_digest_errors do |log|
+        assert_nil ViewComponent::CacheDigest.send(:constantize_component, "BoomComponent")
+
+        assert_match "Ignored an error while resolving BoomComponent: ArgumentError", log.string
       end
-    end)
+    end
+  end
 
-    assert_nil ViewComponent::CacheDigest.send(:constantize_component, "BoomComponent")
-  ensure
-    Object.send(:remove_const, :BoomComponent)
+  def test_constantizing_raises_unexpected_errors_locally
+    with_boom_component do
+      assert_raises(ArgumentError) { ViewComponent::CacheDigest.send(:constantize_component, "BoomComponent") }
+    end
+  end
+
+  def test_digest_errors_are_swallowed_without_a_logger
+    without_raising_digest_errors do
+      ViewComponent::CacheDigest.stub(:logger, nil) do
+        assert_nil ViewComponent::CacheDigest.handle_error(RuntimeError.new("boom"), "digesting")
+      end
+    end
   end
 
   def test_install_is_idempotent
@@ -487,6 +543,37 @@ def recompile(component)
     component.__vc_compile(force: true)
   end
 
+  # The digest machinery raises in local environments, and the sandbox runs as
+  # `test`, so the swallow-and-report path has to be opted into explicitly.
+  def without_raising_digest_errors
+    previous = ViewComponent::Base.config.raise_on_cache_digest_errors
+    ViewComponent::Base.config.raise_on_cache_digest_errors = false
+
+    yield
+  ensure
+    ViewComponent::Base.config.raise_on_cache_digest_errors = previous
+  end
+
+  def swallowing_digest_errors
+    log = StringIO.new
+
+    without_raising_digest_errors do
+      ViewComponent::CacheDigest.stub(:logger, ActiveSupport::Logger.new(log)) { yield log }
+    end
+  end
+
+  def with_boom_component
+    Object.const_set(:BoomComponent, Class.new do
+      def self.__vc_cacheable?
+        raise ArgumentError
+      end
+    end)
+
+    yield
+  ensure
+    Object.send(:remove_const, :BoomComponent)
+  end
+
   def build_template(source)
     ActionView::Template.new(
       source,