Re: SolrException.ErrorCode

[Chris Hostetter <ho...@fucit.org>]

: Any reason why ErrorCode.code isn't public final?  It seems weird that we have

no idea (to clarify, it was "final" and grant recently made it "public final")

: public void assertQEx(String message, SolrQueryRequest req, int code ) {
: 
: instead of
: public void assertQEx(String message, SolrQueryRequest req, SolrException.ErrorCode ) {

I think because that assert code predates the ErrorCode class (once upon a 
time Solr was also using numeric codes like "0", "-1" for some internal 
errors) and when the ErrorCode class got added, no one updated the 
AbstractTestCase to make it easy to use.

: Also, if it is public final, there really isn't any harm in exposing it 
: elsewhere.  However, it does seem weird that we have these codes, but 
: they aren't logged either, AFAICT.

They do get logged, but indirectly -- since ErrorCode is an inner class of 
SolrException the SolrException class has always had access to it even 
though it wasn't public, so it inspects it when it's used to construct a 
SolrException, and then SolrException has a "public int code()" method for 
returning it.  that method is used by the SolrDispatchFilter to 
set the response code, which the Servlet Container uses for logging.

: Finally, do we have a plan for how they should be used?  Can we add new 
: values to them?  Do we document anywhere what they mean?

The enum names seem fairly self documenting.  The ErrorCode enum came 
about in SOLR-249, after there had been a push to move away from
inconsistent values depending on where the SolrException was 
"expected" to wind up (0,-1 wsa getting used by some internal exception, 
and code that the UpdateServlet dealt with, while SOlrExceptions that were 
getting thrown to the user used HTTP status codes.

I think we should try to stick with using a subset of the HTTP codes so we 
can always be safe "leaking" them to outside clients (via the Servlet 
Containers default error page mechanism).  If we feel like we need finer 
grained control then that in some cases we could consider adding a 
"sub-code" to the ErrorCode enum -- but that sounds like it would 
smell fishy.  If we find ourselves wanting more detial like that, we 
should probably subclass SolrException instead of adding more codes (we 
should probably be subclassing SolrException a lot more anyway)




-Hoss