[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: |
|
||||||||||||||||
| 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. 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. |
| Comment by Magnolia International [ 13/Jun/13 ] |
|
Wow, almost 2 years - but thanks for the comment Danilo |