lukaszlenart opened a new pull request, #1864:
URL: https://github.com/apache/struts/pull/1864

   Fixes [WW-5690](https://issues.apache.org/jira/browse/WW-5690)
   
   `DefaultDispatcherErrorHandler.init()` built a FreeMarker configuration and 
loaded `/org/apache/struts2/dispatcher/error.ftl` on **every** startup — 
including production, where the problem report is never rendered. 
`handleError()` only reaches it when devMode is on; otherwise it delegates to 
the container's error page.
   
   So every Struts application today pays to initialise FreeMarker for a page 
most of them never show.
   
   ## Change
   
   `init()` now just stores the `ServletContext`. A new `protected 
getTemplate()` loads the template on first use and caches it in a `volatile` 
field.
   
   No lock: two threads racing on the very first error may both load it, which 
is harmless — FreeMarker caches templates in its own `Configuration` — and 
cheaper than locking a path taken once per application lifetime.
   
   ## One behavioural change
   
   A missing or unparsable `error.ftl` used to throw `StrutsException` from 
`init()` and fail the application at boot. It now surfaces on the first 
dev-mode error, where the existing `catch (Exception exp)` in 
`handleErrorInDevMode()` already degrades to `sendError(code, "Unable to show 
problem report: ...")`.
   
   I'd argue that's an improvement — a dev-only template shouldn't stop a 
production application starting — but it is a change in failure timing and 
worth a reviewer's eye.
   
   ## Scope
   
   Startup cost only. The `FreemarkerManager` injection, the 
`freemarker.template.Template` import and `error.ftl` itself all stay. Removing 
core's FreeMarker dependency outright is the remaining half of WW-5690, and 
part of the lean-core work tracked in 
[WW-5689](https://issues.apache.org/jira/browse/WW-5689) alongside 
[WW-5691](https://issues.apache.org/jira/browse/WW-5691) and 
[WW-5692](https://issues.apache.org/jira/browse/WW-5692).
   
   ## Tests
   
   Two added to `DefaultDispatcherErrorHandlerTest`, using a 
`FreemarkerManager` subclass that records whether `getConfiguration` was called 
(no mocking framework needed — it delegates to `super`, so the template really 
renders):
   
   - `testInitDoesNotLoadErrorTemplate` — asserts `init()` leaves FreeMarker 
untouched. **Verified failing before the change**, on the assertion, which is 
what makes it a real regression guard.
   - `testErrorTemplateLoadedOnFirstDevModeError` — asserts the deferred load 
actually happens, so the report can't silently stop rendering.
   
   The four existing tests are unchanged and still pass. Full `core` suite: 
3197 tests, 0 failures, 0 errors.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to