Prohibit Throwables.propagate() (apache#7121)

* Throw caught exception. * Throw caught exceptions. * Related checkstyle rule is added to prevent further bugs. * RuntimeException() is used instead of Throwables.propagate(). * Missing import is added. * Throwables are propogated if possible. * Throwables are propogated if possible. * Throwables are propogated if possible. * Throwables are propogated if possible. * * Checkstyle definition is improved. * Throwables.propagate() usages are removed. * Checkstyle pattern is changed for only scanning "Throwables.propagate(" instead of checking lookbehind. * Throwable is kept before firing a Runtime Exception. * Fix unused assignments.
will-lauer · Mar 14, 2019 · 7ada1c4 · 7ada1c4
1 parent e113648
commit 7ada1c4
Show file tree

Hide file tree

Showing 229 changed files with 462 additions and 646 deletions.
diff --git a/benchmarks/src/main/java/org/apache/druid/benchmark/GroupByTypeInterfaceBenchmark.java b/benchmarks/src/main/java/org/apache/druid/benchmark/GroupByTypeInterfaceBenchmark.java
@@ -23,7 +23,6 @@
 import com.fasterxml.jackson.dataformat.smile.SmileFactory;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
-import com.google.common.base.Throwables;
 import com.google.common.io.Files;
 import org.apache.commons.io.FileUtils;
 import org.apache.druid.benchmark.datagen.BenchmarkDataGenerator;
@@ -442,7 +441,7 @@ public void tearDown()
     }
     catch (IOException e) {
       log.warn(e, "Failed to tear down, temp dir was: %s", tmpDir);
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
   }
 

diff --git a/benchmarks/src/main/java/org/apache/druid/benchmark/datagen/SegmentGenerator.java b/benchmarks/src/main/java/org/apache/druid/benchmark/datagen/SegmentGenerator.java
@@ -19,7 +19,6 @@
 
 package org.apache.druid.benchmark.datagen;
 
-import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.io.Files;
@@ -170,7 +169,7 @@ public QueryableIndex generate(
         return merged;
       }
       catch (IOException e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
     }
   }

diff --git a/benchmarks/src/main/java/org/apache/druid/benchmark/query/GroupByBenchmark.java b/benchmarks/src/main/java/org/apache/druid/benchmark/query/GroupByBenchmark.java
@@ -23,7 +23,6 @@
 import com.fasterxml.jackson.dataformat.smile.SmileFactory;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
-import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
@@ -535,7 +534,7 @@ public void tearDown()
     }
     catch (IOException e) {
       log.warn(e, "Failed to tear down, temp dir was: %s", tmpDir);
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
   }
 

diff --git a/codestyle/checkstyle.xml b/codestyle/checkstyle.xml
@@ -204,6 +204,12 @@
       <property name="message" value="Use org.apache.druid.server.http.HttpMediaType#TEXT_PLAIN_UTF8 instead"/>
     </module>
 
+    <module name="Regexp">
+      <property name="format" value='^Throwables.propagate\('/>
+      <property name="illegalPattern" value="true"/>
+      <property name="message" value="Throwables.propagate() shouldn't be used in new code"/>
+    </module>
+
     <module name="PackageName">
       <property name="format" value="^org.apache.druid.*$"/>
     </module>

diff --git a/core/src/main/java/org/apache/druid/collections/OrderedMergeSequence.java b/core/src/main/java/org/apache/druid/collections/OrderedMergeSequence.java
@@ -20,7 +20,6 @@
 package org.apache.druid.collections;
 
 import com.google.common.base.Function;
-import com.google.common.base.Throwables;
 import com.google.common.collect.Ordering;
 import org.apache.druid.java.util.common.guava.Accumulator;
 import org.apache.druid.java.util.common.guava.CloseQuietly;
@@ -116,7 +115,7 @@ public T accumulate(T accumulated, T in)
                 retVal.close();
               }
               catch (IOException e) {
-                throw Throwables.propagate(e);
+                throw new RuntimeException(e);
               }
               return null;
             } else {
@@ -165,7 +164,7 @@ private <OutType> Yielder<OutType> makeYielder(
           yielder.close();
         }
         catch (IOException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       } else {
         pQueue.add(yielder);

diff --git a/core/src/main/java/org/apache/druid/collections/ReferenceCountingResourceHolder.java b/core/src/main/java/org/apache/druid/collections/ReferenceCountingResourceHolder.java
@@ -19,7 +19,6 @@
 
 package org.apache.druid.collections;
 
-import com.google.common.base.Throwables;
 import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.logger.Logger;
 import sun.misc.Cleaner;
@@ -137,7 +136,7 @@ private void decrement()
         closer.close();
       }
       catch (IOException e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
     }
   }

