[
https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17558320#comment-17558320
]
A. Sophie Blee-Goldman edited comment on KAFKA-13978 at 6/24/22 4:24 AM:
-------------------------------------------------------------------------
Yeah we've discussed this a bit and personally, I agree that we should revert
that commit at least until we can do a better job of classifying/identifying
exceptions. Maybe in the long term we would want to, for example, not allow the
user to opt to replace the thread/continue processing if the exception is known
to be fatal, or corrupting data, etc – but we don't have that kind of error
parsing in place right now, so it doesn't make sense to just unilaterally
consider all IllegalState/IllegalArgument exceptions as fatal (and even if they
are, we should still invoke the handler to notify the user)
was (Author: ableegoldman):
Yeah we've discussed this a bit and personally, I agree that we should revert
that commit at least until we can do a better job of classifying/identifying
exceptions. Maybe in the long term we would want to, for example, not allow the
user to opt to replace the thread/continue processing if the exception is known
to be fatal, or corrupting data, etc – but we don't have that kind of error
parsing in place right now, so it doesn't make sense to just unilaterally
consider all IllegalState/IllegalArgument exceptions as fatal (and even if they
are, we should still invoke the handler to notify the user)
> Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState
> exceptions
> --------------------------------------------------------------------------------
>
> Key: KAFKA-13978
> URL: https://issues.apache.org/jira/browse/KAFKA-13978
> Project: Kafka
> Issue Type: Bug
> Components: streams
> Affects Versions: 3.1.0, 3.2.0, 3.1.1
> Reporter: Ben
> Priority: Major
>
> In [KAFKA-12887|https://issues.apache.org/jira/browse/KAFKA-12887]
> [(PR)|https://github.com/apache/kafka/pull/11228/files] changes were made to
> prevent thrown IllegalStateException and IllegalArgumentExceptions from being
> passed to a registered exception handler.
>
> I believe these changes should be reverted for the following reasons:
> * Making this change is backwards incompatible with existing applications
> which may have expected those exceptions to be handled.
>
> * Users can (and do!) throw these exceptions, often for legitimate reasons.
> For instance, IllegalArgumentException is thrown when a method is passed the
> wrong argument. This is exactly the type of uncaught exception a user would
> expect to be handled by the uncaught exception handler, rather than by the
> calling code.
>
> * The change is inconsistent. Why only these two exceptions, and not all
> runtime exceptions?
>
> * The change is not well documented. There are even tutorial resources which
> actually use these exceptions, [for example
> here|https://developer.confluent.io/tutorials/error-handling/confluent.html].
> If we make this change, it should be better communicated. As implemented, it
> is extremely surprising that this happens.
>
> * Finally, what value is the change actually adding to the project? It
> restricts user freedom, increases complexity, and does not improve safety. We
> should only make a backwards-incompatible change like this if there is clear
> value in doing so.
>
> As a note, reverting this is not (in my view) going to impact users
> negatively. It is unlikely many people depend on this functionality, and if
> they do, it should be easy to communicate in the release notes, and for them
> to adjust their code accordingly.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
