You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2021/10/25 19:00:01 UTC

[lucene-solr] branch branch_8x updated: SOLR-15628: The SolrException.log() helper method has been fixed to correctly passes the Throwable to the Logger w/o stringification

This is an automated email from the ASF dual-hosted git repository.

hossman pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new eac7e9e  SOLR-15628: The SolrException.log() helper method has been fixed to correctly passes the Throwable to the Logger w/o stringification
eac7e9e is described below

commit eac7e9efc054357d7219040b1e36a9dc202deea1
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Tue Sep 14 10:47:18 2021 -0700

    SOLR-15628: The SolrException.log() helper method has been fixed to correctly passes the Throwable to the Logger w/o stringification
    
    (cherry picked from commit b60642c719f73f998d5130837b0fbaa1b91687a7)
---
 solr/CHANGES.txt                                   |   2 +
 .../java/org/apache/solr/common/SolrException.java | 112 ++++++++++++++-------
 2 files changed, 76 insertions(+), 38 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 06f1a3f..a1e3619 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -34,6 +34,8 @@ Bug Fixes
 
 * SOLR-15696: Incremental backups no longer fail on collections where a 'SPLITSHARD' operation previously occurred. (Jason Gerlowski)
 
+* SOLR-15628: The SolrException.log() helper method has been fixed to correctly passes the Throwable to the Logger w/o stringification (hossman)
+
 Build
 ---------------------
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/SolrException.java b/solr/solrj/src/java/org/apache/solr/common/SolrException.java
index 107fe19..0a6a9ea 100644
--- a/solr/solrj/src/java/org/apache/solr/common/SolrException.java
+++ b/solr/solrj/src/java/org/apache/solr/common/SolrException.java
@@ -20,7 +20,6 @@ import java.io.CharArrayWriter;
 import java.io.PrintWriter;
 import java.util.Map;
 import java.util.Set;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.solr.common.util.NamedList;
 import org.slf4j.Logger;
@@ -137,35 +136,45 @@ public class SolrException extends RuntimeException {
     return getMetadata(ROOT_ERROR_CLASS);
   }
 
-  public void log(Logger log) { log(log,this); }
+  /** @see #ignorePatterns */
+  public void log(Logger log) {
+    log(log,this);
+  }
+  
+  /** @see #ignorePatterns */
   public static void log(Logger log, Throwable e) {
-    String stackTrace = toStr(e);
-    String ignore = doIgnore(e, stackTrace);
-    if (ignore != null) {
-      log.info(ignore);
-      return;
+    if (log.isErrorEnabled()) {
+      String ignore = doIgnoreToStr(null, e);
+      if (ignore != null) {
+        log.info(ignore);
+        return;
+      }
+      log.error(e.toString(), e); // nowarn (we are inside of isErrorEnabled, toString as msg is ok)
     }
-    log.error(stackTrace);
-
   }
 
+  /** @see #ignorePatterns */
   public static void log(Logger log, String msg, Throwable e) {
-    String stackTrace = msg + ':' + toStr(e);
-    String ignore = doIgnore(e, stackTrace);
-    if (ignore != null) {
-      log.info(ignore);
-      return;
+    if (log.isErrorEnabled()) {
+      String ignore = doIgnoreToStr(msg, e);
+      if (ignore != null) {
+        log.info(ignore);
+        return;
+      }
+      log.error(msg, e);
     }
-    log.error(stackTrace);
   }
   
+  /** @see #ignorePatterns */
   public static void log(Logger log, String msg) {
-    String ignore = doIgnore(null, msg);
-    if (ignore != null) {
-      log.info(ignore);
-      return;
+    if (log.isErrorEnabled()) {
+      String ignore = doIgnoreToStr(msg, null);
+      if (ignore != null) {
+        log.info(ignore);
+        return;
+      }
+      log.error(msg);
     }
-    log.error(msg);
   }
 
   public static String toStr(Throwable e) {
@@ -174,35 +183,62 @@ public class SolrException extends RuntimeException {
     e.printStackTrace(pw);
     pw.flush();
     return cw.toString();
-
-/** This doesn't work for some reason!!!!!
-    StringWriter sw = new StringWriter();
-    PrintWriter pw = new PrintWriter(sw);
-    e.printStackTrace(pw);
-    pw.flush();
-    System.out.println("The STRING:" + sw.toString());
-    return sw.toString();
-**/
   }
 
-
   /**
-   * For test code - do not log exceptions that match any of these regular expressions.
+   * For test code: If non-null, prevents calls to {@link #log} from logging any msg or exception (stack trace) that matches an included regular expressions.
+   *
    * A {@link java.util.concurrent.CopyOnWriteArraySet is recommended}.
    */
   public static Set<String> ignorePatterns;
 
-  /** Returns null if this exception does not match any ignore patterns, or a message string to use if it does. */
-  public static String doIgnore(Throwable t, String m) {
-    Set<String> ignorePatterns = SolrException.ignorePatterns; // guard against races, albeit unlikely
-    if (ignorePatterns == null || m == null) return null;
+  /** 
+   * Returns null if this exception does not match any ignore patterns; or an INFO message string to log instead if it does.
+   *
+   * @param t the original exception (only used for assertion checking)
+   * @param stacktrace the stringified stack trace of the exception, used for the acutal regex checking
+   * @see #ignorePatterns
+   * @see #toStr
+   */
+  public static String doIgnore(Throwable t, String stacktrace) { 
     if (t != null && t instanceof AssertionError) return null;
+    
+    Set<String> ignorePatterns = SolrException.ignorePatterns; // guard against races, albeit unlikely
+    // legacy public API: caller is required to have already stringified exception...
+    return doIgnoreToStr(ignorePatterns, stacktrace, null);
+  }
 
-    for (String regex : ignorePatterns) {
+  /** @see #doIgnoreToStr(Set, String, Throwable) */
+  private static String doIgnoreToStr(String msg, Throwable t) {
+    if (t != null && t instanceof AssertionError) return null;
+    
+    Set<String> ignorePatterns = SolrException.ignorePatterns; // guard against races, albeit unlikely
+    return doIgnoreToStr(ignorePatterns, msg, t);
+  }
+  
+  /** 
+   * Returns null if the stringToCheck + exceptionToCheck does not match any of the ignore patterns; 
+   * or an INFO message string to log instead if it does.
+   *
+   * @param ignorePats patterns to match against
+   * @param stringToCheck arbitrary string to check against each ignore pattern
+   * @param exceptionToCheck if non-null, will be stringified and concatenated with stringToCheck before testing patterns
+   * @see #ignorePatterns
+   * @see #toStr
+   */
+  private static String doIgnoreToStr(Set<String> ignorePats, String stringToCheck, Throwable exceptionToCheck) {
+    if (null == ignorePats) return null;
+    
+    // we have some patterns, so we can't avoid stringifying exception for checks.
+    if (null != exceptionToCheck) {
+      // legacy concat of msg + throwable...
+      stringToCheck = (null == stringToCheck ? "" : stringToCheck+':') + toStr(exceptionToCheck);
+    }
+    
+    for (String regex : ignorePats) {
       Pattern pattern = Pattern.compile(regex); // TODO why do we compile late; why not up-front?
-      Matcher matcher = pattern.matcher(m);
       
-      if (matcher.find()) return "Ignoring exception matching " + regex;
+      if (pattern.matcher(stringToCheck).find()) return "Ignoring exception matching " + regex;
     }
 
     return null;