diff --git a/core/src/main/java/org/apache/druid/common/config/ConfigManager.java b/core/src/main/java/org/apache/druid/common/config/ConfigManager.java
@@ -20,7 +20,6 @@
 package org.apache.druid.common.config;
 
 import com.google.common.base.Supplier;
-import com.google.common.base.Throwables;
 import com.google.inject.Inject;
 import org.apache.druid.java.util.common.concurrent.ScheduledExecutors;
 import org.apache.druid.java.util.common.lifecycle.LifecycleStart;
@@ -159,10 +158,10 @@ public ConfigHolder<T> call()
       }
       catch (InterruptedException e) {
         Thread.currentThread().interrupt();
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
       catch (ExecutionException e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
     }
 

diff --git a/core/src/main/java/org/apache/druid/common/config/JacksonConfigManager.java b/core/src/main/java/org/apache/druid/common/config/JacksonConfigManager.java
@@ -22,7 +22,6 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.base.Throwables;
 import com.google.inject.Inject;
 import org.apache.druid.audit.AuditEntry;
 import org.apache.druid.audit.AuditInfo;
@@ -94,7 +93,7 @@ public byte[] serialize(T obj)
           return jsonMapper.writeValueAsBytes(obj);
         }
         catch (JsonProcessingException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
 
@@ -105,7 +104,7 @@ public String serializeToString(T obj)
           return jsonMapper.writeValueAsString(obj);
         }
         catch (JsonProcessingException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
 
@@ -120,7 +119,7 @@ public T deserialize(byte[] bytes)
           return jsonMapper.readValue(bytes, clazz);
         }
         catch (IOException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
     };
@@ -137,7 +136,7 @@ public byte[] serialize(T obj)
           return jsonMapper.writeValueAsBytes(obj);
         }
         catch (JsonProcessingException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
 
@@ -148,7 +147,7 @@ public String serializeToString(T obj)
           return jsonMapper.writeValueAsString(obj);
         }
         catch (JsonProcessingException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
 
@@ -163,7 +162,7 @@ public T deserialize(byte[] bytes)
           return jsonMapper.readValue(bytes, clazz);
         }
         catch (IOException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
     };

diff --git a/core/src/main/java/org/apache/druid/common/config/Log4jShutdown.java b/core/src/main/java/org/apache/druid/common/config/Log4jShutdown.java
@@ -19,7 +19,6 @@
 
 package org.apache.druid.common.config;
 
-import com.google.common.base.Throwables;
 import org.apache.druid.java.util.common.ISE;
 import org.apache.logging.log4j.core.LifeCycle;
 import org.apache.logging.log4j.core.util.Cancellable;
@@ -184,7 +183,7 @@ private synchronized State waitForTransition(State expected, State transition, l
           }
         }
         catch (InterruptedException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
       return current;

diff --git a/core/src/main/java/org/apache/druid/data/input/impl/AbstractTextFilesFirehoseFactory.java b/core/src/main/java/org/apache/druid/data/input/impl/AbstractTextFilesFirehoseFactory.java
@@ -20,7 +20,6 @@
 package org.apache.druid.data.input.impl;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.LineIterator;
@@ -82,7 +81,7 @@ public LineIterator next()
                   "Exception reading object[%s]",
                   object
               );
-              throw Throwables.propagate(e);
+              throw new RuntimeException(e);
             }
           }
         },

diff --git a/core/src/main/java/org/apache/druid/data/input/impl/prefetch/Fetcher.java b/core/src/main/java/org/apache/druid/data/input/impl/prefetch/Fetcher.java
@@ -20,7 +20,6 @@
 package org.apache.druid.data.input.impl.prefetch;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Throwables;
 import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.logger.Logger;
 
@@ -224,7 +223,7 @@ private OpenedObject<T> openObjectFromLocal() throws IOException
         }
       }
       catch (InterruptedException e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
     }
     final FetchedFile<T> maybeCached = cacheIfPossible(fetchedFile);
