[ 
https://issues.apache.org/jira/browse/LUCENE-5405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13875715#comment-13875715
 ] 

Robert Muir commented on LUCENE-5405:
-------------------------------------

I dont like it being LuceneAnalysisException really: I don't see the need for a 
custom class.

I think it would just be throw new IllegalArgumentException("exception from 
analysis chain for field '" + field + "':", fieldName, originalException).

{quote}
 I think it makes life better, as I don't see much evil in exception wrapping 
like this.
{quote}

There is a lot of evil still, it changes the class type of the exception (by 
wrapping it), by nesting, it makes exceptions harder to digest for the client 
(lots of lucene users use the same Analyzer for every field, so the field is 
just not interesting).

Remember this is *all about the buggy analyzer case*. Its not like we are 
supporting a real use case here.

So that's why i said, i'm not sure about this whole idea on SOLR-5623. I'm just 
not sure its the right tradeoffs.

Besides, there are possibly even less invasive solutions. If we hit an 
exception from the analyzer, we could write that we hit an exception (not the 
exception text, just that we hit one) for field X to the infostream in the if 
(!success) case. This means there is no catch block at all! I would support 
such a change today. sure its not "perfectly user friendly" but it has 
absolutely no downsides at all, and only helps the user, and remember, this is 
*all about the buggy analyzer case*. We don't need to be user friendly for 
that, just safe.

> Exception strategy for analysis improved
> ----------------------------------------
>
>                 Key: LUCENE-5405
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5405
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Benson Margulies
>
> SOLR-5623 included some conversation about the dilemmas of exception 
> management and reporting in the analysis chain. Here is a 5.0 proposal:
> TokenStream.reset and TokenStream.incrementToken (and perhaps the rest) 
> should have a new, checked, exception in their signatures: call it 
> AnalysisError if you like. Unlike IOException, it will have a full set of 
> constructors, including the constructors that can wrap a 'cause'. Its 
> constructors will accept a field name.
> TokenStream will have a fieldName field, accepted in a constructor argument. 
> (OK, this might a bit authoritarian.)
> TokenStream will have:
>   protected void throwAnalysisException(String explanation, Throwable cause) {
>     throw new AnalysisException(fieldName, explanation, cause);
>   }
> Implementors of analysis will be thus encouraged to write things like:
>   try {
>     doSomething();
>   } catch (IOExceptionOrWhatever e) {
>     throwAnalysisException("Some Explanation", e);
>  }
> Then, situations like Solr can diagnose the field name.
> Note that no information is lost here, due to the use of exception wrapping.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to