From 9aef9921184d4f8fbf2e7af4875ca922173e2a8d Mon Sep 17 00:00:00 2001 From: Divij Vaidya Date: Tue, 23 Aug 2022 11:34:39 +0200 Subject: [PATCH] =?UTF-8?q?MINOR:=20Catch=C2=A0InvocationTargetException?= =?UTF-8?q?=C2=A0explicitly=20and=20propagate=20underlying=20cause=20(#122?= =?UTF-8?q?30)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Catch InvocationTargetException explicitly and propagate underlying cause Reviewers: Ismael Juma , Matthew de Detrich , Kvicii, Luke Chen --- .../common/utils/LoggingSignalHandler.java | 9 +++- .../org/apache/kafka/common/utils/Utils.java | 3 +- .../apache/kafka/common/KafkaFutureTest.java | 44 ++++++------------- .../kafka/metadata/RecordTestUtils.java | 2 +- 4 files changed, 23 insertions(+), 35 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/utils/LoggingSignalHandler.java b/clients/src/main/java/org/apache/kafka/common/utils/LoggingSignalHandler.java index 112d7fdb3ed..55eb49a2704 100644 --- a/clients/src/main/java/org/apache/kafka/common/utils/LoggingSignalHandler.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/LoggingSignalHandler.java @@ -21,6 +21,7 @@ import org.slf4j.LoggerFactory; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Arrays; @@ -75,8 +76,12 @@ public class LoggingSignalHandler { private Object createSignalHandler(final Map jvmSignalHandlers) { InvocationHandler invocationHandler = new InvocationHandler() { - private String getName(Object signal) throws ReflectiveOperationException { - return (String) signalGetNameMethod.invoke(signal); + private String getName(Object signal) throws Throwable { + try { + return (String) signalGetNameMethod.invoke(signal); + } catch (InvocationTargetException e) { + throw e.getCause(); + } } private void handle(Object signalHandler, Object signal) throws ReflectiveOperationException { diff --git a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java index 7d84167cf24..fef671896ed 100755 --- a/clients/src/main/java/org/apache/kafka/common/utils/Utils.java +++ b/clients/src/main/java/org/apache/kafka/common/utils/Utils.java @@ -463,8 +463,7 @@ public final class Utils { throw new ClassNotFoundException(String.format("Unable to access " + "constructor of %s", className), e); } catch (InvocationTargetException e) { - throw new ClassNotFoundException(String.format("Unable to invoke " + - "constructor of %s", className), e); + throw new KafkaException(String.format("The constructor of %s threw an exception", className), e.getCause()); } } diff --git a/clients/src/test/java/org/apache/kafka/common/KafkaFutureTest.java b/clients/src/test/java/org/apache/kafka/common/KafkaFutureTest.java index 0218ce15bd0..c9c36926c39 100644 --- a/clients/src/test/java/org/apache/kafka/common/KafkaFutureTest.java +++ b/clients/src/test/java/org/apache/kafka/common/KafkaFutureTest.java @@ -122,6 +122,14 @@ public class KafkaFutureTest { assertEquals(CancellationException.class, cancellationException.getClass()); } + private Object invokeOrThrow(final Method method, final Object obj, final Object... args) throws Throwable { + try { + return method.invoke(obj, args); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + } + @Test public void testCompleteFutures() throws Exception { KafkaFutureImpl future123 = new KafkaFutureImpl<>(); @@ -591,7 +599,7 @@ public class KafkaFutureTest { @Test @SuppressWarnings("unchecked") - public void testLeakCompletableFuture() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + public void testLeakCompletableFuture() throws Throwable { final KafkaFutureImpl kfut = new KafkaFutureImpl<>(); CompletableFuture comfut = kfut.toCompletionStage().toCompletableFuture(); assertThrows(UnsupportedOperationException.class, () -> comfut.complete("")); @@ -600,44 +608,20 @@ public class KafkaFutureTest { // so test reflectively if (Java.IS_JAVA9_COMPATIBLE) { Method completeOnTimeout = CompletableFuture.class.getDeclaredMethod("completeOnTimeout", Object.class, Long.TYPE, TimeUnit.class); - assertThrows(UnsupportedOperationException.class, () -> { - try { - completeOnTimeout.invoke(comfut, "", 1L, TimeUnit.MILLISECONDS); - } catch (InvocationTargetException e) { - throw e.getCause(); - } - }); + assertThrows(UnsupportedOperationException.class, () -> invokeOrThrow(completeOnTimeout, comfut, "", 1L, TimeUnit.MILLISECONDS)); Method completeAsync = CompletableFuture.class.getDeclaredMethod("completeAsync", Supplier.class); - assertThrows(UnsupportedOperationException.class, () -> { - try { - completeAsync.invoke(comfut, (Supplier) () -> ""); - } catch (InvocationTargetException e) { - throw e.getCause(); - } - }); + assertThrows(UnsupportedOperationException.class, () -> invokeOrThrow(completeAsync, comfut, (Supplier) () -> "")); Method obtrudeValue = CompletableFuture.class.getDeclaredMethod("obtrudeValue", Object.class); - assertThrows(UnsupportedOperationException.class, () -> { - try { - obtrudeValue.invoke(comfut, ""); - } catch (InvocationTargetException e) { - throw e.getCause(); - } - }); + assertThrows(UnsupportedOperationException.class, () -> invokeOrThrow(obtrudeValue, comfut, "")); Method obtrudeException = CompletableFuture.class.getDeclaredMethod("obtrudeException", Throwable.class); - assertThrows(UnsupportedOperationException.class, () -> { - try { - obtrudeException.invoke(comfut, new RuntimeException()); - } catch (InvocationTargetException e) { - throw e.getCause(); - } - }); + assertThrows(UnsupportedOperationException.class, () -> invokeOrThrow(obtrudeException, comfut, new RuntimeException())); // Check the CF from a minimal CompletionStage doesn't cause completion of the original KafkaFuture Method minimal = CompletableFuture.class.getDeclaredMethod("minimalCompletionStage"); - CompletionStage cs = (CompletionStage) minimal.invoke(comfut); + CompletionStage cs = (CompletionStage) invokeOrThrow(minimal, comfut); cs.toCompletableFuture().complete(""); assertFalse(kfut.isDone()); diff --git a/metadata/src/test/java/org/apache/kafka/metadata/RecordTestUtils.java b/metadata/src/test/java/org/apache/kafka/metadata/RecordTestUtils.java index c21bdb54478..17d8663b1a2 100644 --- a/metadata/src/test/java/org/apache/kafka/metadata/RecordTestUtils.java +++ b/metadata/src/test/java/org/apache/kafka/metadata/RecordTestUtils.java @@ -78,7 +78,7 @@ public class RecordTestUtils { } } } catch (InvocationTargetException e) { - throw new RuntimeException(e); + throw new RuntimeException(e.getCause()); } catch (IllegalAccessException e) { throw new RuntimeException(e); }