[jira] [Commented] (SOLR-15697) replace SolrException.ignorePatterns with something that doesn't depend on SolrException.log()

["Chris M. Hostetter (Jira)" <ji...@apache.org>]

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

Chris M. Hostetter commented on SOLR-15697:
-------------------------------------------

bq. * replace all {{SolrException.log}} usage in Solr to just call {{log.error(...)}} directly

This is non-trivial, and not strictly neccessary as long as we have a good replacement for {{SolrTestCaseJ4.ignoreException(...)}} so i spun it off into SOLR-15703 (where i've explained in more depth why it's a PITA)

bq. * audit all usage of {{SolrTestCaseJ4.ignoreException(...)}} to ensure it's expecting osmethign in the actaul log message, ant no deep in the stacktrace of the Throwable.

This is also a PITA ... i tried to validate this by adding a bit of a hack to insure that if {{SolrTestCaseJ4.ignoreException(...)}} is called explicitly by a test, then there must be at least one matching ERROR that gets Muted -- but it turns out there are _lot_ of usages of {{SolrTestCaseJ4.ignoreException(...)}} that are just plain "wrong" in the sense that they typos or other subtle differences from the actual error message logged.

I did however confirm that are a non-zero (and non-one) number of places in the code that do things like this...

{noformat}
      } catch (RuntimeException se) {
        log.error("Error executing command  ", se);
        throw se;
{noformat}

...and then the testcase calls {{SolrTestCaseJ4.ignoreException(...)}} on something in the exception's message.

I had initially thought that if i found situations like these i would just audit the code to be more like {{log.error("Error executing command {}", se.toString(), se);}} so that we didn't need to do this -- something that i think would be much nicer for users in most cases -- but i'm not willing to tackle that right now.  I think it's safe to just make the ErrorLogMuter also check the message strings of the Throwable (and it's nested throwables since we distributed code that frequently just wraps a remote exception and tests check for the nested message)

(The real irony of some of these {{SolrTestCaseJ4.ignoreException(...)}} usages already don't work today becuase the code in question -- like the snippet above -- doesn' even _use_ {{SolrException.log(...)}} ... ugh.)

> replace SolrException.ignorePatterns with something that doesn't depend on SolrException.log()
> ----------------------------------------------------------------------------------------------
>
>                 Key: SOLR-15697
>                 URL: https://issues.apache.org/jira/browse/SOLR-15697
>             Project: Solr
>          Issue Type: Sub-task
>            Reporter: Chris M. Hostetter
>            Assignee: Chris M. Hostetter
>            Priority: Major
>         Attachments: SOLR-15697.patch
>
>
> We should deprecated & remove the following code:
>  * {{SolrException.log()}}
>  * {{SolrException.ignorePatterns}}
>  * {{SolrTestCaseJ4.ignoreException()}}
>  * {{SolrTestCaseJ4.unIgnoreException()}}
>  * {{SolrTestCaseJ4.resetExceptionIgnores()}}
> ...and replace with something that hooks directly into logj4 Filtering



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org