[MGNLCACHE-48] Cache filter should rethrow exceptions instead of wrapping them (at least when unrelated) Created: 05/Jul/11  Updated: 21/Nov/14  Resolved: 10/Nov/14

Status: Closed
Project: Cache Modules
Component/s: None
Affects Version/s: None
Fix Version/s: 5.3

Type: Bug Priority: Neutral
Reporter: Magnolia International Assignee: Roman Kovařík
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MAGNOLIA-3595 Add an exception handler for filters. Closed
is related to MAGNOLIA-3566 Cache filter should ignore ClientAbor... Closed
is related to MAGNOLIA-5113 Generalize/centralize treatment of br... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

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.



 Comments   
Comment by Danilo Ghirardelli [ 05/Jul/11 ]

There is already a little bit of discussion in the linked issues about the exception handling.
I think most of the "problem" of cache filtering exception handling is that the filter catches everything, logs, wraps and rethrow, which is not always the desired behaviour. If the original problem is just to skip the ehcache exception you may want to try something similar to what I did for Tomcat's ClientAbort (which is defined internally in Tomcat only), in issue MAGNOLIA-3566.

Comment by Magnolia International [ 13/Jun/13 ]

Wow, almost 2 years - but thanks for the comment Danilo We're running (again) into similar issues; See MAGNOLIA-5112 and MAGNOLIA-5113

Generated at Sun Feb 11 23:51:41 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.