[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17585016#comment-17585016 ] Ben commented on KAFKA-13978: - Looks like it was resolved in this PR: https://github.com/apache/kafka/pull/12421 > 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 >Assignee: Ben >Priority: Blocker > > 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.10#820010)
[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17578023#comment-17578023 ] John Roesler commented on KAFKA-13978: -- Hey [~bryves] , I just checked trunk and 3.3, and I don't see the code that you're removing in your PR. It seems like someone else may have fixed this issue in the mean time. Can you confirm whether or not the issue is resolved? > 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 >Assignee: Ben >Priority: Blocker > Fix For: 3.3.0 > > > 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.10#820010)
[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17559086#comment-17559086 ] Ben commented on KAFKA-13978: - Thanks! PR is up - though I had some CI issues which don't seem to be related to my changes. > 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 >Assignee: Ben >Priority: Blocker > Fix For: 3.3.0 > > > 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)
[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17558322#comment-17558322 ] A. Sophie Blee-Goldman commented on KAFKA-13978: Assigned to [~bryves] , also I added you as a contributor so you should be able to self-assign tickets from now on > 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 >Assignee: 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)
[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17558321#comment-17558321 ] A. Sophie Blee-Goldman commented on KAFKA-13978: [~bryves] feel free to open a PR to revert this > 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)
[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ 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 commented on KAFKA-13978: 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)
[jira] [Commented] (KAFKA-13978) Trigger UncaughtExceptionHandler for IllegalArgument and IllegalState exceptions
[ https://issues.apache.org/jira/browse/KAFKA-13978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17552762#comment-17552762 ] Ben commented on KAFKA-13978: - I'm also happy to work on making these changes. > 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)
