Uploaded image for project: 'Cache Modules'
  1. Cache Modules
  2. MGNLCACHE-48

Cache filter should rethrow exceptions instead of wrapping them (at least when unrelated)

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Neutral Neutral
    • 5.3
    • None
    • None
    • None

      The CacheFilter currently does catch (Throwable th) {, essentially, as far as my understanding goes, to catch LockTimeoutException without being ehcache-specific. There's an if() block right there which knows what the problem is. When that if block isn't evaluated, we still throw new RuntimeException(t), which is confusing when reading logs. Even if cache is bypassed, and the underlying page/filter/whatever throws some exception, it will appear as coming from the CacheFilter.
      I'd suggest simply throw th (unless perhaps for those cases covered by the if-block) - and yeah, non-runtime exceptions will need to be wrapped...but then again, they're the only ones we should catch here.

      Another approach would be to check with the i.m.m.c.Cache implementation which Exception the filter should handle.

      Lastly, we should probably document the fact that using the cache for other purposes than caching pages with the CacheFilter should probably imply handling those exceptions as well.

      Also see http://wiki.magnolia-cms.com/display/DEV/Concept+-+Cache+Improvements#Concept-CacheImprovements-Removeboilerplateandhidelockingconcerns for a related suggestion.

        Acceptance criteria

              rkovarik Roman Kovařík
              gjoseph Magnolia International
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved:

                  Bug DoR
                  Task DoD