WW-5578 Don't catch and swallow ConfigurationException in InterceptorBuilder.#1377
WW-5578 Don't catch and swallow ConfigurationException in InterceptorBuilder.#1377lukaszlenart merged 1 commit into
Conversation
…Builder. This can mask configuration errors and result in a non-working application. Extend FetchMetadataInterceptorTest; in order to correctly load all of the interceptors from struts-testing.xml, it needs additional configuration providers.
There was a problem hiding this comment.
Pull Request Overview
This PR removes exception swallowing in the InterceptorBuilder class to prevent configuration errors from being silently ignored. When an interceptor fails to load due to a missing class or configuration issue, the error should propagate rather than being logged and hidden, ensuring developers are immediately aware of configuration problems.
Key changes:
- Removed try-catch block in
InterceptorBuilder.constructInterceptorReferencethat was catching and loggingConfigurationException - Updated test to provide additional configuration providers needed for proper initialization
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
core/src/main/java/org/apache/struts2/config/providers/InterceptorBuilder.java |
Removed exception handling that was masking interceptor configuration failures |
core/src/test/java/org/apache/struts2/interceptor/FetchMetadataInterceptorTest.java |
Updated test setup to include required configuration providers |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Interceptor inter = objectFactory.buildInterceptor(config, refParams); | ||
| result.add(new InterceptorMapping(refName, inter, refParams)); |
There was a problem hiding this comment.
With the removal of exception handling, this method now throws ConfigurationException without catching it. The method signature should be updated to document this change by adding throws ConfigurationException to the method declaration, or the Javadoc should be updated to indicate that this exception can be thrown.
|
Good call, interceptors can be used to enforce auth/security and so failing fast is indeed desirable. If users have a genuine need for an optionally loaded interceptor, then we should implement some mechanism to explicitly declare that. |
WW-5578
This can mask configuration errors and result in a non-working application.