@@ -252,7 +251,7 @@ private OpenedObject<T> openObjectFromRemote() throws IOException
         return new OpenedObject<>(cached);
       }
       catch (Exception e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
     } else {
       final T object = objects.get(nextFetchIndex);

diff --git a/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java b/core/src/main/java/org/apache/druid/guice/JsonConfigurator.java
@@ -27,7 +27,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Strings;
-import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 import com.google.inject.ProvisionException;
@@ -167,7 +166,7 @@ public <T> T configurate(
           }
         }
         catch (NoSuchFieldException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
 
         messages.add(StringUtils.format("%s - %s", path.toString(), violation.getMessage()));

diff --git a/core/src/main/java/org/apache/druid/java/util/common/FileUtils.java b/core/src/main/java/org/apache/druid/java/util/common/FileUtils.java
@@ -20,7 +20,6 @@
 package org.apache.druid.java.util.common;
 
 import com.google.common.base.Predicate;
-import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.ByteSource;
 import com.google.common.io.Files;
@@ -85,7 +84,7 @@ public static FileCopyResult retryCopy(
       return new FileCopyResult(outFile);
     }
     catch (Exception e) {
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
   }
 

diff --git a/core/src/main/java/org/apache/druid/java/util/common/RetryUtils.java b/core/src/main/java/org/apache/druid/java/util/common/RetryUtils.java
@@ -93,7 +93,7 @@ public static <T> T retry(
           awaitNextRetry(e, messageOnRetry, nTry, maxRetries, nTry <= quietTries);
         } else {
           Throwables.propagateIfInstanceOf(e, Exception.class);
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       }
     }

diff --git a/core/src/main/java/org/apache/druid/java/util/common/StreamUtils.java b/core/src/main/java/org/apache/druid/java/util/common/StreamUtils.java
@@ -20,7 +20,6 @@
 package org.apache.druid.java.util.common;
 
 import com.google.common.base.Predicate;
-import com.google.common.base.Throwables;
 import com.google.common.io.ByteSink;
 import com.google.common.io.ByteSource;
 import com.google.common.io.ByteStreams;
@@ -118,7 +117,7 @@ public static long retryCopy(
       );
     }
     catch (Exception e) {
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
   }
 }
diff --git a/core/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/core/src/main/java/org/apache/druid/java/util/common/StringUtils.java
@@ -20,7 +20,6 @@
 package org.apache.druid.java.util.common;
 
 import com.google.common.base.Strings;
-import com.google.common.base.Throwables;
 
 import javax.annotation.Nullable;
 import java.io.UnsupportedEncodingException;
@@ -80,7 +79,7 @@ public static String fromUtf8(final byte[] bytes)
     }
     catch (UnsupportedEncodingException e) {
       // Should never happen
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
   }
 
@@ -103,7 +102,7 @@ public static byte[] toUtf8(final String string)
     }
     catch (UnsupportedEncodingException e) {
       // Should never happen
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
   }
 

diff --git a/core/src/main/java/org/apache/druid/java/util/common/concurrent/ExecutorServices.java b/core/src/main/java/org/apache/druid/java/util/common/concurrent/ExecutorServices.java
@@ -19,7 +19,6 @@
 
 package org.apache.druid.java.util.common.concurrent;
 
-import com.google.common.base.Throwables;
 import org.apache.druid.java.util.common.lifecycle.Lifecycle;
 
 import java.util.concurrent.ExecutorService;
@@ -47,7 +46,7 @@ public void stop()
       );
     }
     catch (Exception e) {
-      Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
     return service;
   }

diff --git a/core/src/main/java/org/apache/druid/java/util/common/guava/ConcatSequence.java b/core/src/main/java/org/apache/druid/java/util/common/guava/ConcatSequence.java
@@ -19,8 +19,6 @@
 
 package org.apache.druid.java.util.common.guava;
 
-import com.google.common.base.Throwables;
-
 import java.io.Closeable;
 import java.io.IOException;
 
@@ -93,7 +91,7 @@ public <OutType> Yielder<OutType> makeYielder(
         yielder.close();
       }
       catch (IOException e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
 
       yielderYielder = yielderYielder.next(null);
@@ -114,7 +112,7 @@ private <OutType> Yielder<OutType> wrapYielder(
         yielder.close();
       }
       catch (IOException e) {
-        throw Throwables.propagate(e);
+        throw new RuntimeException(e);
       }
 
       return makeYielder(yielderYielder.next(null), nextInit, accumulator);

diff --git a/core/src/main/java/org/apache/druid/java/util/common/guava/MergeSequence.java b/core/src/main/java/org/apache/druid/java/util/common/guava/MergeSequence.java
@@ -20,7 +20,6 @@
 package org.apache.druid.java.util.common.guava;
 
 import com.google.common.base.Function;
-import com.google.common.base.Throwables;
 import com.google.common.collect.Ordering;
 import org.apache.druid.java.util.common.io.Closer;
 
@@ -110,7 +109,7 @@ private <OutType> Yielder<OutType> makeYielder(
           yielder.close();
         }
         catch (IOException e) {
-          throw Throwables.propagate(e);
+          throw new RuntimeException(e);
         }
       } else {
         pQueue.add(yielder);