divijvaidya commented on code in PR #12230:
URL: https://github.com/apache/kafka/pull/12230#discussion_r955866446
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -463,8 +463,7 @@ public static <T> T newParameterizedInstance(String
className, Object... params)
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());
Review Comment:
True, but I would disagree that it is a downside.
In my opinion, a caller should be handling the underlying cause because the
`InvocationTargetException` is being thrown from business logic inside the
constructor. If the caller catches a generic `ReflectiveOperationException` it
doesn't get to know (without inspecting details) whether the exception is due
to a "reflection op error" such as method not found, class not found etc. or
whether it is an exception thrown by the business logic itself. I believe that
propagating the underlying business logic exception is better in this scenario.
Having said that, I don't have a strong opinion on this one as it's a minor
improvement that will only impact code debuggability when looking at exception
traces. Happy to revert if you think otherwise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
