Description
We no longer have logging jars in the webapp since SOLR-3706. Because of this we added a extra check in SolrDispatchFilter's ctor to print a nice exception if the logging jars were failing. This check was unfortunately never tests and recently broke:
The check delays initialization of the Logger instance to inside a try-catch block inside the explicit ctor. If it fails with ClassNotFound, it throws Exception.
Recently we upgraded to a newer HttpClient version. Unfortunately SolrDispatchFliter also has an implicit constructor a few lines before the main constructor:
protected final HttpClient httpClient = HttpClientUtil.createClient(new ModifiableSolrParams()); // <-- this breaks the detection private static final Charset UTF8 = StandardCharsets.UTF_8; public SolrDispatchFilter() { try { log = LoggerFactory.getLogger(SolrDispatchFilter.class); } catch (NoClassDefFoundError e) { throw new SolrException( ErrorCode.SERVER_ERROR, "Could not find necessary SLF4j logging jars. If using Jetty, the SLF4j logging jars need to go in " +"the jetty lib/ext directory. For other containers, the corresponding directory should be used. " +"For more information, see: http://wiki.apache.org/solr/SolrLogging", e); } }
The first line above HttpClientUtil.createClient(new ModifiableSolrParams()); breaks the whole thing, because it is executed before the declared constructor. The user just sees a ClassNotFoundEx at this line of code, the nice error message is hidden.
Because this is so easy to break, we should make the whole thing more safe (any maybe test it). 2 options:
- Into the webapp add a fake Servlet (not bound to anything, just loaded first) that does not use any Solr classes at all, nothing only plain java
- Alternatively add a Superclass between ServletFilter and SolrDispatchFilter (pkg-private). When the servlet container loads SolrDispatchFilter, it has in any case to first load the superclass. And this superclass does the check and throws ServletException or whatever (no Solr Exception) with the message from the current code.
I tend to the second approach, because it does not need to modify web-inf. It will also work with other Solr servlets, they must just extend this hidden class. I will provide a patch for that.
Attachments
Attachments
Issue Links
- relates to
-
SOLR-3706 Ship setup to log with log4j.
- Closed