[
https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ben updated KAFKA-13978:
------------------------
Description:
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 caught.
* 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.
was:
In 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 caught.
* 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.
> 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 caught.
>
> * 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)
